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 to core-js@3 #7646

Merged
merged 112 commits into from Mar 19, 2019
Merged

Update to core-js@3 #7646

merged 112 commits into from Mar 19, 2019

Conversation

zloirock
Copy link
Member

@zloirock zloirock commented Mar 31, 2018

Current state:

@babel/runtime

@babel/polyfill

  • Should be deprecated in favor of separate usage required features from core-js and regenerator-runtime with an informative message.

@babel/preset-env

  • Uses for built-ins data from core-js-compat instead of compat-table since information from compat-table is not enough.
  • useBuilIns now requires direct setting of corejs version option, without it will be used 2 by default and shown deprecation warning.
  • Added support of minor core-js versions for simplify updating in the future.
  • For preventing some order-related problems, polyfills in the both core-js@3 plugins added on post stage in the order of core-js-compat data.
  • Divided plugins and polyfills parts of preset-env, instead of 2 internal plugins for adding polyfills, we have 6: usage and entry versions of plugins for core-js@2, core-js@3, regenerator-runtime.
  • Added support samsung target (for Samsung Internet) since core-js-compat and compat-table now contains mapping for this, fixes Add support for Samsung Internet browser #6602.

useBuilIns: entry with corejs: 3

  • No longer transforms @babel/polyfill.
  • Transforms all possible core-js entry points to import of related modules (based on data from core-js-compat).
  • Since of this, we no longer need shippedProposals / proposals flags with useBuilIns: entry.
  • Removes regenerator-runtime/runtime import where it's not required.

useBuilIns: usage with corejs: 3

  • In addition to shippedProposals, added flag proposals (in corejs: { version: 3, proposals: true } format) for polyfill all proposals from core-js.
  • Fixed list of dependencies in built-in definitions.
  • Improved the way of determination method / built-in name and source of this method.
  • Adds import of required polyfills on MemberExpression, ObjectPattern, in operator.
  • Adds import of required polyfills on access to global object properties.
  • Adds import of all required common iterators on all syntax features which use iterators protocol (for-of, destructuring, spread, yield delegation, etc.).
  • Adds import of promises on syntax features which use promises (async functions/generators, dynamic import, etc.), fixes @babel/preset-env not always polyfilling Promise #9250, Bug Report: babel-preset-env feature-detection error with dynamic import() statements. #7402, etc.

core-js@2 stuff

I didn't want to tough core-js@2-related stuff, however

  • Fixed some serious errors in definitions which breaks Object.getOwnPropertySymbols, Symbol.toStringTag logic, Promise#finally, Array#forEach, etc.
  • Array#flatMap and trim methods moved to stable features as a part of ES2019 and loaded by deprecated @babel/polyfill and @babel/preset-env with corejs: 2 option.

2018.12.30 Explanation of the current situation:

@babel/runtime

Here we have no serious problems.

Breaking changes

@babel/runtime can be updated without any problems by adding @babel/runtime-corejs3 package. But not @babel/polyfill and @babel/preset-env.

Updating @babel/polyfill to core-js@3 will break @babel/preset-env. @babel/preset-env with useBuiltIns transforms import of @babel/polyfill to import of required core-js modules. Most users don't add core-js dependency directly. In case of a minor update, even if @babel/polyfill dependency will be updated with @babel/preset-env, in the top level of node_modules we will have core-js@2, not core-js@3 because a serious part of dependencies depends on packages with core-js@2 in dependencies.

Updating @babel/preset-env will cause the same problem. More other, @babel/preset-env transforms also imports of core-js. If someone has core-js@2 in dependencies, a minor update of @babel/preset-env will break it.

Before babel@7 release, I proposed to deprecate @babel/polyfill in favor of separate inclusion of core-js and regenerator-runtime, but it was not done.

So,

Options for @babel/polyfill:

  • A major bump of @babel/polyfill.
  • Deprecation of @babel/polyfill package in favor recommendation of separate inclusion of required parts of core-js and babel-runtime.

Options for @babel/preset-env:

  • A major bump of @babel/preset-env.
  • Adding an option with a version of core-js like in @babel/plugin-transform-runtime. I'm not a fan of this way, because in this case, we will use a deprecated version of core-js by default, will cause spaghetti in configs and the code base and will cause problems for other mentioned here changes, at least, see polyfilling of proposals section.
  • Deprecation of useBuiltIns option of @babel/preset-env in favor of creation plugin(s) / preset in the scope of core-js for polyfills only.

@babel/preset-env

Some of the improvements already added, some should be added.

  • compat-table is not enough for correct detection of environments where core-js modules required. More info about it you could find here. So I made core-js-compat package with required data which should be used here.
  • Built-ins definitions should be updated for include correct dependencies. Some parts of that were added before babel@7 release, some still not.
  • All syntax features that work with iterators should add common iterators, async functions / generators should add Promise with dependencies (@babel/preset-env not always polyfilling Promise #9250), etc.

Polyfilling of proposals

In the final babel@7 versions, from all polyfilling-related stuff was removed polyfilling of proposals. Seems that it was a mistake.

New babel plugins for proposals will rely on new standard library features. And, with the current approach to polyfilling of proposals, it's too problematic.

I can understand why polyfilling of proposals was removed from the default @babel/polyfill. But in this case, we should have added entry points which include proposals.

Adding polyfills of proposals from core-js when the main part of polyfills loaded from @babel/polyfill is not a good solution because they could be loaded from another instance of core-js and it will cause problems.

Removing proposals from runtime will kill usage of new proposal plugins which depends on new standard library features with the runtime. They should be available in helpers. Even import of required part of core-js without global namespace pollution is rocket science for a serious part of developers. Also, @babel/runtime-corejsx is not @babel/runtime-stable-corejsx-features.

The same for @babel/preset-env, but not so critical.

@babel/preset-env transforms core-js only to import of stable ES features. But the main core-js entry point also contains proposals and someone can expect some of them.

runtime and preset-env inject polyfills based on feature detection, only when they are used, so I don't see any arguments why polyfilling of proposals was removed from those tools.

The shippedProposals option of @babel/preset-env looks like something strange. We haven't any equal in core-js entry points, so I don't see what should be transformed. Implementing in browser definitely is not an indicator that it should be polyfilled, but non-implemented proposals should not. I don't think that it's something that should be supported.

So, by my vision, the best approach is:

  • Deprecate @babel/polyfill in favor recommendation of separate inclusion of required parts of core-js and babel-runtime. If someone needs polyfills of proposals - he will use polyfills of proposals, not - not.
  • Deprecate transform of @babel/polyfill in @babel/preset-env.
  • Add to preset-env with useBuiltIns: entry (or a successor for polyfilling) transforms: core-js to all modules, core-js/es, core-js/proposals and core-js/web to related parts of core-js.
  • Allow runtime and preset-env inject proposals polyfills. Maybe - with an option for that, but option will make it harder.

--------------------

Before the next step, we should decide what should we do with @babel/polyfill, @babel/preset-env breaking changes and polyfilling of proposals. Thoughts?

@zloirock zloirock added the PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release label Mar 31, 2018
packages/babel-plugin-transform-runtime/src/definitions.js Outdated Show resolved Hide resolved
packages/babel-preset-env/src/built-in-definitions.js Outdated Show resolved Hide resolved
packages/babel-preset-env/src/built-in-definitions.js Outdated Show resolved Hide resolved
getOwnPropertyDescriptors: "es7.object.get-own-property-descriptors",
assign: "es.object.assign",
create: "es.object.create",
defineProperty: "es.object.define-property",
Copy link
Member

Choose a reason for hiding this comment

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

I wonder; will, in core-js 3, it be possible to omit including the polyfill for this if your target does not include a browser that needs it?

Right now, in core-js 2, even if you don't include it via preset-env, it'll be overwhelmingly likely that whatever feature you do include the polyfill of will indirectly require the defineProperty polyfill.

It's true that it's safer that way, but it's unfortunate that you can't get significantly smaller code size even when you're specifically being unsafe by using this instead of just loading all of core-js.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, if any polyfill requires this logic - it will be added as an internal module. And it's about almost all fundamental ES5 features.

It's related this issue. You talk about the second option from this comment. It's not hard for implementation, but it will cause too many problems.

For example, Array#fill from the issue. It requires not just Object.defineProperty for adding this polyfill, but, for example, Object.create for adding Array#[@@unscopables] where should be the name of this method. Object.create depends on Object.defineProperties, Object.defineProperties depends on Object.getOwnPropertyNames, etc.

So for working es.array.fill in IE8 we should add all of those dependencies. It's critically hard to determinate this list for manual adding. It's not so critical for babel-preset-env, but the list of internal dependencies could be changed in core-js, for example, after fixing any engine bug and it could be a breaking change for babel-preset-env.

I wanted to propose this issue to discuss before core-js@4. And it's the main problem which can be some time solved by dropping IE8 support in core-js.

packages/babel-preset-env/src/built-in-definitions.js Outdated Show resolved Hide resolved
@existentialism
Copy link
Member

@zloirock this is looking great, mind if I regen and push the test fixtures so we can see the effects?

@zloirock
Copy link
Member Author

zloirock commented Apr 5, 2018

@existentialism makes sense, I wanted to add some more changes and after that work on stabilisation and tests.

@sujiangyin
Copy link

I meet the same error like:
if('get' in Attributes || 'set' in Attributes)throw TypeError('Accessors not supported!');`
hen I load up the page in IE8,and I use webpack3.xx and babel. any ways for me to handle this?thx

@zloirock
Copy link
Member Author

zloirock commented Apr 25, 2018

@suarasaur support of getters and setters can't be polyfilled or transpiled in IE8-, so you can't use it or use transforms which use it. Webpack use it for modules, so, in this case, I recommend to use babel modules-commonjs transform with loose mode option.

@buildsize
Copy link

buildsize bot commented Mar 19, 2019

File name Previous Size New Size Change
babel-preset-env.js 2.05 MB 2.27 MB 227.54 KB (11%)
babel-preset-env.min.js 1.1 MB 1.3 MB 208.28 KB (19%)
babel.js 2.78 MB 2.8 MB 21.57 KB (1%)
babel.min.js 1.55 MB 1.56 MB 9.68 KB (1%)

@nicolo-ribaudo nicolo-ribaudo merged commit 3303b07 into master Mar 19, 2019
@zloirock zloirock deleted the update-to-core-js-3 branch March 19, 2019 20:09
@sodatea sodatea mentioned this pull request Mar 21, 2019
19 tasks
weaverryan added a commit to symfony/webpack-encore that referenced this pull request Mar 29, 2019
… version (Lyrkan)

This PR was merged into the master branch.

Discussion
----------

Set useBuiltIns to false by default and allow to set core-js version

While working on #544 I noticed that the latest version of `@babel/preset-env` displayed a warning (which luckily made [one of the tests](https://travis-ci.org/symfony/webpack-encore/jobs/509655320) fail):

```
WARNING: We noticed you're using the `useBuiltIns` option without declaring a core-js version.
Currently, we assume version 2.x when no version is passed.
Since this default version will likely change in future versions of Babel, we recommend explicitly setting the core-js version you are using via the `corejs` option.

You should also be sure that the version you pass to the `corejs` option matches the version specified in your `package.json`'s `dependencies` section.
If it doesn't, you need to run one of the following commands:

  npm install --save core-js@2    npm install --save core-js@3
  yarn add core-js@2              yarn add core-js@3
```

This is related to the `core-js` 3 update that happened recently (see babel/babel#7646).

As explained in parcel-bundler/parcel#2819 (comment) the issue is that users are normally supposed to add `core-js` in their root `package.json` when setting `useBuiltIns` (which is currently already set by Encore to `entry` by default).

This is a problem for us since it means that:
* we are requiring users to add `core-js` to their project by default (or manually setting `useBuiltIns` to `false`)
* if they do they'll still have a warning because the new `corejs` option of `@babel/preset-env` won't be set (we can't do that because we won't know which version they choose to install).

For now I suggest that we set `useBuiltIns` to `false` by default and add a new option to `Encore.configureBabel()` that allows to set the `corejs` option.

Commits
-------

85896ef Set useBuiltIns to false by default and allow to set corejs version
wincent added a commit to wincent/js that referenced this pull request Mar 31, 2019
Seeing a lot of:

    _Object$defineProperty is not a function

Scanning the release notes:

    https://github.com/babel/babel/releases

This one stands out:

    https://github.com/babel/babel/releases/tag/v7.4.0

Specifically, "Update to core-js@3":

    babel/babel#7646

There are 133 comments which I confess to not having read, and various
offshoots to places like:

    babel/babel#9442

and:

    facebook/regenerator#369

I did find this enormous update doc, but haven't found the answer in
there yet and the Babel docs themselves don't appear to have been
updated:

    https://github.com/zloirock/core-js/blob/master/docs/2019-03-19-core-js-3-babel-and-a-look-into-the-future.md

Although this blog post is briefer and covers a few things:

    https://babeljs.io/blog/2019/03/19/7.4.0

In the end the build works if I get rid of
`@babel/plugin-transform-runtime`, which doesn't seem to play too well
in conjunction with `@babel/preset-env`, at least in this set-up.
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 4, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: polyfill pkg: preset-env PR: Needs Review A pull request awaiting more approvals PR: New Feature 🚀 A type of pull request used for our changelog categories Priority: High
Projects
None yet