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

proposal: change checks for undefined #11363

Closed
4 tasks done
DanielRuf opened this issue Jun 29, 2018 · 4 comments · Fixed by #11368
Closed
4 tasks done

proposal: change checks for undefined #11363

DanielRuf opened this issue Jun 29, 2018 · 4 comments · Fixed by #11368

Comments

@DanielRuf
Copy link
Contributor

Description

Currently we have checks like val === undefined which is not correct in general
Correct would be typeof val === 'undefined' as this could fail on some systems.

Should we change this to use typeof?

Possible Solution

Use typeof check.

Checklist (all required):

  • I have read and follow the CONTRIBUTING.md document.
  • There are no other issues similar to this one.
  • The issue title is descriptive.
  • The template is correctly filled.
@ncoden
Copy link
Contributor

ncoden commented Jun 30, 2018

Nice idea. But just to be sure, could you provide some references or tell on which systems val === undefined would fail ?

@DanielRuf
Copy link
Contributor Author

on which systems val === undefined would fail ?

It failed on a local setup with some Node versions with foundation-sites 6.4.3 (not sure if 6 or 8 or 10, was on Friday at work).

@ncoden
Copy link
Contributor

ncoden commented Jun 30, 2018

https://stackoverflow.com/a/4725697/4317384

For undeclared variables, typeof foo will return the string literal "undefined", whereas the identity check foo === undefined would trigger the error "foo is not defined".

https://stackoverflow.com/a/4726198/4317384

For example, the following is used in IE for parsing XML:

var x = new ActiveXObject("Microsoft.XMLDOM");

To check whether it has a loadXML method safely:

typeof x.loadXML === "undefined"; // Returns false

On the other hand:

x.loadXML === undefined; // Throws an error

UPDATE

Another advantage of the typeof check that I forgot to mention was that it also works with undeclared variables, which the foo === undefined check does not, and in fact throws a ReferenceError. Thanks to @LinusKleen for reminding me. For example:

typeof someUndeclaredVariable; // "undefined"
someUndeclaredVariable === undefined; // throws a ReferenceError

@ncoden
Copy link
Contributor

ncoden commented Jun 30, 2018

Ok LGTM. Label it as a fix if there is a particular issue you can reproduce and resolve. As a style or refactor otherwise (I'm not sure what would be the more appropriate).

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

Successfully merging a pull request may close this issue.

2 participants