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
minify helpers-generated.ts
#13837
minify helpers-generated.ts
#13837
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 9be85ca:
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/49856/ |
I am conservative on a mini-minifier used in babel-helpers only for maintenance cost. Can we run |
Agreed. I wasn't too confident about this to begin with, and have since encountered two potential issues -- one is that this cannot recognize a regex literal in the source, so could potentially destroy it; the other is with eating semicolons. I had to put two in a helper with an empty
Looking into it now, and it seems pretty straightforward. With |
I'm having trouble committing changed
Said package Should I just |
Whops sorry, I didn't notice your question. |
1e711ac
to
7410ea6
Compare
helpers-generated.ts
helpers-generated.ts
Rebased on main, and replaced the hacky fake-minify-by-regexp with calling terser. |
// Remove multiple newlines | ||
.replace(/\n{2,}/g, "\n"); | ||
const source = await minify(fileContents, { | ||
compress: false, |
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.
Does the default compressor option break babel helper? May leave a note/todo here.
I think we disable mangle
because babel helper does not ship with a source map.
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.
compress: true
works too, I disabled it only to keep the code as is (compress rewrites some expressions, moves declarations and conditionals into for loop initialization, etc.); if readability is not a concern here, it can be enabled.
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.
As long as debuggability is not affected too much (I want to be able to understand stack traces in bug reports), that's ok.
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.
Yeah the stack traces will be fine since we have disabled mangle
, which renames identifiers to the shortest available.
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.
Thanks! I went ahead and enabled compression so that we can merge this. I had to tweak the custom plugin we use to handle module.exports
because it was causing infinite loops when running on the compressed code.
This obviates the need to carefully handle helper names that cannot be directly used as identifiers, and also very slightly reduces the amount of boilerplate that ends up in babel-standalone.
23983b8
to
a0cac58
Compare
Interesting, haven't noticed that. It's quite likely I disabled compress right after seeing the output, without first running all tests with it. I thought it was purely cosmetic. |
terser
; additionally yarn dedupe upgradedterser
andsource-map-support
I came across this comment in
babel-helpers/scripts/generate-helpers.js
:// TODO: We can minify the helpers in production
and thought it should not be difficult to at least strip unnecessary whitespace, of which there is a lot in the output.
Given that the helpers don't (can't?) contain template strings, which would require properly parsing the source to avoid removing important whitespace, redundant whitespace outside string literals can quite easily be removed with a few regex replacements.Another trick is to export a single object from
helpers-generated.ts
, instead of individual constants, which avoids the need to rename identifier-unfriendly helper liketypeof
to_typeof
and back.edit: Changed to properly minify helpers with terser. 🛩️