Include preset modules#11083
Conversation
34fba37 to
301ef63
Compare
|
Ready for review! |
|
This is something we would want to at least move to defaulting to true in Babel 8, correct? And we are just waiting since we don't want to break anyone in case (minor version) |
developit
left a comment
There was a problem hiding this comment.
This is truly a herculean effort @nicolo-ribaudo. The ecosystem is in forever your debt.
Just a couple of questions/nits, nothing super blocking from me.
| { | ||
| "transform-regenerator": [] | ||
| } No newline at end of file | ||
| "transform-async-to-generator": [ |
There was a problem hiding this comment.
Should transform-regenerator be here too?
There was a problem hiding this comment.
Ideally yes. We did it in the past,but had to revert it because regenerator doesn't support async functions with polyfilled promises.
I'd like to revisit it in Babel 8 somehow.
There was a problem hiding this comment.
Okay. At least when bugfixes is enabled though, regenerator will never be included, right?
There was a problem hiding this comment.
It's only included when targeting browsers not supporting generators (we don't use it for async functions).
It's unrelated to bugfixes: it isn't included because of the esmodules option.
| "chrome": "55", | ||
| "edge": "15", | ||
| "firefox": "52", | ||
| "safari": "11", |
There was a problem hiding this comment.
Safari should be 10.1 / 10.3 here I think? Same as async-to-generator. I may have had references to Safari 11 in the plugin since that's what I had chosen to target, but the fix should work in all ES-Modules-supporting versions of Safari (10.1+ desktop / 10.3+ ios)
Edit: I keep thinking this list is "versions that are fixed by X", but it's actually "oldest version where this plugin is not needed", right?
There was a problem hiding this comment.
oldest version where this plugin is not needed
👍
| @@ -0,0 +1,125 @@ | |||
| { | |||
There was a problem hiding this comment.
@nicolo-ribaudo - meta-comment, more on the data format itself: do you think it might be easier to maintain this file if the data only specified which versions were actually affected by the bugfix plugins? For example, the edge function name plugin could just be listed as { "edge": "79" } rather than including the other browser versions which it doesn't have an effect on. A missing browser version could just be taken as "no change".
There was a problem hiding this comment.
This data is generated by the same script which generated the old data, so it's easier to generate all the targets 😅
There was a problem hiding this comment.
it could be interesting to create a rendered version based on the data on our website or something? Or just showcasing this in general in a more readable manner?
There was a problem hiding this comment.
Something like compat-table but with plugin names rather than ECMAScript features?
There was a problem hiding this comment.
Yeah, then you could generate what @developit was saying allow about seeing the diff's or what was actually affected? Maybe we should add a simple readme to explain each json file
| bugfixes: validateBoolOption( | ||
| TopLevelOptions.bugfixes, | ||
| opts.bugfixes, | ||
| false, |
There was a problem hiding this comment.
I recall we'd said this would be enabled by default - I'm guessing this is false until Babel 8?
There was a problem hiding this comment.
Yes 👍
This shouldn't technically be a breaking change, but it's safer to wait (Babel 8 will be released soonish anyway).
hzoo
left a comment
There was a problem hiding this comment.
let's do this, will be a big win for everyone!
92b41b4 to
af6fa3b
Compare
| import transformTypeofSymbol from "@babel/plugin-transform-typeof-symbol"; | ||
| import transformUnicodeRegex from "@babel/plugin-transform-unicode-regex"; | ||
|
|
||
| import bugfixAsyncArrowsInClass from "@babel/preset-modules/lib/plugins/transform-async-arrows-in-class"; |
There was a problem hiding this comment.
nit: We should export theses plugins from @babel/preset-modules instead of relying on internal file structures.
There was a problem hiding this comment.
I would like to just move the plugin files inside @babel/preset-env in the future, if @developit is ok with that.
JLHwung
left a comment
There was a problem hiding this comment.
This is awesome! Can you also update https://babeljs.io/docs/en/babel-preset-env#options ?
90a6848 to
ecf47c9
Compare
This PR introduces a new option for
@babel/preset-env:bugfixes. When it is enabled,Babel uses the plugins from
@babel/preset-modulesto replace "normal" plugins when possible.