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

Calling indexOf and includes in a readonly array results in a "not assignable" error message #54422

Closed
ytxmobile98 opened this issue May 27, 2023 · 11 comments
Labels
Duplicate An existing issue was already created

Comments

@ytxmobile98
Copy link

Bug Report

I sometimes need to define a readonly array with a fixed set of values, and check if an unknown value matches one of those fixed values – for example, checking if an HTTP status code is one of the null body status codes (101, 103, 204, 205, 304) or redirect status codes (301, 302, 303, 307, 308).

But, after I put these status codes in a readonly array, and then call the indexOf method or the includes method on it, the compiler would generate an error message that the argument of the indexOf method or the includes method "is not assignable to parameter of type …" followed by the constants in the array.

🔎 Search Terms

  • indexOf
  • includes
  • readonly
  • array
  • not assignable

🕗 Version & Regression Information

  • typescript NPM package version: 5.0.4

⏯ Playground Link

https://www.typescriptlang.org/play?#code/MYewdgzgLgBAFlKAHAylAhlArhAwiAEwFMIYBeGAbwCgYYwsAbRgIUIE8AuGAbRgEYADPwA0AwQGYxAJkEAWGYICsYifJgBdEbRgAnIgQCW+4FG581omGumrJdgOx2AHJu0BfGOlKhIUANzU1ABmWGCmhuAwhhAASgbGRKYAFNCYOPjE3AwAtgBGRLoAlFQ6+ti6YPCIqBjYeIQkAHT6RiZQTYbhjFjEEKl1GY1Fge5AA

💻 Code

const httpStatusCodes = {
  nullBody: [ 101, 103, 204, 205, 304 ],
  redirect: [ 301, 302, 303, 307, 308 ],
} as const;

function isRedirect(statusCode: number) {
  return httpStatusCodes.redirect.includes(statusCode);
}

🙁 Actual behavior

An error is marked under the statusCode argument in the includes call.

The error message is

Argument of type 'number' is not assignable to parameter of type '301 | 302 | 303 | 307 | 308'. ts(2345)

If I use indexOf instead, the error is the same.

🙂 Expected behavior

The code should compile normally without error, because includes and indexOf are both used to check if a user-supplied argument matches one of the values in the array. In pure JavaScript this operation is totally valid for variables of any types.

@jcalz
Copy link
Contributor

jcalz commented May 27, 2023

Duplicate of #14520 by way of #26255 by way of #42351

@fatcerberus
Copy link

fatcerberus commented May 27, 2023

To summarize past discussion around this: Ideally it should be an error to pass e.g. a string here, because that’s likely to be a mistake, so that’s why it doesn’t just take any. However there’s currently no mechanism to allow “any type that overlaps with the type of the array” (that’s what #14520 is for); the best TS can do is thus “value must be assignable to the type of the array”

@fatcerberus
Copy link

btw it’s not actually the readonly typing that’s the issue here but rather the fact that as const also infers literal types.

@guillaumebrunerie
Copy link

Would it make sense to have overloads for the special case "Array.includes on an array of literal numbers should accept an arbitrary number" (and the same one with "string" instead of "number"). I feel like the vast majority of complaints about Array.includes is when the array contains only literal strings or only literal numbers.

@fatcerberus
Copy link

fatcerberus commented May 27, 2023

TS doesn’t have anything resembling template specialization for generics so I don’t think that’s possible.

It also doesn’t solve the equally common case where someone has e.g. string[] and wants to check that array for a value typed string | number.

@whzx5byb
Copy link

@guillaumebrunerie

Try this workaround:

interface Array<T> {
    includes</* U super T */ U>(this: T extends U ? this : never, searchElement: U, fromIndex?: number | undefined): boolean;
}

interface ReadonlyArray<T> {
    includes</* U super T */ U>(this: T extends U ? this : never, searchElement: U, fromIndex?: number | undefined): boolean;
}

@ytxmobile98
Copy link
Author

ytxmobile98 commented May 27, 2023

@fatcerberus

To summarize past discussion around this: Ideally it should be an error to pass e.g. a string here, because that’s likely to be a mistake, so that’s why it doesn’t just take any. However there’s currently no mechanism to allow “any type that overlaps with the type of the array” (that’s what #14520 is for); the best TS can do is thus “value must be assignable to the type of the array”

I don't think this is a real issue within a scope of readonly array with a set of compile-time fixed values.

Consider the use case as you have mentioned: I have a string variable x and passes it into an array arr with a fixed set of integers.

  • If the outer logic is "if the arr contains x, then do something with x":
    • This is meaningless. An array full of integers can never contain a string. The "do something" action will never be executed.
  • If the outer logic is "if the arr does not contain the number x, then do something with x":
    • Now x is supposed to be a number, but is actually a string. If you call methods of x that are only present on number variables, or if you pass x to a different function which expects a number, such mistakes will still be caught in the corresponding contexts.

So this type mismatch is not surely an issue that Array.prototype.includes or Array.prototype.indexOf should take a notice of. The mismatch itself could indicate a problem at a larger scope within the software project, which could mostly be caught in higher-level components in the codebase.

@fatcerberus
Copy link

So this type mismatch is not surely an issue that Array.prototype.includes or Array.prototype.indexOf should take a notice of. The mismatch itself could indicate a problem at a larger scope within the software project, which could mostly be caught in higher-level components in the codebase.

You’re right, and the “higher-level component” that’s taking notice of it is TypeScript—which is specifically designed to catch potential mistakes in your code at compile time based on the types used. Unfortunately that sometimes means this particular method is too narrow, due to inherent constraints in the type system.

Anyway, I don’t make the rules, I’m only telling you how this discussion has gone in the past based on responses from team members. Don’t shoot the messenger.

@fatcerberus
Copy link

See for example #53904 (comment)

@fatcerberus
Copy link

I don't think this is a real issue within a scope of readonly array with a set of compile-time fixed values.

Just wanted to address this - like I said above, TS doesn’t have template specialization, so this isn’t a distinction that can be made. The fixed readonly arrays necessarily have to share the same include typing with the mutable ones.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label May 30, 2023
@typescript-bot
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

7 participants