We read every piece of feedback, and take your input very seriously.
To see all available qualifiers, see our documentation.
There was an error while loading. Please reload this page.
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
This PR fixes multiple issues related to indexed access types applied to mapped types. The fixes enable most of the problems outlined in #30581 to be solved using an approach that involves "distributive object types" created using indexed access types applied to mapped types.
First, a summary of the issue we're trying to solve:
type UnionRecord = | { kind: "n", v: number, f: (v: number) => void } | { kind: "s", v: string, f: (v: string) => void } | { kind: "b", v: boolean, f: (v: boolean) => void }; function processRecord(rec: UnionRecord) { rec.f(rec.v); / Error, 'string | number | boolean' not assignable to 'never' }
The UnionRecord type is a union of records with two correlated properties, v and f, where the type of v is always the same as the type of f's parameter. In the processRecord function we're attempting to take advantage of the correlation by calling rec.f(rec.v) for some record. By casual inspection this seems perfectly safe, but the type checker doesn't see it that way. From its viewpoint, the type of rec.v is string | number | boolean and the type of rec.f is (v: never) => void, the never originating from the intersection string & number & boolean. The type is basically trying to check if a v from some record can be passed as an argument to an f from some record--but not necessarily the same record.
UnionRecord
v
f
processRecord
rec.f(rec.v)
rec.v
string | number | boolean
rec.f
(v: never) => void
never
string & number & boolean
In the following I will outline an approach to writing types for this kind of pattern. Key to the approach is that the union types in question contain a discriminant property with a type that can itself serve as a property name (e.g. a string literal type or a unique symbol type). Note that the examples below only compile successfully with the fixes in this PR.
The following formulation of the UnionRecord type encodes the correspondence between kinds and types and the correlated properties:
type RecordMap = { n: number, s: string, b: boolean }; type RecordType<K extends keyof RecordMap> = { kind: K, v: RecordMap[K], f: (v: RecordMap[K]) => void }; type UnionRecord = RecordType<'n'> | RecordType<'s'> | RecordType<'b'>;
The manual step of creating a union of RecordType<K> for each key in RecordMap can instead be automated:
RecordType<K>
RecordMap
type UnionRecord = { [P in keyof RecordMap]: RecordType<P> }[keyof RecordMap];
The pattern of applying an indexed access type to a mapped type is effectively a device for distributing a type (in this case RecordType<P>) over a union (in this case keyof RecordMap). We can even go one step further and allow a UnionRecord to be created for any arbitrary subset of keys:
RecordType<P>
keyof RecordMap
type UnionRecord<K extends keyof RecordMap = keyof RecordMap> = { [P in K]: RecordType<P> }[K];
We can then merge RecordType<K> into UnionRecord<K>, which removes the possibility of creating non-distributed records. This leaves us with the final formulation:
UnionRecord<K>
type RecordMap = { n: number, s: string, b: boolean }; type UnionRecord<K extends keyof RecordMap = keyof RecordMap> = { [P in K]: { kind: P, v: RecordMap[P], f: (v: RecordMap[P]) => void }}[K];
And, using this formulation, we can now write types for the processRecord function that reflect the correlation and work for singletons as well as unions:
function processRecord<K extends keyof RecordMap>(rec: UnionRecord<K>) { rec.f(rec.v); / Ok } declare const r1: UnionRecord<'n'>; / { kind: 'n', v: number, f: (v: number) => void } declare const r2: UnionRecord; / { kind: 'n', ... } | { kind: 's', ... } | { kind: 'b', ... } processRecord(r1); processRecord(r2); processRecord({ kind: 'n', v: 42, f: v => v.toExponential() });
And, because everything is expressed in terms of the mapping in RecordMap, new kinds and data types can be added in a single place, nicely conforming to the DRY principle.
Fixes #30581.
Sorry, something went wrong.
Fix multiple issues with indexed access types applied to mapped types
d42e4c2
@typescript-bot perf test faster @typescript-bot test this @typescript-bot user test inline @typescript-bot run dt
Heya here.
Update: The results are in!
@typescript-bot user test this inline
@ahejlsberg The results of the perf run you requested are in!
Comparison Report - main..47109
Download Benchmark
@ahejlsberg Great news! no new errors were found between main..refs/pull/47109/merge
I'm kind of surprised at how much of this already works in TS4.5 without these fixes. Both processRecord(r1); and processRecord(r2); are fine. Sure, the K parameter isn't inferred, but it falls back to keyof RecordMap which is fine for a distributive object type. Only
processRecord(r1);
processRecord(r2);
K
processRecord({ kind: 'n', v: 42, f: v => v.toExponential() }); / error
fails outright, and that's just because of unfortunate contextual typing of v as RecordMap[keyof RecordMap] . In current TS I'd be inclined to fix that with either of these:
RecordMap[keyof RecordMap]
processRecord<'n'>({ kind: 'n', v: 42, f: v => v.toExponential() }); / okay processRecord({ kind: 'n', v: 42, f: (v: number) => v.toExponential() }); / okay
Am I right in thinking the current fix is mostly helping inference for K succeed here so that these are not necessary? I'm definitely interested in playing around with this. Thanks for working on it!!
Playground link
Add tests
f561faf
I'm kind of surprised at how much of this already works in TS4.5 without these fixes.
Yes, the nice thing is that this approach isn't really a new feature. The core capabilities were all there, they just didn't work quite right in a few cases. This in contrast to a whole new feature like existential types, which would be a very complex undertaking.
Am I right in thinking the current fix is mostly helping inference for K succeed here so that these are not necessary?
The PR has three fixes in it. Two of them relate to inference, specifically that we would fail to infer from object literals that contain contextually typed arrow functions.
The third fix relates to indexing non-generic mapped types with generic index types. For example:
type RecordFuncMap = { [P in keyof RecordMap]: (x: RecordMap[P]) => void }; const recordFuncs: RecordFuncMap = { n: n => n.toFixed(2), s: s => s.length, b: b => b ? 1 : 0, } function callRecordFunc<K extends keyof RecordMap>(rec: UnionRecord<K>) { const fn = recordFuncs[rec.kind]; / RecordFuncMap[K] fn(rec.v); / Would fail, but works with this PR }
The call in callRecordFunc failed to compile because we transformed type RecordFuncMap[K] into RecordFuncMap[typeof RecordMap] by constraint substitution when obtaining what we call the apparent type of fn. With the fix in the PR, we instead transform RecordFuncMap[K] into (x: RecordMap[K]) => void.
callRecordFunc
RecordFuncMap[K]
RecordFuncMap[typeof RecordMap]
fn
(x: RecordMap[K]) => void
There are more examples of this pattern in the tests I just added to the PR. See here.
Performance is unaffected by this PR and all tests look clean. I think this is ready to merge.
3d3825e
lock typescript version
1482d5d
Anders Hejlsberg fixed issues of version 4.5.4 in microsoft/TypeScript#47109 so I revert typescript version until it will be released Hejlsberg saved my day ❤️
I have run into an oddity that might be related to this PR. Consider the following code:
enum MyEvent { One, Two } type Callback<T> = (arg: T) => void; interface CallbackTypes { [MyEvent.One]: number [MyEvent.Two]: string } type Callbacks = { [E in MyEvent]: Callback<CallbackTypes[E]>[] }; class CallbackContainer { callbacks: Callbacks = { [MyEvent.One]: [], [MyEvent.Two]: [] }; constructor() { } setListener<E extends MyEvent>(event: E, callback: Callback<CallbackTypes[E]>) { const callbacks = this.callbacks[event]; callbacks.push(callback); / okay } getListeners<E extends MyEvent>(event: E) { return this.callbacks[event]; } }
Thanks to this PR above code now works! But when I change const callbacks = this.callbacks[event] to const callbacks: Array<Callback<CallbackTypes[E]>> = this.callbacks[event] the compiler throws Type 'Callbacks[E]' is not assignable to type 'Callback<CallbackTypes[E]>[]'. But when (in both versions) examining the type of callbacks.push, the Array is typed as Array<Callback<CallbackTypes[E]>>.
const callbacks = this.callbacks[event]
const callbacks: Array<Callback<CallbackTypes[E]>> = this.callbacks[event]
Type 'Callbacks[E]' is not assignable to type 'Callback<CallbackTypes[E]>[]'
callbacks.push
Array<Callback<CallbackTypes[E]>>
Why then, is Callbacks[E] not assignable to Callback<CallbackTypes[E]>[] while in the push call the Callbacks[E] is automagically converted to that very same type Array<Callback<CallbackTypes[E]>>?
Callbacks[E]
Callback<CallbackTypes[E]>[]
push
satisfies
any
Remove some @ts-expect-errors for mapping types
79e8387
This does some hacky things to get inference that works for the mapping types. Based on: microsoft/TypeScript#47109 Also adds two comments where the expect error cannot reasonably be removed (as far as I can see). Experiment with it here: https://www.typescriptlang.org/play?ts=5.3.3#code/C4TwDgpgBAYglgJwgEwPKQQQ2HA9gOygF4oBvKUSALigCIAzRFWgGigGMIAbLm-AVwC2AIwgIoAXwBQlaAFlcANxTox2PIRLlZNWoKXM2nHnyGiEbMLgDOcHARoBtASLFsX5gLqSZ4aKqx7TVgmNAx1AigAHygFZTC1IKlkgHoUij8oAIjg8kYkZBpUQTsAHngC7KC2WllaAD42fXiikuBSuJVw6ro6+skAbl9ILO6NABVMkiqNR1q-Wk9k2VHEjTlMMFKAaSgIAA9gCHxka1XAib9+rShHAAUoOEJtzxpSKShPjOooO5YpCRQABkWTapQAoocsOx2jMCGxSDpfhJGr0FvUAY4XkNhtAYPx8DD1psdntDsdTuccpNINcyB8vvdHs9XlAABS4MYOKlBDZbO71ACUxH6ilwcGQAOS7AI1mAUHoBKJBD5NHxhN5JLh+BpEDp7y+CtCNDZ+RQAH1OWtuRUutb8IKaGKJSL6YbDWbkJaufgAHTGLgMz4Sf6G5ooE3hr1Wi7czoJWMOp3i5Cug3uz5R732-3cQMZzMGaM+31WWxJQ3SaRSRUajRQTZgLggbWkg5HE5nbW6+ocn1FH18nZC5Mu9OfGX4OVQfbEKAxnK+2RBjiy+W19hzjeasCOfZLD1Kvv2wVSoA Signed-off-by: anderium <[email protected]>
undefined
This PR is linked from #47370
Remove some @ts-expect-errors for mapping types (#298)
ddbf25f
RyanCavanaugh RyanCavanaugh approved these changes
DanielRosenwasser Awaiting requested review from DanielRosenwasser
weswigham Awaiting requested review from weswigham
ahejlsberg
Successfully merging this pull request may close these issues.