Skip to content

Incorrect substitution on index access with generic key type on generic mapped type with key remapping #47794

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

Closed
LANGERGabrielle opened this issue Feb 8, 2022 · 6 comments · Fixed by #47889
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Help Wanted You can do this

Comments

@LANGERGabrielle
Copy link

Bug Report

As soon as this bug is approved I can open the PR with my fix.
This seems somewhat related to #44115

🔎 Search Terms

mapped remapping

🕗 Version & Regression Information

  • This bug is present since 4.1.0, when remapping was introduced

⏯ Playground Link

Playground link for ^4.4
Playground link for ^4.1

💻 Code

type Foo<T extends string> = {
    [RemappedT in T as `get${RemappedT}`]: RemappedT;
};
const get = <T extends string>(t: T, foo: Foo<T>): T => foo[`get${t}`]; // Type '`get${T}`' is not assignable to type 'T'

🙁 Actual behavior

When resolving the type of foo[`get${t}`], Typescript substitutes RemappedT with `get${T}`. foo[`get${t}`] ends up resolving to `get${T}`.

🙂 Expected behavior

Typescript should resolve foo[`get${t}`] with the type T.

@ahejlsberg
Copy link
Member

We consider an indexed access type { [P in K]: E }[X] to be constrained to an instantiation of E where P is replaced with X. However, this simplification is correct only when the mapped type doesn't specify an as clause, and we fail to check for that in some places. While it may be desirable to perform the constraint simplification for mapped types with as clauses, it isn't possible because we can't consistently construct a reverse mapping for the as clause (indeed, it may not even be a 1:1 mapping).

So, the issue here isn't that the example should type check, but rather that the error message is wrong. We should be reporting that type Foo<T>[`get${T}`] is not assignable to type T.

@LANGERGabrielle
Copy link
Author

LANGERGabrielle commented Feb 14, 2022

You are the judge on this, but I don't think we have to prevent the simple cases simply because complex cases are not supported. In this bug's case, we don't need to do a reverse mapping, as instantiating E with P replaced by K is enough and as far as I can think of correct in terms of typing. The behavior would be similar to the current one when X is any in non strict mode.

For more context, here is an example where reverse mapping would be necessary :

type Mapped2<K extends string> = { [P in K as `get${P}`]: { a: P } };

function f2<K extends string>(obj: Mapped2<K>, key: `get${Exclude<K, 'A'>}`) {
    const x: { a: Exclude<K, 'A'> } = obj[key];  // This is obviously correct, but needs reverse mapping to be type checked correctly
}

If you'd prefer, I might be able to implement a reverse mapping that works in most cases, even if it won't be complete.

@j3k0
Copy link

j3k0 commented Feb 17, 2022

It looks like my issue is a more complex version of the same thing. I'm not TypeScript superstar, so I'm not completely sure though, checking here before posting another issue.

Here's what my code does:

type Type = "a" | "b";

type Stuff = {
  type: Type;
};

const Factory = {
    a (_arg: string): Stuff { return { type: "a" } },
    b (_arg: number): Stuff { return { type: "b" } }
}

type FactoryParameter = {
  [type in Type]: Parameters<typeof Factory[type]>[0];
}

function foo<T extends Type>(type: T, param: FactoryParameter[T]): Stuff {
    return Factory[type](param);
}

const v1 = foo("a", "hello");
const v2 = foo("b", 12);

The issue is in function "foo()". It builds fine with foo<T extends "a"> and foo<T extends "b"> but not with foo<T extends Type> (with Type = "a" | "b").

I get the error below.

error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'never'.
  Type 'string' is not assignable to type 'never'.

19     return Factory[type](param);
                            ~~~~~

Is this the same bug? Is there something I'm missing here?

@LANGERGabrielle
Copy link
Author

LANGERGabrielle commented Feb 17, 2022

@j3k0 your code is incorrect, as it would allow

foo<'a' | 'b'>('a', 12)

Here is how I would do it.

@j3k0
Copy link

j3k0 commented Feb 18, 2022

@LANGERGabrielle you are right, thanks. Though my use case doesn't really allow your pattern, my actual "foo" function needs to use the Factory, but do many other things with the thing it builds: it can't just return the Factory unfortunately. All I can do is build the "Stuff" outside the function (makes the things less nice to use... but OK)

For the record, my issue is more like those: #13995 #25879 - TypeScript can't force a type to be singleton (yet... I hope).

Thanks again.

@ahejlsberg ahejlsberg modified the milestones: Backlog, TypeScript 4.7.0 Feb 19, 2022
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Feb 21, 2022
@LANGERGabrielle
Copy link
Author

I still think it's a shame that key remapping is unusable with generics. I had the impression that what I was proposing was sound.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Help Wanted You can do this
Projects
None yet
5 participants