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

Rule proposal: no-typeof-undefined #1492

Closed
dimaMachina opened this issue Aug 18, 2021 · 13 comments · Fixed by #1966
Closed

Rule proposal: no-typeof-undefined #1492

dimaMachina opened this issue Aug 18, 2021 · 13 comments · Fixed by #1966

Comments

@dimaMachina
Copy link
Contributor

Less code written

Fail

typeof foo === 'undefined'

Pass

foo === undefined
@fisker
Copy link
Collaborator

fisker commented Aug 18, 2021

How about consistent-undefined-check, allow one of these two? Personally I always use typeof.

@fisker
Copy link
Collaborator

fisker commented Aug 18, 2021

When it's undefined variable, foo === undefiend will throw

foo === undefined
VM227:1 Uncaught ReferenceError: foo is not defined
    at <anonymous>:1:1

@dimaMachina
Copy link
Contributor Author

When it's undefined variable, foo === undefiend will throw

foo === undefined
VM227:1 Uncaught ReferenceError: foo is not defined
    at <anonymous>:1:1

in node REPL, yes, but in real projects there is always enabled no-undef ESLint rule from eslint:recommended config, so this case will be reported

@fisker
Copy link
Collaborator

fisker commented Aug 18, 2021

I don't think no-undef is safe, for example, when you write code for browser, the following won't report, because AbortController is considered "defined" as globals.

if (AbortController !== undefined) {}

But when you run this code in old browsers don't support AbortController , it will throw.

if (typeof AbortController !== 'undefined') {}

This won't.

Of cause, 'AbortController' in globalThis or globalThis.AbortController are safer. But nothing wrong to use typeof.

@dimaMachina
Copy link
Contributor Author

dimaMachina commented Aug 18, 2021

Ok, but we can check if the variable is declared in scope with var, let, or const keyword or variable is a function parameter.
If so, inform the suggestion, otherwise do nothing.

@sindresorhus
Copy link
Owner

This is now accepted.

@sindresorhus
Copy link
Owner

sindresorhus commented Sep 17, 2021

How about consistent-undefined-check, allow one of these two? Personally I always use typeof.

@fisker You are potentially silently hiding bugs by always using typeof though. I think it's generally better to only use it for things that are known to not exist, like when polyfilling. Any other times, IMHO, not using typeof is better.

@fisker
Copy link
Collaborator

fisker commented Sep 20, 2022

I'm going to work on this. And I'm going to add an option to ignore global variables.

@fisker
Copy link
Collaborator

fisker commented Nov 17, 2022

I've sent #1966, we'll check all typeof ANYTHING === 'undefined' by default.

When set {checkGlobalVariables: false}, the rule will ignore variables if it's not defined in file, including unresolved and known globals. typeof foo === 'undefined' and typeof Array === 'undefined' are both valid.

WDYT? @zloirock @papb

Or do you still prefer use typeof?

@zloirock
Copy link
Contributor

@fisker I think that checkGlobalVariables: false should be set by default or this rule should be missed in the recommended.

@fisker
Copy link
Collaborator

fisker commented Nov 17, 2022

I don't really mind, I'll use checkGlobalVariables: false anyway. @sindresorhus your call.

@sindresorhus
Copy link
Owner

I'm fine with having it false by default.

@fisker
Copy link
Collaborator

fisker commented Nov 17, 2022

Changed checkGlobalVariables to false by default, also use suggestions for global variables to be safe.

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