Skip to content
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

Unwind IIFE class patterns #2332

Closed
Andarist opened this issue Sep 25, 2017 · 4 comments · Fixed by #3373
Closed

Unwind IIFE class patterns #2332

Andarist opened this issue Sep 25, 2017 · 4 comments · Fixed by #3373

Comments

@Andarist
Copy link
Contributor

feature request

ES5

Uglify version (uglifyjs -V)
latest (testes on uglify-es too)

JavaScript input

var MyClass = function () {
  function MyClass() {}

  MyClass.prototype.method = function method() {};

  return MyClass;
}();

instance = new MyClass()

The uglifyjs CLI command executed or minify() options used.

uglifyjs --toplevel -mc passes=3,unsafe

JavaScript output

var n=function(){function n(){}return n.prototype.method=function(){},n}();instance=new n;

Requested JavaScript output
Something along this:

function n(){};n.prototype.method=function(){},instance=new n
@alexlamsl
Copy link
Collaborator

@kzc
Copy link
Contributor

kzc commented Sep 25, 2017

It's more complicated than it first appears. Hoisting the function may require renaming it or it may conflict with a symbol in the outer scope. Presently compress and mangle are two independent transform phases.

In the current code base function inlining only works on single statement functions (this includes an AST_Sequence statement - which is useful). If an inline candidate function contains a function definition or a variable declaration then it breaks that assumption since you'll have more than one statement. Also, inlining of functions with multiple statements would may involve introducing a block in the outer scope.

All of this could be done of course, it's just not easy to get all the corner cases right.

@Andarist
Copy link
Contributor Author

Andarist commented Sep 26, 2017

Hoisting the function may require renaming it or it may conflict with a symbol in the outer scope.

I guess it depends how detection of the "class pattern" can be done. Or is it possible to safely do such optimization on things other than "class pattern".

Talking purely about this specific scenario of "class pattern" we do not have to introduce any new identifier to the outer scope. The IIFE could be unwinded to:

var n = function (){};n.prototype.method=function(){},instance=new n

which doesnt introduce any new identifiers than whats outputted today.

Also, inlining of functions with multiple statements would may involve introducing a block in the outer scope.

could u elaborate?

All of this could be done of course, it's just not easy to get all the corner cases right.

Agreed, I would love to help with implementing this, but Im afraid I'm just not able to dive to yet another codebase right now 😢 .

Possible gains from having this feature implemented are probably quite big in es6 classes / TS world (most of the newer projects), so I hope somebody could try to work on this some day.

EDIT:// Im not sure what phases of the compiler are there, but this should ofc should be performed after dropping #__PURE__ calls etc

@kzc
Copy link
Contributor

kzc commented Sep 26, 2017

If a multiline function is inlined/hoisted from within a single statement if body, for example, an AST_BlockStatement would have to be introduced to replace the AST_SimpleStatement. Just an implementation detail one would discover if implementing the feature. You can view the uglify AST with this CLI option: uglifyjs code.js -o ast.

An alternative approach - scan each function to see whether it produces any external side effects beyond the function and mark it internally as equivalent to /*#__PURE__*/ and let the existing compress transforms do the rest. In your down-leveled class example in the top post it only alters a locally introduced function definition (adds a prototype method), but does not have any external side effects beyond the function. Easier said than done, of course.

@alexlamsl alexlamsl added harmony and removed harmony labels Nov 10, 2017
alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Apr 21, 2019
alexlamsl added a commit that referenced this issue Apr 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants