GeezerVison

TypeScript Bot (@typescript-bot) perf test faster
TypeScript Bot (@typescript-bot) test this
TypeScript Bot (@typescript-bot) user test inline
TypeScript Bot (@typescript-bot) run dt

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the abridged perf test suite on this PR at d42e4c2. You can monitor the build here.

Update: The results are in!

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the extended test suite on this PR at d42e4c2. You can monitor the build here.

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the parallelized Definitely Typed test suite on this PR at d42e4c2. You can monitor the build here.

TypeScript Bot (@typescript-bot) user test this inline

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the inline community code test suite on this PR at d42e4c2. You can monitor the build here.

Update: The results are in!

Anders Hejlsberg (@ahejlsberg)
The results of the perf run you requested are in!

Anders Hejlsberg (@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({ 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:

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

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.

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.

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]>>.

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]>>?

This PR is linked from TypeScript 4.6 release notes but the implementation changed soon after it here: #47370