-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Split regeneratorRuntime
into multiple helpers
#17238
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
Split regeneratorRuntime
into multiple helpers
#17238
Conversation
regeneratorRuntime
into multiple helpers
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/59131 |
I still need to figure out the e2e failure, but this is ready for review. |
if (helpers.isInternal(name)) { | ||
throw new Error("Cannot use internal helper " + name); | ||
} | ||
return this.#addHelper(name); |
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.
Since this file will be compiled to Node.js 6, can we turn on the privateFieldsAsProperties
assumption in the build config?
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 using privateFieldsAsSymbols
instead, just to avoid potential conflicts.
1d55e87
to
d1264fa
Compare
Oh the e2e tests are catching an existing bug: compiling private fields with Babel 7.0.0 and a newer version of the plugin does not work, because the plugin assumes that I'll just avoid using |
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 should probably write something to remove the useless files in this directory.
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.
Great work!
_regenerator _tryCatch _regeneratorValues _regeneratorDefineIM _regeneratorDefine
_regeneratorAsync _regeneratorAsyncGen _regeneratorAsyncIterator _awaitAsyncGenerator _OverloadYield _regenerator _tryCatch _regeneratorValues _regeneratorDefineIM _regeneratorDefine
_regeneratorAsyncGen _regeneratorAsyncIterator _awaitAsyncGenerator _OverloadYield _regenerator _tryCatch _regeneratorValues _regeneratorDefineIM _regeneratorDefine
_regeneratorKeys _regenerator _tryCatch _regeneratorValues _regeneratorDefineIM _regeneratorDefine
These are helpers used in all four examples, do you mind if I merge some of it in a subsequent PR to make the output cleaner and mangle the function names better?
Also can you merge it into a new branch? I'd like to take this opportunity to improve context
a bit. :)
Done, in |
Fixes #1, Fixes #2
Ignore the
=== PR STARTS HERE ===
commit, it's just wrong. All commits are relevant :)This PR splits the
regeneratorRuntime
helper into multiple helpers, so that only the necessary ones are loaded. There are now multiple "entry points":regenerator
, always usedregeneratorKeys
, only used when there is ayield
inside afor-in
regeneratorAsyncGen
, only used forasync function* ()
andasync function ()
(when transpiling them through regenerator)regeneratorAsync
, only used forasync function ()
(when transpiling them through regenerator)There are a bunch of new internal helpers, I added a
@internal
marker to not expose them as part of the public@babel/runtime
API.These are the size differences, when using inline helpers and only
@babel/plugin-transform-regenerator
, after giving the output to Terser: