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

[BUG] Fatal logic error when modify function argument by function define statement. #369

Closed
tinymins opened this issue Jun 13, 2019 · 6 comments
Labels
bug

Comments

@tinymins
Copy link

@tinymins tinymins commented Jun 13, 2019

Bug report or Feature request?
Bug report.

Version (complete output of terser -V or specific git commit)
terser 4.0.0

Complete CLI command or minify() options used

terser --compress -- test.js

terser input

// test.js
var printTest = function (ret) {
    function ret() {
        console.log('Value after override');
    }
    return ret;
}('Value before override');

printTest();

terser output or error

var printTest=function(ret){return"Value before override"}();printTest();

Expected result

var printTest=function(ret){function ret(){console.log("Value after override")}return ret}();printTest();

or

var printTest=function(ret){return function ret(){console.log("Value after override")}}();printTest();

or

var printTest=function(){console.log("Value after override")};printTest();

Link
https://github.com/tinymins/terser/blob/issue-369/test/compress/issue-369.js
https://github.com/tinymins/terser/blob/issue-369/lib/compress/index.js#L1394

@tinymins tinymins changed the title Fatal logic error when modify function argument by function define sentence. [BUG] Fatal logic error when modify function argument by function define sentence. Jun 13, 2019
@tinymins tinymins changed the title [BUG] Fatal logic error when modify function argument by function define sentence. [BUG] Fatal logic error when modify function argument by function define statement. Jun 13, 2019
@fabiosantoscode fabiosantoscode added the bug label Jun 16, 2019
@fabiosantoscode

This comment has been minimized.

Copy link
Collaborator

@fabiosantoscode fabiosantoscode commented Jun 16, 2019

Thank you very much for your report and the test!

Does this occur even if you specify different variable names for the argument ret and the function ret? If so this is high priority.

@tinymins

This comment has been minimized.

Copy link
Author

@tinymins tinymins commented Jun 16, 2019

@tinymins

This comment has been minimized.

Copy link
Author

@tinymins tinymins commented Jun 16, 2019

I'm not sure if I got a misunderstanding.

The bug is, if you assign the argument by define function statement, unused and collapse_vars config will cause compress error. The compressor think the define statement is an unused code block, and will remove it.

@fabiosantoscode

This comment has been minimized.

Copy link
Collaborator

@fabiosantoscode fabiosantoscode commented Jun 16, 2019

Got it. This is low priority as I don't see too many people creating functions that are the same name as variables which already exist.

@tinymins

This comment has been minimized.

Copy link
Author

@tinymins tinymins commented Jun 17, 2019

Yep, I also think nobody will code like this.
But I triggered this bug by using a 3rd-party dev tools. I have to disable 'collapse_vars' config for using this tool. https://eruda.liriliri.io/
I don't know why they write this kind of code: https://github.com/liriliri/eruda/blob/8027c2435ca78f91927121e18faa78292b27e0bf/src/lib/util.js#L1272 , maybe it was created by webpack module?

@TrySound

This comment has been minimized.

Copy link
Contributor

@TrySound TrySound commented Jun 17, 2019

@fabiosantoscode lodash is one of the most popular libraries. Similar problem with collapse_vars.
#333

fabiosantoscode added a commit that referenced this issue Aug 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.