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 interaction between nullish coalescing operator and evaluate option #1007

Closed
phiberger-argo opened this issue Jun 10, 2021 · 4 comments

Comments

@phiberger-argo
Copy link

phiberger-argo commented Jun 10, 2021

Bug report or Feature request?
Bug

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

Complete CLI command or minify() options used
terser -c evaluate=true

terser input
(trying to minify pdf.js...)

class BaseViewer {
  constructor(options) {
    if (!(option.container?.tagName === "DIV")) {
      throw new Error("Invalid `container` and/or `viewer` option.");
    }
  }
}

terser output or error

class BaseViewer{constructor(options){throw new Error("Invalid `container` and/or `viewer` option.")}}

Expected result
The value of "this.container?.tagName clearly cannot be known in advance and the comparison shouldn't be optimized out. The expected code is as produced with evaluate=false:

class BaseViewer{constructor(options){if("DIV"!==option.container?.tagName)throw new Error("Invalid `container` and/or `viewer` option.")}}

Note: if replacing the ?. with "!(option.container && option.container.tagName === "DIV"), all is good.

@phiberger-argo phiberger-argo changed the title Incorrect interraction between nullish coalescing operator and evaluate option Incorrect interaction between nullish coalescing operator and evaluate option Jun 10, 2021
@WofWca
Copy link
Contributor

WofWca commented Jun 29, 2021

5.7.1 doesn't have this bug.

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
@rricard
Copy link
Contributor

rricard commented Aug 20, 2021

This can be closed as #1045 shows it is already behaving correctly

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
@rricard
Copy link
Contributor

rricard commented Aug 22, 2021

@fabiosantoscode I think you can close this one now that there is a regression test for it

@fabiosantoscode
Copy link
Collaborator

Indeed!

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

No branches or pull requests

4 participants