Skip to content

Should warn against invalid typeof expression #29200

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

Open
zheeeng opened this issue Dec 30, 2018 · 12 comments
Open

Should warn against invalid typeof expression #29200

zheeeng opened this issue Dec 30, 2018 · 12 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@zheeeng
Copy link

zheeeng commented Dec 30, 2018

TypeScript Version: 3.2.2

Search Terms: typeof undefined

Code

// This condition will always return 'false' since the types '"string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"' and '42' have no overlap.
typeof 42 === 42

typeof 42 === undefined

typeof 42 === null

Expected behavior:

typeof 42 === undefined

typeof 42 === null

Should warn as what typeof 42 === 42 warns.

Actual behavior:

No warnings

Playground Link:
http://www.typescriptlang.org/play/index.html#src=%2F%2F%20This%20condition%20will%20always%20return%20'false'%20since%20the%20types%20'%22string%22%20%7C%20%22number%22%20%7C%20%22bigint%22%20%7C%20%22boolean%22%20%7C%20%22symbol%22%20%7C%20%22undefined%22%20%7C%20%22object%22%20%7C%20%22function%22'%20and%20'42'%20have%20no%20overlap.%0Atypeof%2042%20%3D%3D%3D%2042%0A%0Atypeof%2042%20%3D%3D%3D%20undefined%0A%0Atypeof%2042%20%3D%3D%3D%20null

Related Issues:

@jack-williams
Copy link
Collaborator

I think fixing this case without breaking many common patterns would require special casing the check (with typeof and null), rather than changing the comparability relation. At that point I wonder if this would be better addressed with a lint rule instead.

@fatcerberus
Copy link

fatcerberus commented Dec 30, 2018

What common pattern compares the result of typeof against undefined or null? It can never be anything other than a handful of strings so such a comparison seems like it would always indicate a logic error.

@ajafff
Copy link
Contributor

ajafff commented Dec 30, 2018

@fatcerberus TypeScript always allows comparing with null or undefined (because reasons). There's nothing special about comparing the result of a typeof expression. That's what @jack-williams means by:

would require special casing the check (with typeof and null), rather than changing the comparability relation.

IMO comparing something with null or undefined shouldn't be treated different from any other value. If you need to, your type declarations are probably wrong. But that's already discussed in another issue and was declined for compatibility reasons. Therefore I've written a lint rule, see below.

At that point I wonder if this would be better addressed with a lint rule instead.

This check already exists as lint rule.

@jack-williams
Copy link
Collaborator

jack-williams commented Dec 30, 2018

EDIT: Thanks @ajafff, that’s a more detailed response than mine!

The common patterns are not those including typeof, but of the form M === undefined for some other arbitrary expression M.

To fix this issue the options seem to be changing the comparable relation for all M, but I’m not sure how desirable that is. Alternatively, special-casing certain forms of M, such as typeof, which I think might be better suited to a lint rule. See above comment for details!

@fatcerberus
Copy link

Oh, that makes sense. I guess I assumed typeof was already special-cased, since it acts as a type guard. But I guess that’s controlled by a different part of the compiler than this error.

@DanielRosenwasser DanielRosenwasser added the Working as Intended The behavior described is the intended behavior; this is not a bug label Dec 31, 2018
@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@rkirov
Copy link
Contributor

rkirov commented Jan 16, 2019

I recently ran into a similar issue, where it would have been nice if 'x === undefined' erred when x's type doesn't include undefined.

Can someone elaborate on:

@fatcerberus TypeScript always allows comparing with null or undefined (because reasons).

@fatcerberus
Copy link

fatcerberus commented Jan 16, 2019

I assume there are 2 main reasons:

  1. It used to be that null and undefined were always in the domain of every type, before strictNullChecks was implemented.
  2. When writing library code, you usually want to put live guards at API boundaries, in case the caller is pure JavaScript and therefore doesn't benefit from the type system to catch null-ish values being passed in (it's much more likely that the caller accidentally passes undefined or null rather than some arbitrary wrong type). If x === undefined was a type error then you'd nearly always have to cast for such checks. That would get annoying fast.

@Jack-Works
Copy link
Contributor

it's definitely an error for typeof x === undefined, I wrote this today, and cause me 30mins to debug. I don't understand why typescript doesn't warn me it's a so obvious bug in my code.

@Jack-Works
Copy link
Contributor

ah please fix this, I shot on my foot again today

@bradzacher
Copy link
Contributor

@DanielRosenwasser you marked this as working as intended, but I wonder if it's worth reopening this and limiting the scope of the enhancement.

We know that typeof will always return a string, right? So is it possible for TS to error on comparisons against typeof results in the following cases:

  • The literals null or undefined (not variables typed with them, just the literals themselves)
  • number or boolean literals

I feel like just these syntactic checks would help immensely in ensuring correctness without causing any unexpected errors in code.

For example I know it's a common mistake to forget the quotes and do

typeof foo === undefined

@RyanCavanaugh RyanCavanaugh reopened this Oct 26, 2022
@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Oct 26, 2022
@Andarist
Copy link
Contributor

Andarist commented Jul 5, 2023

There is a major problem with index signatures outside of noUncheckedIndexedAccess with this:

function fn(config: Record<string, number>) {
  if (typeof config.foo === undefined) {} 
  const num = config.foo
  if (typeof num === undefined) {} 
}

Both should work but TS has no way to track the origin of this number type used with the typeof

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests