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.
This version of the error message should only appear when the parent is a type alias
There was a problem hiding this comment.
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.
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.
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.
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.
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)