Disable function inlining by default

[jfo84]

This PR contains a:

• new feature

Motivation / Use-Case

After dealing with evil bugs like this one, it seems obvious to me that function inlining behavior in tools like UglifyJS/Terser is silly. Here's another well-documented bug in UglifyJS2.

It's too aggressive, not well-documented, and pointless since engines like V8 will inline function calls anyway. Not to mention that these libaries (Uglify, Terser) are hardly maintained and the entire community still uses them. V8 has built detailed abstractions to handle function inlining in a predictable way; the approach in Terser is more reminiscent of the tales I have heard of the morass of OracleDB.

Breaking Changes

The behavior isn't documented anyway, so this is hardly breaking.

Additional Info

Bugs that disappear when you add logging are soul-destroying monsters that no engineer should ever have to deal with. Please disable this behavior by default.

[jsf-clabot]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

[alexander-akait]

No need rush, just disable this in your build, in near future it will be fixed and released new version with patch.

[alexander-akait]

Also terseris maintain and constantly improving, you can see the git story. Don't forget it is open source and you can help with problem, it is easy to say what all is bad and broken than start to help, thanks

[jfo84]

@evilebottnawi It's cool that the Terser/Uglify people built the functionality, but it's useless and buggy

[jfo84]

@evilebottnawi Contributing to a project where inlining bugs are solved with a test case with a combination of feature flags is just bad engineering.

V8 already optimizes function calls... I don't think I'm missing anything here. If I am, do tell.

[alexander-akait]

@jfo84 can you provide link on documentation or link on code where it is already optimized? Also project use difference developers which use difference ecma version.

[jfo84]

@evilebottnawi Yes. Here's a link to V8's JavaScript inlining heuristic:

https://chromium.googlesource.com/v8/v8/+/refs/heads/lkgr/src/compiler/js-inlining-heuristic.cc

[alexander-akait]

@jfo84 And so? I can't find any benchmark with example or something related. Guys from terser do many works and investigate many situation. Do you have experience with v8 code? Do you know all edge cases in v8? Do you know how better compress size to avoid extra bytes for clients? All I heard was words of shame towards developers of terser. This is not professional. You just ran into a bug and instead of helping to start their shaming.

the approach in Terser is more reminiscent of the tales I have heard of the morass of OracleDB.

Also, you have negative comments about OracleDB developers.

My advice - don't use OSS and no problems.

[jfo84]

@evilebottnawi The engines didn't inline function calls when UglifyJS was originally written. Now they do, but the code is still there.

Sorry if I was a bit abrasive, but putting your head in the sand here benefits no one. Open source software is about helping everyone; it's not about politics. This is an extra layer of complexity that shouldn't exist in 2019.

[alexander-akait]

@jfo84 create issue about this in terser repo, we just wrapper around this tool

[jfo84]

I ran webpack with and without the inlining feature on our production app and it decreased the bundle size by 2 KB, or 0.4% and our vendor bundle by .01 MB, or 0.4%. This doesn't feel like enough benefit for the added surface area for bugs.

[alexander-akait]

@jfo84 for you, other developers can have more than 0.4% and for bundle around 10mb, 0.4% is great improvement.