Use `handlebars.js` 3.0.2

[seanpdoyle]

Includes es6 build fixes found in:

https://github.com/wycats/handlebars.js/commit/0263aa48bc8c8cdcd332edd01a644a9a0fd1cc81

[mmun]

You'll need to update the transpiler in this project as well.

[seanpdoyle]

@mmun looks like it:

https://travis-ci.org/tildeio/htmlbars/builds/59241871#L145-L157

Should I follow the same pattern as whats in handlebars.js, or is there another project that I should use as a reference?

[seanpdoyle]

cc: @stefanpenner

[mmun]

I'd go with whatever rsvp + backburner + ember are doing. This project is structured similarly to ember.

[seanpdoyle]

@mmun to be particular, htmlbars uses Grunt and babel-loader for es6 modules, whereas ember uses broccoli and esperanto.

@rwjblue how should we go about building htmlbar's modules like the other projects?

[rwjblue]

We need to change https://github.com/tildeio/htmlbars/blob/master/package.json#L29 to use broccoli-es6modules.

[stefanpenner]

esperanoOptions -> esperantoOptions

[seanpdoyle]

@rwjblue that worked and the lib is now building.

We're still failing, though:

https://travis-ci.org/tildeio/htmlbars/builds/59276313#L545

ember test is green locally, and I'm not sure how to resolve the JSHint failures, since they seem to be in the build output (I think they're in files relative to dist/?)

Any ideas how to reproduce this failure, since it happens elsewhere on SauceLabs

[mmun]

The JSHint failures shouldn't be killing the build.

[mmun]

Actually, I think you can just fix the JSHint failures. Add more files to exclude near

  // jsHint tests
  var jsHintLibTree = new Funnel(libTree, {
    include: [new RegExp(packageName), new RegExp(packageName + '.+\.js$')],
    exclude: [/htmlbars-(syntax|util)\/handlebars/],
    destDir: packageName+'-tests/'
  });

[seanpdoyle]

@mmun ok, I figured as much, but unfortunately there isn't much helpful information in the failure message.

[mmun]

Travis reports that something is breaking on IE11.

[seanpdoyle]

@mmun judging by https://travis-ci.org/tildeio/htmlbars/builds/59276313#L545 I also think IE 11 is the culprit.

Unfortunately, there isn't a stack trace or any other failure information in the logs.

What would be the best approach to drilling deeper into this issue?

Are they SauceLab builds public? Where could I find the URLs?

[mmun]

The simplest thing is to test on a windows machine or download a VM: https://www.modern.ie/en-us/virtualization-tools. You can also see the build results here: https://saucelabs.com/tests/721bb921f91f43c7bb564bc5e76cf2dc.

Since the failure is only in IE11 my gut reaction is that there's no actual bug in the library code but instead the bug is in faulty test helpers / assertions that aren't normalizing their inputs sufficiently.

[mmun]

Bam https://assets.saucelabs.com/jobs/721bb921f91f43c7bb564bc5e76cf2dc/0002screenshot.png. Gut reaction wrong! Not sure exactly what's going on though.

[seanpdoyle]

@mmun we no longer have failing tests, but unfortunately, there once against isn't much actionable information in the failure trace:

https://travis-ci.org/tildeio/htmlbars/builds/59294040#L513-L517

[mmun]

Getting warmer! This seems useful, no?

513  Running: Node test runner against dist/cjs/htmlbars-tests/htmlbars-node-test.js
514  Error running node tests
515  Failed! { [Error: define is not defined] message: 'define is not defined' }

Why is define being called in node?

[seanpdoyle]

@mmun it does, but that's in the compiled output.

https://github.com/tildeio/htmlbars/blob/bd68a56e126e58a4ba3f1254f7bb7f147e6740b8/packages/htmlbars/tests/htmlbars-node-test.js

is producing code that invokes define without defining it (presumably because it's defined elsewhere in the testing harness).

From a build-tools perspective, how would we correct this?

[mmun]

Not sure, seems like it's compiling to AMD when it should be compiling to CJS. The options could be wrong?

[seanpdoyle]

@mmun since you wrote the previous transpilation pipeline, could you tell me if the moduleName: true behavior is matched by the new build output?

[seanpdoyle]

@mmun new failure: https://travis-ci.org/tildeio/htmlbars/builds/59304193#L489-L498

[seanpdoyle]

@mmun without strict mode: https://travis-ci.org/tildeio/htmlbars/builds/59305867#L490-L497

[mmun]

let shouldn't be reaching node. you should configure the transpiler to change them to vars. You should configure it the same as Ember. First babel, then esperanto.

[rwjblue]

we need esperanto for modules, but also babel for syntax

[rwjblue]

let won't work in node-land

[mmun]

@seanpdoyle can add emberjs-build as an NPM dependency and use this module to do the transpiling? https://github.com/emberjs/emberjs-build/blob/831a3a7dd9a0a8de25222c0242184dd470121287/lib/utils/transpile-es6.js

require("emberjs-build/lib/utils/transpile-es6")

This will ensure we don't go out of sync with Ember's settings which could lead to havoc.

[seanpdoyle]

@mmun emberjs-build's transpile-es6 doesn't accept format options, and even if it did, it explicitly calls toAmd (https://github.com/emberjs/emberjs-build/blob/831a3a7dd9a0a8de25222c0242184dd470121287/lib/utils/transpile-es6.js#L32), so cjs is unsupported

[mmun]

@seanpdoyle Yeah, we have to pass an option through for that.

[mmun]

You're welcome to copy & paste for now and we can refactor later.

[seanpdoyle]

@mmun @rwjblue even while using the same setup as emberjs-build, const is still getting through:

https://travis-ci.org/tildeio/htmlbars/builds/59318157#L567-L576

----
Running: Node test runner against dist/cjs/htmlbars-tests/htmlbars-node-test.js
/home/travis/build/tildeio/htmlbars/dist/cjs/htmlbars-syntax/handlebars/exception.js:4
const errorProps = ['description', 'fileName', 'lineNumber', 'message', 'name'
^^^^^
Error running node tests
Failed! { [Error: Use of const in strict mode.] message: 'Use of const in strict mode.' }

[mmun]

That's weird. It should be covered by blockScoping: http://babeljs.io/docs/learn-es6#let-const

[mmun]

Only thing I can think of is that this file is not going through the transpiler?

[seanpdoyle]

@mmun it must be.

heres the source file: https://github.com/wycats/handlebars.js/blob/master/lib/handlebars/exception.js

and the output:

(note the 'use strict' and exports['default'] = Exception;)

'use strict';

const errorProps = ['description', 'fileName', 'lineNumber', 'message', 'name', 'number', 'stack'];

function Exception(message, node) {
  var loc = node && node.loc,
      line = undefined,
      column = undefined;
  if (loc) {
    line = loc.start.line;
    column = loc.start.column;

    message += ' - ' + line + ':' + column;
  }

  var tmp = Error.prototype.constructor.call(this, message);

  // Unfortunately errors are not enumerable in Chrome (at least), so `for prop in tmp` doesn't work.
  for (var idx = 0; idx < errorProps.length; idx++) {
    this[errorProps[idx]] = tmp[errorProps[idx]];
  }

  if (Error.captureStackTrace) {
    Error.captureStackTrace(this, Exception);
  }

  if (loc) {
    this.lineNumber = line;
    this.column = column;
  }
}

Exception.prototype = new Error();

exports['default'] = Exception;

[mmun]

Yeah - it's definitely going through esperanto (the module transpiler), but possibly not Babel. It should definitely be converting the const to a var if its running. You can play with http://babeljs.io/repl.

[mmun]

Hmm, it's weird that the lets are transformed to vars though.

[mmun]

Sigh - I think I am wrong. I think we also need to enable es6.constants. The list is here: http://babeljs.io/docs/usage/transformers/

constants and blockScoping both link to the same anchor :P

[seanpdoyle]

@mmun :facepunch: :sparkles:

[seanpdoyle]

let me know if you think its good to go and I'll rebase into a single commit

[mmun]

It's good! :football:

[seanpdoyle]

@mmun @rwjblue followup PR:

https://github.com/emberjs/emberjs-build/pull/96

[seanpdoyle]

@mmun @rwjblue follow-up follow-up PR:

https://github.com/tildeio/htmlbars/pull/331

[seanpdoyle]

@mmun @rwjblue could we cut a release containing this commit?