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: collapse_vars is broken since 3.3.23 #3247

Closed
thorn0 opened this issue Sep 4, 2018 · 5 comments · Fixed by #3333
Closed

bug: collapse_vars is broken since 3.3.23 #3247

thorn0 opened this issue Sep 4, 2018 · 5 comments · Fixed by #3333

Comments

@thorn0
Copy link

thorn0 commented Sep 4, 2018

Bug report or feature request?

Bug report

Uglify version (uglifyjs -V)

3.3.23...3.4.9

JavaScript input

// foo.js
function main() {
    var z = { 
        a: 1 
    };
    z = Object.assign({}, z, {
        a: 2
    });
    z.prop = true;
    fn(z, true);
}

function fn(a1) {
    console.log(a1.a === 2 ? 'works as expected' : 'bug');
}

main();

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

> uglifyjs foo.js -c -b -o foo1.js && node foo1
bug

JavaScript output or error produced.

function main() {
    var z = {
        a: 1
    };
    fn(z, (z = Object.assign({}, z, {
        a: 2
    })).prop = !0);
}

function fn(a1) {
    console.log(2 === a1.a ? "works as expected" : "bug");
}

main();

It works fine if I disable collapse_vars:

> uglifyjs foo.js -c collapse_vars=false -b -o foo1.js && node foo1
works as expected

BTW, I don't understand why this specific transformation is triggered by collapse_vars. As per the docs, what collapse_vars does is 'Collapse single-use non-constant variables', however in this case it seems to do something else.

@Maximaximum
Copy link

The old issue (for reference): #3096

@Maximaximum
Copy link

Any progress here? :(

@ingmarh
Copy link

ingmarh commented Sep 24, 2018

Perhaps related: A fix for a collapse_vars regression has been merged in February, but hasn't been included in a new uglify-es (yet). See the commit.

We since switched to using terser (fork of uglify-es), because uglify-es seems to be no longer maintained (and terser includes the above mentioned fix).

@thorn0
Copy link
Author

thorn0 commented Sep 24, 2018

No, @ingmarh. It was a completely different issue.

@ingmarh
Copy link

ingmarh commented Sep 24, 2018

@thorn0 👌 I see. I mistakenly thought it is an uglify-es issue.

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