-
-
Notifications
You must be signed in to change notification settings - Fork 390
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
New option to wrap function arguments #427
Comments
Could you please provide an example of what you want to happen exactly? I'm not sure I follow. |
@fabiosantoscode the Terser OptimizeJS has an additional optimization that Terser currently doesn't: it will wrap function arguments, so this:
Becomes this:
The assumption here is that most function arguments will be executed immediately in that function call, which seems to be a pattern commonly used in Node-style errbacks, Promise chains, and UMD/Browserify/Webpack module declarations. The change takes just a few lines of code: OptimizeJS is now unmaintained and it has some bugs, like in sourcemap generation; I think adding that option would allow Terser to replace that plugin functionality entirely. |
This is very interesting. Thanks for bringing this up. I'm a bit skeptical that functions passed to other functions are invoked immediately though, node-style callbacks and promises are not called immediately. Although most other functions are. I'm actually happy to remove the options that turn It might be nice to have @nolanlawson's input here, IIUC he used to be behind optimize. |
I was certain that wrapping-function heuristic had a benefit, but I certainly didn't know how much would it be, so I decided to adapt a benchmark from OptimizeJS. I ran Terser with the following options:
Firefox 48 results:
Firefox 68 results (for some reason the resolution is very poor):
Chrome 76 results:
The |
Interesting results. Have you disabled |
|
The results in Chrome 76:
In FF 48:
|
Looks like |
Please send a PR with your commits :) it would be cool to enable this option by default actually, so if it doesn't break too many tests please do do that. That would mean a lot of websites could immediately benefit from this change. Thanks a lot for bringing this up! |
@fabiosantoscode surprisingly, no tests break when I made About enabling it by default, I see two potential problems:
So I see this change as somewhat controversial. I think the safe option would be to add a section on the README that goes more into detail about |
Created PR: #433 |
Feature request
OptimizeJS
is deprecated, and thewrap_iife
option in Terser is a very valid replacement. However,OptimizeJS
will also wrap functions that are passed as arguments under the assumption that they're likely to be called immediately. It would be nice to have an additional option in order to wrap those functions (i.e.wrap_func_args
).Happy to provide a PR.
The text was updated successfully, but these errors were encountered: