Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the babel-compiler package use to Babel 7, like the command-line tool. #9440

Merged
merged 7 commits into from Dec 5, 2017

Conversation

benjamn
Copy link
Contributor

@benjamn benjamn commented Dec 5, 2017

Since Meteor 1.6, we've been quietly beta-testing Babel 7 by using it to compile the meteor command-line tool codebase. This dog-fooding was vital when we first introduced Babel compilation in Meteor 1.2, and it later helped us gain confidence in the upgrade from Babel 5 to Babel 6. Now it's time to make the leap to Babel 7!

There are a number of important changes hidden behind the abstraction of meteor-babel, but the most disruptive change that will be visible to application developers is that the babel-runtime package is now called @babel/runtime. Since this package is typically installed in the application node_modules directory, developers will likely need to run meteor npm install @babel/runtime and/or edit their package.json files.

Probably the next most disruptive change is that the Meteor babel-compiler package has jumped a major version, from 6.24.7 to 7.0.0, which may require packages that depend directly on babel-compiler to adjust their version constraints.

There are a few other notable changes, but I think it will be easier if I comment on the commits themselves, below.

@benjamn benjamn added this to the Release 1.6.1 milestone Dec 5, 2017
@benjamn benjamn self-assigned this Dec 5, 2017
@@ -22,7 +22,7 @@ Npm.depends({
});

Package.onUse(function (api) {
api.use('babel-compiler@6.19.4');
api.use('babel-compiler@6.19.4||7.0.0');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GeoffreyBooth Good news: this syntax actually works!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something, at least. It lets me as a package developer write something like api.use('coffeescript@1||2'); (I assume that works, and not just exact 1.12.7||2.0.3?)

It doesn’t really solve the issue of #9244, though, because to take advantage of this syntax you still need to fork a package. There still needs to be some way for end users to override the dependencies a package developer has set, if they haven’t thought to use this syntax or they picked versions that don’t work with other packages the user might want to install. But this is progress.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree!

@@ -62,7 +66,7 @@ class Foo {}
class Bar extends Foo {}
`);

test.isTrue(contains(output, 'helpers/inherits'));
test.isTrue(contains(output, 'helpers/builtin/inherits'));
Copy link
Contributor Author

@benjamn benjamn Dec 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was necessary because of meteor/babel@242538d, which makes @babel/plugin-tranform-runtime generate imports for @babel/runtime/helpers/builtin/whatever instead of @babel/runtime/helpers/whatever. The helpers/builtin/* helpers are important because they do not import any polyfills from core-js, and instead simply assume global APIs like Symbol are natively supported or have been properly polyfilled. Since Meteor takes care to provide appropriate polyfills, especially with #9439, it's worth letting other parts of the Meteor framework import core-js polyfills, rather than leaving that job to a generic tool like Babel.

doc.filename = frame.getFileName();
const fileName = frame.getFileName();
if (fileName && fileName.match(/:tests\.js/)) {
doc.filename = fileName;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why this change became necessary after upgrading Babel, but somehow frame.getFileName() started returning null in some cases, so I decided to make this code more defensive.

This applies only to the command-line tool, for now.
This temporarily reverts back to using @babel/runtime/helpers/* rather
than @babel/runtime/helpers/builtin/*, since some helpers (for example,
`slicedToArray`) were using code patterns that cannot be made to work via
polyfills in older browsers, e.g.

  if (Symbol.iterator in Object(arr)) {...}

to test whether `arr` is iterable.
@benjamn benjamn changed the base branch from web.browser.legacy to devel December 5, 2017 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants