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

Include preset modules #11083

Merged
merged 13 commits into from Mar 16, 2020
Merged

Conversation

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Jan 31, 2020

Q                       A
Fixed Issues?
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature? Yes
Tests Added + Pass? Yes
Documentation PR Link babel/website#2193
Any Dependency Changes?
License MIT

This PR introduces a new option for @babel/preset-env: bugfixes. When it is enabled,
Babel uses the plugins from @babel/preset-modules to replace "normal" plugins when possible.

@nicolo-ribaudo nicolo-ribaudo added this to the v7.9.0 milestone Jan 31, 2020
@nicolo-ribaudo nicolo-ribaudo force-pushed the nicolo-ribaudo:include-preset-modules branch from 34fba37 to 301ef63 Feb 15, 2020
@nicolo-ribaudo nicolo-ribaudo marked this pull request as ready for review Feb 15, 2020
@nicolo-ribaudo

This comment has been minimized.

Copy link
Member Author

nicolo-ribaudo commented Feb 15, 2020

Ready for review!

@hzoo

This comment has been minimized.

Copy link
Member

hzoo commented Feb 15, 2020

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)

Copy link
Member

developit left a 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-async-to-generator": [

This comment has been minimized.

Copy link
@developit

developit Feb 17, 2020

Member

Should transform-regenerator be here too?

This comment has been minimized.

Copy link
@nicolo-ribaudo

nicolo-ribaudo Feb 18, 2020

Author Member

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.

This comment has been minimized.

Copy link
@developit

developit Feb 18, 2020

Member

Okay. At least when bugfixes is enabled though, regenerator will never be included, right?

This comment has been minimized.

Copy link
@nicolo-ribaudo

nicolo-ribaudo Feb 18, 2020

Author Member

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",

This comment has been minimized.

Copy link
@developit

developit Feb 17, 2020

Member

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?

This comment has been minimized.

Copy link
@existentialism

existentialism Feb 17, 2020

Member

oldest version where this plugin is not needed

👍

@@ -0,0 +1,125 @@
{

This comment has been minimized.

Copy link
@developit

developit Feb 17, 2020

Member

@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".

This comment has been minimized.

Copy link
@nicolo-ribaudo

nicolo-ribaudo Feb 18, 2020

Author Member

This data is generated by the same script which generated the old data, so it's easier to generate all the targets 😅

This comment has been minimized.

Copy link
@developit

developit Feb 18, 2020

Member

Ah! gotcha.

This comment has been minimized.

Copy link
@hzoo

hzoo Feb 18, 2020

Member

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?

This comment has been minimized.

Copy link
@nicolo-ribaudo

nicolo-ribaudo Feb 18, 2020

Author Member

Something like compat-table but with plugin names rather than ECMAScript features?

This comment has been minimized.

Copy link
@hzoo

hzoo Feb 28, 2020

Member

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,

This comment has been minimized.

Copy link
@developit

developit Feb 17, 2020

Member

I recall we'd said this would be enabled by default - I'm guessing this is false until Babel 8?

This comment has been minimized.

Copy link
@nicolo-ribaudo

nicolo-ribaudo Feb 18, 2020

Author Member

Yes 👍
This shouldn't technically be a breaking change, but it's safer to wait (Babel 8 will be released soonish anyway).

@hzoo
hzoo approved these changes Feb 28, 2020
Copy link
Member

hzoo left a comment

let's do this, will be a big win for everyone!

@existentialism existentialism self-assigned this Feb 28, 2020
@nicolo-ribaudo nicolo-ribaudo force-pushed the nicolo-ribaudo:include-preset-modules branch from 92b41b4 to af6fa3b Mar 1, 2020
@@ -49,7 +49,20 @@ import transformTemplateLiterals from "@babel/plugin-transform-template-literals
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";

This comment has been minimized.

Copy link
@JLHwung

JLHwung Mar 5, 2020

Contributor

nit: We should export theses plugins from @babel/preset-modules instead of relying on internal file structures.

This comment has been minimized.

Copy link
@nicolo-ribaudo

nicolo-ribaudo Mar 5, 2020

Author Member

I would like to just move the plugin files inside @babel/preset-env in the future, if @developit is ok with that.

@JLHwung
JLHwung approved these changes Mar 5, 2020
Copy link
Contributor

JLHwung left a comment

This is awesome! Can you also update https://babeljs.io/docs/en/babel-preset-env#options ?

@nicolo-ribaudo nicolo-ribaudo force-pushed the nicolo-ribaudo:include-preset-modules branch 2 times, most recently from 90a6848 to ecf47c9 Mar 5, 2020
@nicolo-ribaudo nicolo-ribaudo force-pushed the nicolo-ribaudo:include-preset-modules branch from 629bad4 to 6261317 Mar 16, 2020
@nicolo-ribaudo nicolo-ribaudo merged commit a11855e into babel:master Mar 16, 2020
5 of 6 checks passed
5 of 6 checks passed
build (13.x)
Details
test262 Workflow: test262
Details
Travis CI - Pull Request Build Passed
Details
build-standalone Workflow: build-standalone
Details
codecov/project 91.95% (target 90.00%)
Details
e2e Workflow: e2e
Details
@nicolo-ribaudo nicolo-ribaudo deleted the nicolo-ribaudo:include-preset-modules branch Mar 16, 2020
sodatea added a commit to sodatea/vue-cli that referenced this pull request Mar 26, 2020
Per the discussion at babel/babel#11083,
it should not be considered a breaking change

Closes vuejs#5301
Closes vuejs#4848
@sodatea sodatea mentioned this pull request Mar 26, 2020
2 of 9 tasks complete
sodatea added a commit to vuejs/vue-cli that referenced this pull request Mar 31, 2020
Per the discussion at babel/babel#11083,
it should not be considered a breaking change

Closes #5301
Closes #4848
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.