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

Add historical note to no-typeof-undefined #1992

Merged
merged 4 commits into from Dec 3, 2022

Conversation

fregante
Copy link
Collaborator

This rule seemingly goes against the "best practice" of always using typeof. I'd like to explain that the practice is no longer "best", but I'm not sure if this is the best way to phrase it.

@fregante fregante requested a review from fisker November 27, 2022 06:46
@fisker
Copy link
Collaborator

fisker commented Nov 27, 2022

I don't know what that mean. I don't think use typeof or not related to the strict mode.

@fregante
Copy link
Collaborator Author

fregante commented Nov 27, 2022

Before strict mode, undefined could be defined and undefined === 1 was entirely possible. For this reason, direct comparison with undefined was frowned upon and is probably still instinctively seen as a bad pattern today.

@fisker
Copy link
Collaborator

fisker commented Nov 27, 2022

In strict mode const undefined = 1 still valid, only globalThis.undefined is readonly. Correct me if I'm wrong.

@fisker
Copy link
Collaborator

fisker commented Nov 28, 2022

I believe the global undefined is readonly because it changed in ES5 https://262.ecma-international.org/5.1/#sec-15.1.1.3 spec.

It has nothing to do with the strict mode.

$ node -p "(function() {'use strict';const undefined = 1; return typeof undefined})()"
number

$ node -p "(function() {const undefined = 1; return typeof undefined})()"
number

@fregante
Copy link
Collaborator Author

fregante commented Nov 28, 2022

Gotcha, so the rule was related to pre-ES5 browsers. I'll update the wording

@@ -9,6 +9,8 @@

Checking if a value is `undefined` by using `typeof value === 'undefined'` is needlessly verbose. It's generally better to compare against `undefined` directly. The only time `typeof` is needed is when a global variable potentially does not exists, in which case, using `globalThis.value === undefined` may be better.

Historical note: Comparing against `undefined` without `typeof` was frowned upon until ES5. This is no longer a problem since all engines currently in use no longer allow reassigning the `undefined` global.
Copy link
Collaborator Author

@fregante fregante Nov 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this more correct? It seems that IE9 did this correctly already according to http://kangax.github.io/compat-table/es5/ (IE8 tests missing)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed broken on IE8 👍

Screen Shot 10

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brokener on IE9 😂

Screen Shot 11

Copy link
Collaborator

@fisker fisker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this, I use typeof(since long time ago, maybe before IE6) not because I'm worried about undefined reassigned, but worried about error thrown on undefined global variable. But I guess it's fine to add this to the doc. Won't block.

@sindresorhus sindresorhus merged commit 84a5816 into main Dec 3, 2022
@sindresorhus sindresorhus deleted the old-no-typeof-undefined branch December 3, 2022 17:40
fisker added a commit that referenced this pull request Jan 9, 2023
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 this pull request may close these issues.

None yet

3 participants