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

Array.isArray guard drops the type of the #56842

Closed
geon opened this issue Dec 21, 2023 · 8 comments
Closed

Array.isArray guard drops the type of the #56842

geon opened this issue Dec 21, 2023 · 8 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@geon
Copy link

geon commented Dec 21, 2023

πŸ”Ž Search Terms

isArray

πŸ•— Version & Regression Information

Bug occurs in 5.3.3

⏯ Playground Link

https://www.typescriptlang.org/play?#code/JYOwLgpgTgZghgYwgAgLIE8CS5rycgbwChlkBnAewFsIAFKCgB2jHQC5ywpQBzAbiIBfIkQQUQZMMiroAwuMkcM2SLEQoAPsgCCUKHHQAeZTjVIAfMgC8hctToNmUVhwBEACwgAbLxVfJBASJgGGQACl19dAA6YDJIgzCZeQkwAEo0whJkAHoc5DFU6TkFMCUsUzwUADJkOBB0AG0AXWzk0saABmbosHc4+iYWdAARCggyADkKMABRAA84qRtXAHcKKC8AE1c+XPzZ1ei2ktToyhpBp1ZrZDWN7d395EPj4SIYAFcQBDBgcWQACMIGBVJh4noDIYACrmMIwCgUDjQ5BaBJGWFpDgIijIOI6SEYyzEUhQEGfKAgAlRWIQqLwxFpATvELhYGg6Dg9FJU6SDJZUh5ZBXYbIADkfQGjmGYwm0zmi0kYuQW3GZGQIBmyAgiqkANYzHFJlUVTFvTIYQATABma0ATjSJxSki6PUlZBFzlGavlCyWt3umx2QUF+QAyn8fECGABrCAgY6kdpnC4OIZegPrIO7IREIA

πŸ’» Code

interface MyInterface {
  someProperty: string;
}

const myConst: MyInterface | Array<MyInterface> = { someProperty: "hello" };

if (Array.isArray(myConst)) {
  // const myConst: MyInterface & any[]
  myConst[0].thisPropertyDoesNotExist = "world"; // Ew.
  myConst.someProperty = "world"; // Ew.
}

πŸ™ Actual behavior

It compiles.

πŸ™‚ Expected behavior

Accessing thisPropertyDoesNotExist should be an error.

Accessing someProperty when isArray says it's an array should be an error.

Additional information about the issue

No response

@jcalz
Copy link
Contributor

jcalz commented Dec 21, 2023

Duplicate of Related to #33700 probably (edit: @fatcerberus is right)

Also: Issue title drops the object of the

@fatcerberus
Copy link

@jcalz #33700 is definitely related, but see below.

You only lose the array type here because the value is narrowed to MyInterface on assignment, so the type guard has to remanufacture the array type from whole cloth. If you actually have a MyInterface | MyInterface[] then it works as expected:

declare const myConst: MyInterface | Array<MyInterface>;

if (Array.isArray(myConst)) {
  // const myConst: MyInterface[]
  myConst[0].thisPropertyDoesNotExist = "world"; // error
  myConst.someProperty = "world"; // also error
}

betterIsArray() doesn't fare much better in this situation - it narrows the value to MyInterface & MyInterface[], which is clearly not what you want (and allows the assignment to myConst.something to succeed). That's not observable in your playground as-is because for some reason (this might be a separate bug) it gets re-widened to MyInterface | MyInterface[] after the first check.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Dec 21, 2023
@RyanCavanaugh
Copy link
Member

This is correct behavior. On this line you said

const myConst: MyInterface | Array<MyInterface> = { someProperty: "hello" };

TypeScript can see this line of code, and can see that { someProperty: "hello" } is a MyInterface, not an array.

So then when you write

if (Array.isArray(myConst)) {

The only situation where this is true is one where whatever initialized myConst was also an array. What kind of array? Well, we don't know, so it's any[].

If the initializer of myConst isn't statically-determined to be MyInterface, then you see the error on line 9 as expected.

@fatcerberus
Copy link

@RyanCavanaugh Is it a bug that myConst widens to the declared type after the isArray check?

@geon
Copy link
Author

geon commented Dec 21, 2023

What kind of array? Well, we don't know, so it's any[]

But we do know. The array is typed before the if, and elements should have the same type inside the if.

@jcalz
Copy link
Contributor

jcalz commented Dec 21, 2023

No, in operator narrowing has apparently precluded the possibility of Array<MyInterface>.

@fatcerberus
Copy link

But we do know. The array is typed before the if, and elements should have the same type inside the if.

The declared type, yes, but the value is narrowed on assignment (TS knows that you didn't assign an array so removes that possibility from the union) so the type system has essentially "forgotten" about the array type by the time you do the isArray check.

@typescript-bot
Copy link
Collaborator

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

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

5 participants