Fix a Runtime Crash

[tobnieder]

I discovered a problem with the options.indent:

You receive an error, if you have indentation in some place:

./node_modules/handlebars/lib/handlebars/runtime.js:99
        let lines = result.split('\n');
TypeError: result.split is not a function

This is due the fact, that result is a Promise and not a string.
To fix this I analyzed the usage of this particular code path and fixed it by using a "tampered" Promise, that has these noop function, but performs the process on the result afterwards, this fixes the problem without modifying handlebar code.

I also added a test, that failed before the fix.

[gastonrobledo]

do we need this dummy methods since we are checking for string type?

[tobnieder]

Yes, the problem is the original handlebars check, since the original code is in the handlebar code, we can't modify it, feel free to remove it locally and see what brakes (it is in the handlebars code) here

[tobnieder]

We could submit a upstream patch, and then remove the dummy methods 🤔

[gastonrobledo]

Ok I commented out the dummy methods run the test and all passes.

 Test async helpers
    ✔ Test single async helper (63ms)
    ✔ Test async helper with partial template (58ms)
    ✔ Test each helper with async helpers inside (368ms)
    ✔ Test each helper with async helpers resolver (61ms)
    ✔ Test lookupProperty
    ✔ Test lookupProperty with nested promises
    ✔ Test with helper (54ms)
    ✔ Test if helper (63ms)
    ✔ Test unless helper (55ms)
    ✔ Test with custom helpers and complex replacements (486ms)
    ✔ Test single variable be a promise value
    ✔ Test each with a stream source
    ✔ Test normal usage of handlebars
    ✔ Test nested partials (61ms)
    ✔ Test indentation with async partial (56ms)
    ✔ Test synchronous helper
    ✔ Test custom helper without noEscape
    ✔ check version


  18 passing (1s)

Or the test is wrong or is ok to remove them.

[gastonrobledo]

in fact if I remove the proxy it also works

const result = compiled.call(handlebars, context, execOptions)
      if (!indent) {
        return result
      } else {

        return result.then(res => {
              if (indent && typeof res === 'string') {
                const lines = res.split('\n');
                for (let i = 0, l = lines.length; i < l; i++) {
                  if (!lines[i] && i + 1 === l) {
                    break;
                  }

                  lines[i] = indent + lines[i];
                }
                res = lines.join('\n');
              }
              return res
            })
        // this are dummy methods to work with handlebar code, it is only designed for usage with that, otherwise it may break!
        // result.split = () => { return result }
        // result.join = () => { return result }
        // result.length = 0

        // this is a proxy to post process the promise result, it overrides the then method and therefore does the indent split - join job after the result is here!
        // const resultProxy = new Proxy(result, {
        //   then(cb) {
            
        //   }
        // })

        // return resultProxy

      }

[tobnieder]

I'll investigate this 👍🏼

[tobnieder]

The issue definetely exists, I have a more advanced and complicated hbs file, where it occurs, it seems that my test doesn't catch the error. I wonder why, since I remember that he did, it may be, that I used a local npm package and i didn't refresh it 🤷🏼‍♂️
I'll search a test case where it breaks and than I try to reduce the usage of Proxy / dummy methods, it may take a while 😓

[gastonrobledo]

Ok let me know when to review again. Thanks

[tobnieder]

After investigating further, I discovered, that we don't handle indentation in many cases, and since it's not that important in HTML anyway; I removed the indentation algorithm at the end of the resolved Promise.
I compared the result of the new test case (that now fails without the small patch) with the standard handlebars compiler, and nearly every line has no indentation, when compiled with this wrapper.
So I decided to just fix the runtime crash, but not add the additional indent algorithm.