-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
πβ¨ Upgrade to Babel 7 #18574
πβ¨ Upgrade to Babel 7 #18574
Conversation
@erwinmombay @jridgewell @kristoferbaxter @choumx I'm sending this out for preliminary high level comments while I continue to work on it. As things stand right now, |
This is now ready for review. |
type: 'lalr', | ||
debug: false, | ||
moduleType: 'js', | ||
'token-stack': true, |
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'm very worried about performance regressions that'll be caused by this. The token-stack appears to create a lot of arrays and array mutations.
Is this still needed? If the _token_stack
label still needs to be stripped, can we do it as a post-process step instead of changing the JISON algorithm?
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.
Compiling with babel
7 fails without this. I found this fix in the jison
codebase. Do you have a suggestion for a better way to strip this label?
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.
@jridgewell WDYT?
extensions/amp-ad-network-doubleclick-impl/0.1/test/test-doubleclick-rtc.js
Show resolved
Hide resolved
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.
One item I believe you should address here, left a few comments.
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.
All comments addressed. PTAL.
extensions/amp-ad-network-doubleclick-impl/0.1/test/test-doubleclick-rtc.js
Show resolved
Hide resolved
@kristoferbaxter I hesitate to make extensive changes to |
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.
Left a few comments on the compile-expr
file.
Happy to have a followup PR for |
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.
@jridgewell Comments addressed. PTAL.
It's been a year since we upgraded to Babel 6 in #9775. It's time to upgrade to Babel 7.
Things this PR does:
babel-core
(version 6) with@babel/core
(version 7, from the newbabel
monorepo)babel
packages with their latest monorepo equivalentsbabelify
from version 8 (compatible withbabel
6) to version 10 (compatible withbabel
7).babelrc
tobabel.config.js
babelify
settings across the codebase intobabel.config.js
(Fixes Centralize browserify configΒ #18582)build-system/tasks/compile-{access|bind|css}-expr.js
into a single filecompile-expr.js
(and removes the_token-stack:
label from the generatedjison
parser in order to satisfybabel
7)global: true
setting forbabelify
added in Upgrade Closure Compiler to v20171112Β #18552 (so thatnode_modules
dependencies are also compiled) and replaces it with a less risky method that does ababelify
transform for only thosenode_modules
packages that we ship with the runtimebabel-plugin-transform-remove-strict-mode
package that was added during thebabel
6 upgrade in upgrade to babel 6Β #9775 to preventuse strict
errors, and enables strict mode during developmentstrict mode
is enabled and enforced during development and unit testingWith this, all traces of
babel
6 should be gone from our code-base, andbabel
7 should be the only means of compiling unminified code.Fixes #18654
Fixes #18582
Closes #18199