-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
deprecates babel/polyfill in favor of core-js@3 #2031
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ module.exports = function(api) { | |
{ | ||
forceAllTransforms: true, | ||
useBuiltIns: 'entry', | ||
corejs: 3, | ||
modules: false, | ||
exclude: ['transform-typeof-symbol'] | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @somebody32 This suggests that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. great point, looks like I need to extend that too (was not using it in our project, that is why didn't spot the problem): https://github.com/zloirock/core-js/blob/master/docs/2019-03-19-core-js-3-babel-and-a-look-into-the-future.md#babelruntime tldr: try to add |
||
|
@@ -56,7 +57,8 @@ module.exports = function(api) { | |
require('@babel/plugin-transform-runtime').default, | ||
{ | ||
helpers: false, | ||
regenerator: true | ||
regenerator: true, | ||
corejs: 3 | ||
} | ||
], | ||
[ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,7 @@ const { nodeEnv } = require('../env') | |
module.exports = { | ||
test: /\.(js|mjs)$/, | ||
include: /node_modules/, | ||
exclude: /@babel(?:\/|\\{1,2})runtime/, | ||
exclude: /@babel(?:\/|\\{1,2})runtime|core-js/, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. running babel on core-js is causing issues |
||
use: [ | ||
{ | ||
loader: 'babel-loader', | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gauravtiwari do you think
usage
should be the new default? From here:it does look like it's stable now. If I understand it correctly, using
usage
won't require additional import. When I use:in my entry point, I get the following message:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would question that.
useBuiltIns: 'usage'
is not assuming anything about user's code, just expect to changeimport core-js
to required imports based on supported env, so you need to opt-in in into it by adding that import into one of your packs.If we switch that to
usage
, it stops being opt-in and starts automatically adding imports to the code. And from my experience, it still has some edge-cases (because core-js detects relevant import by statically analyzing, so it could not predict what you could have in a runtime), so that creates cases when you still need to add some polyfills by hand.To summarize: I think
useBuiltIns: 'usage'
is a safer choice, in general, that is very easy to change if you know what you're doing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense 👍