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

Incorrect transformation of nullish coalescing operator when passing "ecma" option. #1003

Closed
ruipin opened this issue Jun 4, 2021 · 2 comments · Fixed by #1045
Closed

Incorrect transformation of nullish coalescing operator when passing "ecma" option. #1003

ruipin opened this issue Jun 4, 2021 · 2 comments · Fixed by #1045

Comments

@ruipin
Copy link

ruipin commented Jun 4, 2021

Bug report

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

Complete CLI command or minify() options used
terser test.js -c ecma=2020

Note that the issue can be reproduced with any value for ecma, e.g. terser test.js -c ecma=1, terser test.js -c ecma=2000000, terser test.js -c ecma, etc.

The issue does not occur if the ecma parameter is omitted, i.e. terser test.js.

terser input
A test.js file containing:

if(!(a ?? b))
    throw new Error("Some error");

terser output or error

if(a??!b)throw new Error("Some error");

Expected result

if(!(a??b))throw new Error("Some error");

For reference, this is the output when not passing any ecma parameter.

To see why the output is incorrect code, see e.g.

const a = 1;
const b = 2;
console.log(!(a ?? b)); // -> false
console.log(a ?? !b); // -> 1
ruipin added a commit to ruipin/fvtt-lib-wrapper that referenced this issue Jun 4, 2021
Seems that terser doesn't handle the nullish coalescing operator
correctly in a specific corner case:

terser/terser#1003

This caused wrappers on overridden properties to fail.
@oxygene
Copy link

oxygene commented Jul 1, 2021

I'd like to note that this is quite a critical bug. It silently changes the logic of the application.

I have a Vue + webpack application with minification only active on production builds. So for use, this bug only becomes visible when the application hits QA, but the developers don't see it while coding.

@alexander-akait
Copy link

cc @fabiosantoscode I think critical

rricard pushed a commit to rricard/terser that referenced this issue Aug 20, 2021
rricard pushed a commit to rricard/terser that referenced this issue Aug 20, 2021
This PR does 2 things:

- It removes special casing for nullish coalescing (??) in negation
  - This fixes terser#1003
- We also made sure optional chaining (?.) is ok
  - This cements terser#1007 as fixed
fabiosantoscode pushed a commit that referenced this issue Aug 22, 2021
This PR does 2 things:

- It removes special casing for nullish coalescing (??) in negation
  - This fixes #1003
- We also made sure optional chaining (?.) is ok
  - This cements #1007 as fixed
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