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

Edge case: converting foo === null || foo === undefined with foo == null is not safe #408

Closed
jhpratt opened this issue Jul 28, 2019 · 4 comments

Comments

@jhpratt
Copy link

jhpratt commented Jul 28, 2019

Bug report

Version: 3.17.0

Complete CLI command or minify() options used

terser input:

terser -c <<< "const foo = document.all; console.log(foo === null || foo === undefined);"

terser output or error

const foo=document.all;console.log(null==foo);

Expected result

const foo=document.all;console.log(null===foo||null===void 0);

document.all returns an HTMLAllCollection, which per the WHATWG specification, matches == null

The Abstract Equality Comparison algorithm, when given objects implementing the HTMLAllCollection interface, returns true when compared to the undefined and null values. (Comparisons using the Strict Equality Comparison algorithm, and Abstract Equality comparisons to other values such as strings or objects, are unaffected.)

@fabiosantoscode
Copy link
Collaborator

This is why people hate javascript lol

@fabiosantoscode
Copy link
Collaborator

I'm not very happy to change Terser's behaviour here, but someone might hit this. I'm going to add the help wanted label. I'm looking at you @rogeriopvl. This is a good first PR for you.

@fabiosantoscode fabiosantoscode added good first issue This issue is good for contributors who are getting started help wanted labels Aug 2, 2019
@jhpratt
Copy link
Author

jhpratt commented Aug 2, 2019

@fabiosantoscode I agree it probably isn't a good idea to change the behavior, given that basically no one writes code using document.all nowadays, let alone needs to minify it.

This issue was really more of documentation, but also putting it out there if someone actually bothers to track usage throughout a program.

@fabiosantoscode
Copy link
Collaborator

I agree that nobody uses document.all nowadays. I started writing javascript using raw DOM, and even if I stayed that way for several lifetimes, I would have never found a use for document.all. I've changed my mind. I don't think this should warrant an increased file size for everything going through Terser checking for both null and undefined. I don't even think this is worth documenting.

If people want to read about trivia from ancient times they can read something else that isn't a piece of documentation.

Some things are best left forgotten.

Thanks for your report though :)

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

2 participants