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

Nested function with shadowed variable bug #348

Closed
ma2ciek opened this issue May 13, 2019 · 8 comments
Closed

Nested function with shadowed variable bug #348

ma2ciek opened this issue May 13, 2019 · 8 comments
Labels

Comments

@ma2ciek
Copy link

ma2ciek commented May 13, 2019

Bug report or Feature request?

Bug report

Version (complete output of terser -V)

3.17.0

Complete CLI command or minify() options used

{
	"output": {
		"beautify": true
	},
	"compress": {
		"passes": 2
	}
}

terser input

function x(e) {
  function f(t) {
    if (t) {
      const e = t;
      if (e)
        return e;
    }
  }

  return f(e);
}

terser output or error

function x(n) {
    return function(n) {
        if (n) {
            const n = n;
            if (n) return n;
        }
    }();
}

Expected result

function x(n) {
    return function(n) {
        if (n) {
            const n = n;
            if (n) return n;
        }
    }(n);
}
@ma2ciek
Copy link
Author

ma2ciek commented Jul 5, 2019

@fabiosantoscode, could you provide some insights were the bug might be located? Maybe I'll be able to dig a little bit into it and try to fix it.

@fabiosantoscode
Copy link
Collaborator

The standard procedure is to disable the "defaults" option so that you can enable and disable compress options, which act as feature flags for Terser. When you find the feature that's actually guilty of this change, you grep for it in the lib/compress/index.js file. It has a lot of conditions like if (compressor.option("unsafe")) and inside those conditions is the code that runs when that option is enabled.

@ma2ciek
Copy link
Author

ma2ciek commented Jul 22, 2019

The standard procedure is to disable the "defaults" option so that you can enable and disable compress options, which act as feature flags for Terser. When you find the feature that's actually guilty of this change, you grep for it in the lib/compress/index.js file. It has a lot of conditions like if (compressor.option("unsafe")) and inside those conditions is the code that runs when that option is enabled.

But the only thing that is used is passes: 2 - I guess it's not an unsafe option. I haven't set the unsafe: true flag which is mentioned in the docs (https://github.com/terser-js/terser#the-unsafe-compress-option).

@ma2ciek
Copy link
Author

ma2ciek commented Jul 22, 2019

The weird thing is that with the "passes": 1 the produced code is correct and further minifications don't break it.

function x(n) {
    return function(n) {
        if (n) {
            const t = n;
            if (t) return t;
        }
    }(n);
}

@ma2ciek
Copy link
Author

ma2ciek commented Jul 22, 2019

I guess that the error is somewhere in the collapse() function in the compress/index.js file, but it's hard for me to debug this function as its body is full of implicit globals and with almost no comments 😞

@ma2ciek
Copy link
Author

ma2ciek commented Jul 22, 2019

 ./bin/uglifyjs input.js --config-file options.json -o out.js --verbose
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Dropping unused function f [input.js:2,13]
WARN: pass 0: last_count: Infinity, count: 21
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing e [input.js:5,17]
WARN: Collapsing t [input.js:3,13]
WARN: Collapsing t [input.js:4,22]
WARN: pass 1: last_count: 21, count: 19

I guess that the last ones cause the error (WARN: Collapsing t [input.js:3,13], WARN: Collapsing t [input.js:4,22]).

@fabiosantoscode
Copy link
Collaborator

Yes, it's certainly collapse_vars. When I start terser with collapse_vars=false it doesn't do this. Thanks for digging into it!

@fabiosantoscode
Copy link
Collaborator

Managed to close this by being a tad more conservative around variable redefinitions. Could've increased the complexity of the code somewhat but I don't feel it's necessary at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants