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

chore: remove unnecessary transform-for-of when building babel-parser #10823

Closed
wants to merge 4 commits into from

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Dec 5, 2019

Q                       A
Tests Added + Pass? Yes
License MIT

This PR applies transform-for-of: assumeArray on standalone build, of which the size decrease is expected.

It removes this transform from babel-parser build config since it is supported in Node >= 6.

@JLHwung JLHwung added PR: Internal 🏠 A type of pull request used for our changelog categories pkg: standalone labels Dec 5, 2019
babel.config.js Outdated
@@ -101,14 +103,12 @@ module.exports = function(api) {
["@babel/plugin-proposal-nullish-coalescing-operator", { loose: true }],

convertESM ? "@babel/transform-modules-commonjs" : null,
isStandalone ? ["@babel/transform-for-of", { assumeArray: true }] : null,
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer that we also apply this transform when running tests outside of @babel/standalone, otherwise we risk introducing a for-of with a Map which isn't caught by the tests.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, this probably already breaks part of @babel/core and all the modules plugins.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's an interesting usage of this plugin! I will try it later.

Copy link
Member

@JLHwung
Copy link
Contributor Author

JLHwung commented Dec 12, 2019

@nicolo-ribaudo I revised this PR to be minimal: It only removes unnecessary transform-for-of for babel-parser.

@JLHwung JLHwung changed the title chore: apply transform-for-of only in standalone mode chore: remove unnecessary transform-for-of when building babel-parser Dec 12, 2019
@nicolo-ribaudo
Copy link
Member

How much bigger it the new bundle size? (Since it will now use the "default" transform for for-of)

@JLHwung
Copy link
Contributor Author

JLHwung commented Dec 17, 2019

@nicolo-ribaudo The unminified babel-preset-env.js increases from 3.0MB to 3.1MB. I will work on a fix later.

@nicolo-ribaudo nicolo-ribaudo changed the base branch from master to main October 12, 2020 13:50
@JLHwung JLHwung deleted the use-for-of-in-standalone branch February 25, 2021 14:54
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 28, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 28, 2021
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: parser PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants