-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Improved error messaging for index signature parameters #20726
Conversation
… when any (not every) was a literal
interface O { | ||
[u: IndexableUnion]: A; | ||
~ | ||
!!! error TS1337: An index signature parameter type cannot be a union type. Use '[K in IndexableUnion]' instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This version of the error message should only appear when the parent is a type alias
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given this suggestion #20726 (comment), is it still applicable?
Looks good overall, just one comment |
src/compiler/checker.ts
Outdated
return grammarErrorOnNode(parameter.name, Diagnostics.An_index_signature_parameter_type_cannot_be_a_type_alias_Use_index_Colon_0_instead, typeToString(type)); | ||
} | ||
|
||
if (type.flags & TypeFlags.Union && every((<UnionType>type).types, t => !!(t.flags & TypeFlags.StringLiteral))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
allTypesAssignableToKind
src/compiler/diagnosticMessages.json
Outdated
"category": "Error", | ||
"code": 1336 | ||
}, | ||
"An index signature parameter type cannot be a union type. Use '[K in {0}]' instead.": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: this won't work if there are any other members in the object type. Maybe we should just write "Consider using a mapped object type instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is supposed to be thrown only if all members are string literals. How about changing the wording to A union of string literals cannot be used as an index signature parameter type directly. Consider writing '[K in {0}]: {1}' instead.
?
Sorry, totally misunderstood that.
src/compiler/diagnosticMessages.json
Outdated
@@ -939,6 +939,14 @@ | |||
"category": "Error", | |||
"code": 1335 | |||
}, | |||
"An index signature parameter type cannot be a type alias. Use '[index: {0}]' instead.": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider writing '[{0}: {1}]: {2}' instead.
@DanielRosenwasser @RyanCavanaugh hopefully addressed all the suggestions. |
Any updates on this? |
Fixes #18992
Implements this suggestion specifically: #18992 (comment)