Improve computed base constraint for Index type on generic IndexedAccess

[Andarist]

Relates to the issue reported here: #49071

export type Context = {
    V1: { "a": string },
    V2: { "b": string },
}

export function path<V extends keyof Context, K extends keyof Context[V]>(v: V, k: K)  {
    return `${v}.${k}`
}

This is what I got here... from what I understand K is constrained to be of type keyof Context[V] and that stays unresolved~ because V is generic. So it stays to be represented as an Index type (which is a "return type" of the keyof operator from what I understand). And as such TS computes its base constraint like this here:

if (t.flags & TypeFlags.Index) {
  return keyofConstraintType;
}

If I reason about this correctly - we can't just compute the base constraint as a union of those inner properties because that would possibly allow some invalid property accesses within the function's body. If we'd compute this indexed access then this constraint would be roughly equivalent to keyof ({ a: string; } | { b: string; }) but that's just never.

So I've sketched out this algorithm for the improved computation - but I'm not sure if it's completely correct.

cc @dragomirtitian

[typescript-bot]

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

[weswigham]

Suggested change

	const narrowed = mappedIndexTypeOfIndexedAccess && mapType(keyofConstraintType, t => {
	const intersected = mappedIndexTypeOfIndexedAccess && getIntersectionType([keyofConstraintType, mappedIndexTypeOfIndexedAccess]);
	const narrowed = intersected.flags & TypeFlags.Never ? undefined : intersected;

should be equivalent and much simpler.

[Andarist]

IIRC I intentionally haven't written it like that - although rn I don't exactly remember why.

Those are not equivalent as my version filters each member of the keyofConstraintType individually, so I end up with a subset of keyofConstraintType. Yours simply intersects keyofConstraintType with mappedIndexTypeOfIndexedAccess and that will preserve subset of the mappedIndexTypeOfIndexedAccess. So the return types of those operations are not the same.

As I said - right now I don't recall right now why I've chosen this approach but I remember that it was on purpose. That, of course, doesn't mean that my version is better here - perhaps yours is.

I feel like there is probably a test case to be written here that would help us to determine which one is correct...

Should this be assignable?

export type Context = {
    V1: { "a": string },
    V2: { "b": string },
}

export function path<V extends keyof Context, K extends keyof Context[V]>(v: V, k: K)  { 
    // should this be assignable?
    const a: 'a' | 'b' = k
    return a
}

With my version of the code it isn't but with yours it is.

Funny enough this indexed access errors with both versions:

export type Context = {
    V1: { "a": string },
    V2: { "b": string },
}

declare const c: {
    a: string;
    b: string
}

export function path<V extends keyof Context, K extends keyof Context[V]>(v: V, k: K)  { 
    const a: 'a' | 'b' = k
    return c[k] // this errors
}

So it probably should be fixed too if your proposed change is correct.

[Andarist]

Actually, I think that your version wouldn't be correct - mainly because of the presented case with this being assignable:

const a: 'a' | 'b' = k

We can't guarantee this. After all, as K might only be instantiated with 'a', 'b or never. Those are the only (?) valid calls here:

path('V1', 'a')
path('V2', 'b')

declare const v: keyof Context
path(v, {} as never)

keyof is not distributable so keyof ({ a: string } | { b: string }) is simply never as it gets the common set of properties.

[weswigham]

keyofConstraintType is string | number | symbol, afaik if mappedIndexTypeofIndexedAccess is T, the operation here is essentially (string & T) | (number & T) | (symbol & T), which is fundamentally what T & (string | number | symbol) works out to, no? Unless I'm missing something very subtle?

[Andarist]

My operation is closer to this:

type Process<T1, T2> = T1 extends any
  ? [T1 & T2] extends [never]
    ? never
    : T1
  : never;

type FilteredSource = Process<PropertyKey, "a" | "b">;
//   ^? string

type Intersected = PropertyKey & ("a" | "b");
//   ^? 'a' | 'b

TS playground

[weswigham]

Ah, I missed that it was returning t and not the intersection result in the non-never case.

You wanna use filterType instead of mapType then.

[Andarist]

You wanna use filterType instead of mapType then.

I've pushed out this change.

q: do you maybe have an idea for a better test than the first snippet here? or do you think that trying to write a "reversed" test here is not worth it/bad idea?

[Andarist]

@weswigham friendly 🏓 do you believe this is a good approach for this problem or would you like me to explore some alternative solutions?

[Andarist]

@MartinJohns thanks for mentioning this PR in one of the issues. I've noticed that and thanks to that I've managed to correct the PR.

[Andarist]

@jakebailey would there be a chance for a playground build here? :)

[jakebailey]

@typescript-bot pack this

[typescript-bot]

Heya @jakebailey, I've started to run the tarball bundle task on this PR at cfcfa9f. You can monitor the build here.

[typescript-bot]

Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/147149/artifacts?artifactName=tgz&fileId=8696CCB5B16DC3EAE0DD071D49FF222CFD7A1D48914F1F1D370AB39AD923A4D602&fileName=/typescript-5.0.0-insiders.20230221.tgz"
    }
}

and then running npm install.

---

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.0.0-pr-49107-5".;