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

2.8.8 can break recursive function calls #1575

Closed
pfhayes opened this issue Mar 7, 2017 · 5 comments · Fixed by #1576
Closed

2.8.8 can break recursive function calls #1575

pfhayes opened this issue Mar 7, 2017 · 5 comments · Fixed by #1576

Comments

@pfhayes
Copy link

pfhayes commented Mar 7, 2017

When minifying the following function

module.exports = function outer() {
  var myFunction = function myFunction() {                                                                       
      myFunction();
  };
  myFunction();
};

with the following incantation

./node_modules/.bin/uglifyjs --compress '' file.js -o minified.js

Then the following warning will be emitted: WARN: Dropping unused variable myFunction

This is not correct, because the variable is used inside the recursive call. Furthermore, this will generate invalid code that references myFunction inside the function call and you will get an undefined variable error at runtime

We've confirmed that this broke in the most recent build (2.8.8)

@pfhayes
Copy link
Author

pfhayes commented Mar 7, 2017

Original example was a bit too simple, updated to reflect real minimal example

@kzc
Copy link
Contributor

kzc commented Mar 7, 2017

Temporary 2.8.8 workaround - use uglifyjs -c reduce_vars=false

WARN: Collapsing variable myFunction [-:6,2]
module.exports = function() {
    !function myFunction() {
        myFunction();
    }();
};

@kzc
Copy link
Contributor

kzc commented Mar 8, 2017

Duplicate: #1573

@kzc
Copy link
Contributor

kzc commented Mar 8, 2017

@alexlamsl This appears to fix it:

--- a/lib/compress.js
+++ b/lib/compress.js
@@ -2611,9 +2611,6 @@ merge(Compressor.prototype, {
                 if (compressor.option("unused")
                     && def.references.length == 1
                     && compressor.find_parent(AST_Scope) === def.scope) {
-                    if (!compressor.option("keep_fnames")) {
-                        exp.name = null;
-                    }
                     self.expression = exp;
                 }
             }

@alexlamsl
Copy link
Collaborator

@pfhayes @kzc thanks for spotting this! PR coming right up...

alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Mar 8, 2017
Function expression can be assigned to a variable and be given a name. Ensure function name is the reduced variable before clearing it out.

fixes mishoo#1573
fixes mishoo#1575
alexlamsl added a commit that referenced this issue Mar 8, 2017
Function expression can be assigned to a variable and be given a name. Ensure function name is the reduced variable before clearing it out.

fixes #1573
fixes #1575
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