Skip to content
New issue

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

Mapped array/tuple types can't be used as generic with a constraint #48740

Closed
Jamesernator opened this issue Apr 17, 2022 · 9 comments
Closed
Labels
Fixed A PR has been merged for this issue

Comments

@Jamesernator
Copy link

Bug Report

πŸ”Ž Search Terms

mapped tuple type

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about mapped type

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

class ValType {
    #isValType = true;
}
class I32 extends ValType {
    #isI32 = true;
}
class Externref extends ValType {
    #isExternref = true;
}

class Local<T extends ValType> {
    readonly #type: T;

    constructor(type: T) {
        this.#type = type;
    }

    get type(): T {
        return this.#type;
    }
}

type LocalsFor<Type extends ReadonlyArray<ValType>> = {
    // Type 'Type[K]' does not satisfy the constraint 'ValType'.
    //   Type 'Type[keyof Type]' is not assignable to type 'ValType'.
    //     Type 'Type[string] | Type[number] | Type[symbol]' is not assignable to type 'ValType'.
    //       Type 'Type[string]' is not assignable to type 'ValType'.(2344)
    [K in keyof Type]: Local<Type[K]>
};

// This is still the correct type, because mapped array/tuples still
// just map their values despite the above error
const locals: LocalsFor<[I32, Externref]> = [
    new Local(new I32()),
    new Local(new Externref()),
];

πŸ™ Actual behavior

It produces an error in the mapped type that Type[K] doesn't satisfy the constraint.

πŸ™‚ Expected behavior

There should be no error, as mapped types do not map over all keys in the array/tuple, it should only consider Type[K] to be the numeric keys of the tuple, i.e. Type[K] in the mapped type will only ever be (subtypes of) ValType because ultimately K will only be assigned to the numeric keys of Type as it's an array type.

In this particular case, we have an explicit Type extends ReadonlyArray<ValType> so TypeScript should be able to deduce just fine that this particular mapped type will only ever be over an array, and hence should be able to deduce that Type[K] will only ever be Type[number] (or Type[`${ number }`] for tuples).

I had another issue, however I closed it and opened this one because this is likely the fundamental cause of both problems.

@whzx5byb
Copy link

whzx5byb commented Apr 17, 2022

I think it is explained in #48398.

For you example, a workaround is replace Local<Type[K]> with Local<Type[K & number]>

@Jamesernator
Copy link
Author

Jamesernator commented Apr 17, 2022

I think it is explained in #48398.

The fact tuple keys aren't subtypes of number is explained, however that is a distinct issue. Here the problem is that K will only be the "numeric like" types (although note from that issue that for tuples, K is never assignable to number, they are exposed as string types only).

For you example, a workaround is replace Local<Type[K]> with Local<Type[K & number]>

No, that doesn't work, it causes the final type in the example to become:

const locals: [Local<I32 | Externref>, Local<I32 | Externref>]

Which is not correct, the existing [Local<I32>, Local<Externref>] is correct already actually, it's just that it warns in the mapped type despite the fact it produced the correct type anyway.

@whzx5byb
Copy link

whzx5byb commented Apr 17, 2022

No, that doesn't work

You are right. Local<Type[K & number]> doesn't work.

Since what we need is only 'numeric like' types in keyof T, I believe it is necessary to introduce a generic type KeyOf<T> to provide only numeric index types for array/tuple, which should be in a form of 0 | 1 | 2 ... | N-1 when T is a N-length tuple, and should be number when T is an unknown-length array.
It could be written as below.

type _GetKeysFromLength<
  L extends number,
  R extends readonly unknown[] = [],
> = number extends L ? number : R['length'] extends L ? R[number] : _GetKeysFromLength<L, readonly [R['length'], ...R]>;

type KeyOf<T extends unknown[]> = _GetKeysFromLength<T['length']> extends infer U extends number ? U : never;
// for versions prior to v4.7
// type KeyOf<T extends unknown[]> = _GetKeysFromLength<T['length']> extends infer U ? U extends number ? U : never : never;

type R1 = KeyOf<[1,2,3]>;
//   ^? type R1 = 0 | 2 | 1
type R2 = KeyOf<number[]>;
//   ^? type R2 = number


type LocalsFor<Type extends Array<ValType>> = {
    [K in KeyOf<Type>]: Local<Type[K]>
};

const locals: LocalsFor<[I32, Externref]> = [
    new Local(new I32()),
    new Local(new Externref()),
]; // seems OK now!

var l0 = locals[0]
// ^? var l0: Local<I32>
var l1 = locals[1]
// ^? var l1: Local<Externref>

Playground

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Apr 18, 2022

This is a correct error.

In this particular case, we have an explicit Type extends ReadonlyArray so TypeScript should be able to deduce just fine that this particular mapped type will only ever be over an array, and hence should be able to deduce that...

This is the incorrect assumption.

Matching the constraint extends ReadonlyArray<ValType> does not provide a bound on an arbitrary Type[keyof Type], as shown:

type M = ReadonlyArray<ValType> & { k: boolean };
type LocalsFor<Type extends ReadonlyArray<ValType>> = {
    // Type 'Type[K]' does not satisfy the constraint 'ValType'.
    //   Type 'Type[keyof Type]' is not assignable to type 'ValType'.
    //     Type 'Type[string] | Type[number] | Type[symbol]' is not assignable to type 'ValType'.
    //       Type 'Type[string]' is not assignable to type 'ValType'.(2344)
    [K in keyof Type]: Local<Type[K]>
};
type G = LocalsFor<M>;
type X = G["k"]; // x: Local<boolean>, a constraint violation

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Apr 18, 2022
@Jamesernator
Copy link
Author

Jamesernator commented Apr 18, 2022

Matching the constraint extends ReadonlyArray<ValType> does not provide a bound on an arbitrary Type[keyof Type], as shown:

These bizarre rules make the whole point of "homomorphic mapped types" seem fairly weak and unreliable, like if there's gonna be special casing for arrays in mapped types to not map stuff like methods, why would we want it to map other random properties that might be attached onto the array? (as an example, symbol-keyed protocols like Symbol.for("nodejs.util.inspect.custom") and such, people probably wouldn't want such properties suddenly appear in mapped types if no other methods do)

The amount of bizarre behaviours in the mapped types feature just constantly surprises me, I don't see how any developers who aren't intimately familiar with TypeScript's internals are expected to have any reasoning about these complicated features (and it doesn't help that that handbook doesn't even mention arrays are (kind've) homomorphic in mapped types to begin with, this was something just mentioned once when the feature was released in the associated release blog post).

@Jamesernator
Copy link
Author

Either way, I think I'll reopen the other issue because that one at the very least checks the keys for "numeric"-ness.

@RyanCavanaugh
Copy link
Member

The amount of bizarre behaviours in the mapped types feature just constantly surprises me

Every single rule we've added to mapped types was put there to remove some other surprise πŸ€·β€β™‚οΈ

@jcalz
Copy link
Contributor

jcalz commented Apr 20, 2022

So is #27995 not really a bug then?

@jcalz
Copy link
Contributor

jcalz commented Apr 27, 2022

This has changed from "working as intended" to "fixed" by #48837. Not sure if anyone wants to alter tags here.

@ahejlsberg ahejlsberg added Fixed A PR has been merged for this issue and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

5 participants