Babel 6 #4862

rwjblue · 2017-03-14T21:53:55Z

No description provided.

rwjblue · 2017-03-15T01:53:31Z

Current failures are due to some node@4 incompatibilities in babel6-plugin-strip-heimdall that have been fixed upstream. I just need to confirm it is released and update here.

rwjblue · 2017-03-15T01:54:38Z

Everything is working locally (I believe), but I'd love for folks to test (using node@6 or higher for now) and report back.

rwjblue · 2017-03-15T02:21:45Z

@runspired - Can you release an updated babel6-plugin-strip-heimdall with node@4 support?

runspired · 2017-03-15T02:25:38Z

@rwjblue yep!

rwjblue · 2017-03-15T19:07:15Z

OK, this is ready for review. It has feature parity with prior babel@5 configuration, and fixes a bug in our globals build (which makes our 2.13.0-beta.1 bower builds completely broken).

The goal with this work is to get ember-cli@2.13 to only include addons that are using babel@6, as such I'd like to get this cherry-picked into beta branch once reviewed.

rwjblue · 2017-03-15T19:08:03Z

@bmac - I have kept "semantic" commits so far, but I don't mind squashing if that makes cherry-picking easier...

runspired · 2017-03-15T22:34:24Z

package.json

+    "babel-plugin-transform-es2015-parameters": "^6.23.0",
+    "babel-plugin-transform-es2015-shorthand-properties": "^6.22.0",
+    "babel-plugin-transform-es2015-spread": "^6.22.0",
+    "babel-plugin-transform-es2015-template-literals": "^6.22.0",


is the assumption here that we build and release the globals version ourselves and thus these don't need to be dependencies?

@rwjblue this was the only thing I spotted I had any question on, otherwise +1 LGTM

Yes, the only place we use these directly is in our own build of the addon itself (e.g. from ember-cli-build.js in this repo which is not used by any other consumers).

stefanpenner · 2017-03-15T22:39:52Z

This may not be an issue, normal installation seems to work fine. I think mixing babel 5 and babel6 in dev mode may have just made yarn get emo in ember-data repo itself.

Something seems fishy others should check to see if its not just an issue on my end:

yarn for me isn't deduping the dependencies of all the babel plugins correctly, i'm ending up with nearly 80,000 files in node_modules. Most of them from duplicated core-js and babel-runtimes (all the same size).

https://gist.github.com/stefanpenner/b3e96ba10a9af08dfc5c136a0c53b292

My warm/colde initial builds are also quite slow likely do to hash-for-dep having to traverse all these dependencies.

stefanpenner · 2017-03-15T22:44:29Z

When installing the babel6 branch in a project via

yarn add "emberjs/data#babel-6"

All appears well. Not quite sure in-project I was having issues.

stefanpenner

babel helpers appear to be duplicated:

  function _classCallCheck(instance, Constructor) {
    if (!(instance instanceof Constructor)) {
      throw new TypeError("Cannot call a class as a function");
    }
  }

  var _createClass = function () {
    function defineProperties(target, props) {
      for (var i = 0; i < props.length; i++) {
        var descriptor = props[i];
        descriptor.enumerable = descriptor.enumerable || false;
        descriptor.configurable = true;
        if ("value" in descriptor) descriptor.writable = true;
        Object.defineProperty(target, descriptor.key, descriptor);
      }
    }

    return function (Constructor, protoProps, staticProps) {
      if (protoProps) defineProperties(Constructor.prototype, protoProps);
      if (staticProps) defineProperties(Constructor, staticProps);
      return Constructor;
    };
  }();

class syntax uses verbose (and hard to debug) array of pojo descriptors. Something #4864 addresses in babel 5. Maybe loose mode is missing?

    _createClass(RecordArrayManager, [{
      key: 'recordDidChange',
      value: function recordDidChange(internalModel) {
        // TODO: change name
        // TODO: track that it was also a change
        this.internalModelDidChange(internalModel);
      }
    }, {

do we expect? I thought @rwjblue added some loose mode for this guy to babel 6.

  Object.defineProperty(exports, "__esModule", {
    value: true
  });

rwjblue · 2017-03-15T23:06:19Z

class syntax uses verbose (and hard to debug) array of pojo descriptors. Something #4864 addresses in babel 5. Maybe loose mode is missing?
   _createClass(RecordArrayManager, [{
     key: 'recordDidChange',
     value: function recordDidChange(internalModel) {
       // TODO: change name
       // TODO: track that it was also a change
       this.internalModelDidChange(internalModel);
     }
   }, {

Yes, we are not using loose. I'll fix.

do we expect? I thought @rwjblue added some loose mode for this guy to babel 6.
 Object.defineProperty(exports, "__esModule", {
   value: true
 });

Yes, we still expect __esModule (the thing I removed was interopRequireDefault / interopRequireWildcard), but we could likely use loose mode.

stefanpenner · 2017-03-15T23:15:49Z

do we expect? I thought @rwjblue added some loose mode for this guy to babel 6.

Object.defineProperty(exports, "__esModule", {
value: true
});
Yes, we still expect __esModule (the thing I removed was interopRequireDefault / interopRequireWildcard), but we could likely use loose mode.

👍

rwjblue · 2017-03-21T18:43:42Z

OK, updated and ensured that the asset output is in loose mode (to match the changes from #4864).

The appveyor failure seems related to an SSL issue, but one of the try results had already passed at this point so I think its fine...

@bmac I'd like for this to land into ember-data@2.13.0-beta.2, would you like me to squash the commits down to make that easier to cherry pick?

bmac · 2017-03-21T18:45:39Z

Yes please @rwjblue.

stefanpenner · 2017-03-21T18:51:46Z

just did a quick check, output looks good.

If you can double check build and yarn install times doesn't regress, we should be good!

rwjblue · 2017-03-21T19:14:46Z

Before changes:

yarn time (after a rm -rf node_modules): 27.44s
du -csh node_modules: 369M
ember build time (cold): 32.80s
ember build time (warm): 6.89s
ember serve rebuild time (after touch addon/index.js): 239ms
ember build -prod time (cold): 36.23s
Asset size of dist/globals/ember-data.prod.js: 691.6KB (137.69 KB gzipped)
Asset size of dist/globals/ember-data.min.js: 164.95KB (37.54 KB gzipped)

After changes:

yarn time (after a rm -rf node_modules): 25.03s
du -csh node_modules: 339M
ember build time (cold): 31.76s
ember build time (warm): 9.29s
ember serve rebuild time (after touch addon/index.js): 211ms
ember build -prod time (cold): 36.97s
Asset size of dist/globals/ember-data.prod.js: 562.04KB (111.86 KB gzipped)
Asset size of dist/globals/ember-data.min.js: 164.76KB (37.83 KB gzipped)

It seems basically the same or slightly better in almost all of the things I measured.

bmac · 2017-03-21T19:45:55Z

Thanks @rwjblue. 🎉

workmanw · 2017-04-27T20:07:50Z

Just wanted to give a heads up that this change caused me to get some cryptic errors that took a little bit of time to track down.

The Broccoli Plugin: [broccoli-persistent-filter:Babel] failed with:
ReferenceError: [BABEL] ember-data/-private/adapters.js: Using removed Babel 5 option: /Users/wesley/Code/buttercup/src/buttercup-epcot/apps/buttercup/package.json.optional - Put the specific transforms you want in the `plugins` option

So I tracked this down to the fact that we had the following in our package.json:

"babel": {
  "optional": [
    "es7.decorators"
  ]
}

I guess maybe this is kind of legacy? The right was seems to be to include the following in my ember-cli-build.js.

var app = new EmberApp(defaults, {
  babel: {
    optional: ['es7.decorators']
  },
  // ...
});

I'm not sure how many people have this in their package.json, but if it's a sizable amount it might be worth mentioning in some upgrade notes / blog post.

Just wanted to call it out.

rwjblue · 2017-04-27T20:30:49Z

Interesting!! I thought we disabled the normal .babelrc discovery behavior, but it looks like we don't. I honestly had no clue that in an ember-cli app you could put babel key in your package.json and have it do anything...

workmanw · 2017-04-27T20:53:54Z

I honestly had no clue that in an ember-cli app you could put babel key in your package.json and have it do anything...

That makes two of us. Another engineer on my team added decorators and when I saw it I immediately thought, why is that in the package.json? Especially because we had other babel options in the ember-cli-build.js. But debugging through the code, it looks like babel-core has an OptionManager which recursively walks the directory ancestor tree, parsing any package.json files it finds and merging them into the over all options. 👣 🔫

Either way we had found that package.json trick somewhere. So presumably we're not the only ones using it. ¯\_(ツ)_/¯

EDIT: And if it's not clear why this is a gun foot, it's because the ember-data package was magically inheriting options set in the package.json file from my application while it was running it's transpile.

ReferenceError: [BABEL] ember-data/-private/adapters.js: Using removed Babel 5 option: /Users/wesley/Code/buttercup/src/buttercup-epcot/apps/buttercup/package.json.optional - Put the specific transforms you want in the `plugins` option
  at Logger.error (/Users/wesley/Code/buttercup/src/buttercup-epcot/apps/buttercup/node_modules/ember-data/node_modules/babel-core/lib/transformation/file/logger.js:41:11)
  /Users/wesley/Code/buttercup/src/buttercup-epcot/apps/buttercup/node_modules/ember-data/node_modules/babel-core/lib/transformation/file/options/option-manager.js:220:20

runspired · 2017-04-28T12:05:54Z

cc @stefanpenner I think you had to do some things to get the babel options from the parent for the rollup/babel re-ordering, any chance that changes the semantics of what is applied when?

rwjblue force-pushed the babel-6 branch from 6274197 to b826696 Compare March 15, 2017 02:20

rwjblue force-pushed the babel-6 branch from b826696 to ce8d0f0 Compare March 15, 2017 12:29

rwjblue mentioned this pull request Mar 15, 2017

Loose es6 class #4864

Merged

rwjblue force-pushed the babel-6 branch from 10827a4 to 1956f0d Compare March 15, 2017 19:00

rwjblue changed the title ~~[WIP] Babel 6~~ Babel 6 Mar 15, 2017

rwjblue requested review from stefanpenner and bmac March 15, 2017 19:07

runspired reviewed Mar 15, 2017

View reviewed changes

stefanpenner suggested changes Mar 15, 2017

View reviewed changes

bmac approved these changes Mar 20, 2017

View reviewed changes

rwjblue force-pushed the babel-6 branch from 1956f0d to 2cc71de Compare March 21, 2017 17:27

stefanpenner approved these changes Mar 21, 2017

View reviewed changes

[BETA] Update dependencies to use babel 6.

fffb635

rwjblue force-pushed the babel-6 branch from 2cc71de to fffb635 Compare March 21, 2017 19:15

bmac merged commit 8e600fe into master Mar 21, 2017

rwjblue deleted the babel-6 branch March 21, 2017 20:05

bmac mentioned this pull request Mar 26, 2017

Update to babel@6 infrastructure. #4859

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Babel 6 #4862

Babel 6 #4862

rwjblue commented Mar 14, 2017

rwjblue commented Mar 15, 2017

rwjblue commented Mar 15, 2017

rwjblue commented Mar 15, 2017

runspired commented Mar 15, 2017

rwjblue commented Mar 15, 2017

rwjblue commented Mar 15, 2017

runspired Mar 15, 2017

runspired Mar 15, 2017

rwjblue Mar 15, 2017

stefanpenner commented Mar 15, 2017 •

edited

Loading

stefanpenner commented Mar 15, 2017

stefanpenner left a comment •

edited

Loading

rwjblue commented Mar 15, 2017

stefanpenner commented Mar 15, 2017

rwjblue commented Mar 21, 2017 •

edited

Loading

bmac commented Mar 21, 2017

stefanpenner commented Mar 21, 2017

rwjblue commented Mar 21, 2017

bmac commented Mar 21, 2017

workmanw commented Apr 27, 2017

rwjblue commented Apr 27, 2017

workmanw commented Apr 27, 2017 •

edited

Loading

runspired commented Apr 28, 2017

Babel 6 #4862

Babel 6 #4862

Conversation

rwjblue commented Mar 14, 2017

rwjblue commented Mar 15, 2017

rwjblue commented Mar 15, 2017

rwjblue commented Mar 15, 2017

runspired commented Mar 15, 2017

rwjblue commented Mar 15, 2017

rwjblue commented Mar 15, 2017

runspired Mar 15, 2017

Choose a reason for hiding this comment

runspired Mar 15, 2017

Choose a reason for hiding this comment

rwjblue Mar 15, 2017

Choose a reason for hiding this comment

stefanpenner commented Mar 15, 2017 • edited Loading

stefanpenner commented Mar 15, 2017

stefanpenner left a comment • edited Loading

Choose a reason for hiding this comment

rwjblue commented Mar 15, 2017

stefanpenner commented Mar 15, 2017

rwjblue commented Mar 21, 2017 • edited Loading

bmac commented Mar 21, 2017

stefanpenner commented Mar 21, 2017

rwjblue commented Mar 21, 2017

bmac commented Mar 21, 2017

workmanw commented Apr 27, 2017

rwjblue commented Apr 27, 2017

workmanw commented Apr 27, 2017 • edited Loading

runspired commented Apr 28, 2017

stefanpenner commented Mar 15, 2017 •

edited

Loading

stefanpenner left a comment •

edited

Loading

rwjblue commented Mar 21, 2017 •

edited

Loading

workmanw commented Apr 27, 2017 •

edited

Loading