Skip to content

TS enforces left-hand side of instanceof must be an object type, but it can be any type. #59492

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

Closed
voliva opened this issue Jul 31, 2024 · 5 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@voliva
Copy link

voliva commented Jul 31, 2024

πŸ”Ž Search Terms

"instanceof", "Symbol.hasInstance"

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about instanceof

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.5.4#code/FAYw9gdgzgLgBAUQG4FMIDkCuBbARigJyjgF44BvYOOAbQGUBPPMAGwDoALAQygEloYXCCBQBdAFxwAFEklCGASlIA+ODAYAHFGABmcJKRJkA5BBz4CxuADJr+uAFI4AJkNwADMAC+AbmDAASz0pAGY4AIEhEV1EVAxzQiglSmpwaFYUNhYwAHMpY2wODjgAd24YAH5jBW9-IOkARndwyOFtPWQ0LDxE5Ko4NKgMrNypACIm8OIUOLGar2AgA

πŸ’» Code

const EvenNumbers = {
  [Symbol.hasInstance]: v => typeof v === 'number' && v % 2 == 0
};

if (3 instanceof EvenNumbers) {
  console.log('mhh what?')
}

if (10 instanceof EvenNumbers) {
  console.log("10 is even")
}

πŸ™ Actual behavior

Typescript shows an error on the 3 and the 10:

The left-hand side of an 'instanceof' expression must be of type 'any', an object type or a type parameter. (2358)

πŸ™‚ Expected behavior

It should not show any error, because the code is valid JS

Additional information about the issue

After having support for hasInstance #55052, this actually means that the left-hand side operand doesn't have to be an object anymore. It can perfectly be primitives, as shown by this example (albeit contrieved...)

The spec for relational expressions also allows any kind of relational expressions (which also includes anything that resolves in a primitive) as well https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-relational-operators

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jul 31, 2024
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 5.7.0 milestone Jul 31, 2024
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jul 31, 2024

I would hate to miss out on catching a certain class of errors just because of a technically-okay but extremely uncommon coding patterns.

For example:

function foo(value: string | number) {
  if (value instanceof Number) {
    // oops - never runs!
  }
}

Can you provide more details on why you want this behavior?

@voliva
Copy link
Author

voliva commented Jul 31, 2024

It's true that it's not a common pattern, and probably anyone doing it is abusing the language, but it's part of the spec and all runtimes support it.

I'm really not eager to have this fixed, and I perfectly understand if it's not prioritised or even if it gets closed. I just identified that this is actually valid JS, yet TS will not allow it, giving back an error that's actually incorrect. The left-hand side can be any value.

I would hate to miss out on catching a certain class of errors just because of a technically-okay but extremely uncommon coding patterns.

Fixing this doesn't mean removing the error altogether, which I agree it's very useful. A possible fix would be to allow for any type of value as long as the right-hand side is an object with Symbol.hasInstance. There are already checks in place for this specific case, like the one that makes sure that the value on the left-hand side is assignable to the parameter of the Symbol.hasInstance of the right-hand side.

@RyanCavanaugh
Copy link
Member

I just identified that this is actually valid JS, yet TS will not allow it, giving back an error that's actually incorrect.

This isn't really the bar for what TS has to do. Math.max("hello", "world") is "valid" JS but it's still an error.

A possible fix would be to allow for any type of value as long as the right-hand side is an object with Symbol.hasInstance

But Number does have a Symbol.hasInstance property, which TS correctly models, and it returns sensible results for all inputs, so is modeled as taking any.

If this issue is just a "looking for problems and successfully finding one" thing then I think we should just close rather than spend time investigating possible options.

@MartinJohns
Copy link
Contributor

I just identified that this is actually valid JS, yet TS will not allow it, [..]

I mean, the very point of TypeScript is to prevent "valid" JavaScript.

@voliva
Copy link
Author

voliva commented Aug 1, 2024

This isn't really the bar for what TS has to do. Math.max("hello", "world") is "valid" JS but it's still an error.

There's a big difference though, which is that Math.max in this case will always return NaN. It was not meant to work with strings.

Now, if the spec for Math.max also included strings, or allowed some kind of configuration that would allow it, so that it returns the string furthest down the alphabet ("world") in this case, should TS forbid using strings because it says Math in it and it wasn't originally meant for that?

This is my point. The original example with even numbers is a valid case for hasInstance, if someone prefers the style for whatever reason. I could actually find a "divulgative" article of someone using them to check on primitive types.

Maybe another point of TS doing a good job that's actually a better one is in the following:

function foo() {
  return 3
}

if (foo) {
  // ....
}

Of course it's valid JS, yet TS will show an error. But this is because TS knows that the if is completely redundant, and nothing will change it. It knows how it's completely impossible for foo to be falsy, and being a function it will tell you "hey, did you meant to call it"?

But again, the big difference here is that EvenNumbers actually works, and gives you an expected result. For me, this error would be more suitable for a linter. Or the class of errors that TS reports when e.g. noFallthroughCasesInSwitch or noImplicitOverride is on (which IMO, Typescript compiler shouldn't really have because this should actually be the job of a linter)

I feel like I'm playing devil's advocate here. This is never a pattern I will find acceptable to use other than in playgrounds / experimental esoteric stuff. Feel free to reopen it if you actually see value in it, or someone comes with a non-contrived use case.

@voliva voliva closed this as completed Aug 1, 2024
@DanielRosenwasser DanielRosenwasser added Suggestion An idea for TypeScript Declined The issue was declined as something which matches the TypeScript vision and removed Needs Investigation This issue needs a team member to investigate its status. labels Aug 1, 2024
@DanielRosenwasser DanielRosenwasser removed this from the TypeScript 5.7.0 milestone Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants