-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Description
type MappedType = {
[Key in Type]: Value;
};Mapped types are a self-contained thing in TS; they can declare exactly one signature - the index signature defining the mapping. They can declare no other properties.
As such we emit a special AST node for them - TSMappedType
typescript-eslint/packages/ast-spec/src/type/TSMappedType/spec.ts
Lines 6 to 13 in a9cb860
| export interface TSMappedType extends BaseNode { | |
| type: AST_NODE_TYPES.TSMappedType; | |
| typeParameter: TSTypeParameter; | |
| readonly?: boolean | '-' | '+'; | |
| optional?: boolean | '-' | '+'; | |
| typeAnnotation?: TypeNode; | |
| nameType: TypeNode | null; | |
| } |
The problem with this AST is that we emit a TSTypeParameter for the indexer.
[ Key in Type ] : Value;
^^^^^^^^^^^ TSTypeParameter
^^^ .name = Identifier
^^^^ .constraint = TypeNodeWe emit this because the underlying TS AST uses a ts.TypeParameter node for this.
This is a pretty weird AST considering the following things:
TSTypeParametersupports things not syntactically[1] valid in a mapped type indexer (eg variance sigils (in,out) and default valueT = Default)..constraintis marked as optional onTSTypeParameter, even though it is a syntactically[1] required.- in all other usages of
TSTypeParameterthe existence of the.constraintproperty implies theextendskeyword:Name extends Constraint. However just for theTSMappedTypethe.constraintinstead implies theinkeyword:Name in Constraint.
Note that in this location it is syntactically[1] invalid to use[Key extends Value]. - it's possible to define a key remapping in a mapped type. This remapping node is defined as a sibling of the
TSTypeParameter, but is part of theTSMappedTypenode:type MappedType = { [ Key in Type as Remapping ]: Value; ^^^^^^^^^^^ TSTypeParameter ^^^^^^^^^ TSMappedType.nameType };
[1] Note that by syntactically valid I mean fatal parser error from TS - not just a semantic error!
It's pretty clear to me that our AST shape here is wrong and should be changed.
I propose that we switch to the following AST shape:
export interface TSMappedType extends BaseNode {
type: AST_NODE_TYPES.TSMappedType;
- typeParameter: TSTypeParameter;
+ key: Identifier;
+ constraint: TypeNode;
readonly?: boolean | '-' | '+';
optional?: boolean | '-' | '+';
typeAnnotation?: TypeNode;
nameType: TypeNode | null;
}Migration Plan
With this sort of breaking AST change it can be difficult to roll out to the ecosystem without breaking the plugin ecosystem that may rely on the existing shape.
I propose the following migration plan:
- implement the above additions (
keyandconstraint) to the AST. - use JSDoc to mark
typeParameteras@deprecated. - (optional) add test-time logging to warn consumers that they're using a deprecated, to-be-removed property.
- broadcast this change (perhaps via a blog post for easy sharing?).
- breaking change remove
typeParameterin a major release.