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

Transformers in a map generated via a Mapped Type with Key Remapping are erroneously inferred to return any #57097

Closed
brotheroftux opened this issue Jan 19, 2024 · 3 comments
Assignees
Labels
Fixed A PR has been merged for this issue

Comments

@brotheroftux
Copy link

πŸ”Ž Search Terms

"mapped types", "key remapping", "indexed access type"

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried (5.2.2, 5.3.2, 5.4.0-dev.20240119), and I reviewed the FAQ.

⏯ Playground Link

https://www.typescriptlang.org/play?target=99&ts=5.4.0-dev.20240119#code/KYDwDg9gTgLgBDAnmYcByEAmwCSA7GYKAMwEMBjYAHgBkBLQqUgGwBVlVRC9MBnOXjCh08AczgBeAUJGiAfJLgBvAFBx1CDgC449Ri3YoA3CoC+JlaEixNKOAGUidFnQBewTPZliAssBikigBKwOTQmFSCwmIANNLR4gA+cACuPMDEIh5yFlbQ8Eh2ACIQALYAoiBCFISYGNhFpAGKqhpweFjAhsA6UbImbaX+pDqOwi7unt6ifgEm5iqW4Pm2qPVdECWlY87MbkRU63BcwDz86-iMZJQKUgAUahod2DrrMSoAlJIKOxMeXglZqRcssbIU1p0gb89u4oD5SGBDp0AKp4OgQPDHKqnPjoTqXIjXYC3ZSPdQAbSOIjx2FR6MxpHOnXJAHJwSyALoc16dVibMrQ-ZQJHYHJmCxhPCCOAAC0ZiiorCx3FxEAARgArUIwOQPNoQHSsd5tADWvWmcGSeBSpTVREtAkQtogzHeHx0JrgdH4JuAiAgxDgSokCgA8prtQA6OW8UMAdzwdwgcRNHwlGOlQkQgvcyN4snhYDAsgVSpOZxpuAIhIoxL1GlKCOLYh52ChTj+cIRirkxqenUNbu+pLadEDdxjd0bRdkcWewEj4I+X1abQ0kulgSk0+bonJ88XHA5FjX64z8DVilId3nacWp-UUH8KSgDJvnTvbVM940MBlUAgON2mAIDunKKAAKgO4WVRXgUiLfIPHaTpVhZO9zCAA

πŸ’» Code

export type NodeInterface<LiteralType extends string = string> = {
    type: LiteralType;
};

export type SerializedStringMeta = Record<string, string | undefined>;

export type DomExtractedNodeData = {
    nodeType: string;
    meta: SerializedStringMeta;
};

export type NodeToDomSerializer<Node extends NodeInterface> = (
    node: Node,
) => SerializedStringMeta;

export type NodeMetaSerializerMap<NodeUnion extends NodeInterface> = {
    [Node in NodeUnion as Node['type']]: NodeToDomSerializer<Node>;
};

const has = <T extends object>(
    o: T,
    k: string | number | symbol,
): k is keyof T => Object.hasOwn(o, k);

const trySerializeUsingMapping = <T extends NodeInterface>(
    mapping: NodeMetaSerializerMap<T>,
    node: T,
) => {
    if (has(mapping, node.type)) {
        const a = mapping[node.type];

        const b = a(node);

        return a(node);
    }

    throw new TypeError('Unsupported node type');
};

πŸ™ Actual behavior

Type of the variable a is correctly inferred to be NodeMetaSerializerMap<T>[keyof NodeMetaSerializerMap<T>]. To be rigorous this should be NodeToDomSerializer<T>.

Type of variable b, however, is inferred to be any. Compiler doesn't warn me about this, and as you can understand, this could lead to all sorts of potential any-leakages. Compiler option strict doesn't change this as well.

πŸ™‚ Expected behavior

I expect type of the variable b to be correctly inferred as SerializedStringMeta, or at least some kind of error to be shown in the call site of a(node).

Additional information about the issue

I've also reviewed this pull request. I've tried the following work-around:

type TypeToNodeMapping<Node extends NodeInterface> = {
    [K in Node as Node['type']]: Node;
};

export type NodeMetaSerializerMap<NodeUnion extends NodeInterface> = {
    [K in keyof TypeToNodeMapping<NodeUnion>]: NodeToDomSerializer<
        TypeToNodeMapping<NodeUnion>[K]
    >;
};

This results in a following error being shown at the call site for a(node):

image

This also seems like a weird behaviour, because I've tested different things and verified that TypeToNodeMapping<NodeInterface>[NodeInterface['type']] is correctly resolved as NodeInterface. Seems to me like the compiler is for some reason refusing to unpack the types further, despite the fact it seems safe to use the upper boundary NodeInterface when resolving types for T.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jan 19, 2024
@ahejlsberg
Copy link
Member

We have bit of a complicated history for our behavior here. We would error on the call to a(node) in TS versions prior to 5.1. In 5.1 we merged #53066 which (erroneously) caused the error to disappear, instead just treating a as an untyped function that takes any parameter list and returns any. That PR was reverted in #57202, so this will become an error again in 5.4 (but isn't an error in the just released 5.4-beta).

@ahejlsberg
Copy link
Member

With the current nightly build, the a variable has type NodeMetaSerializerMap<T>[keyof NodeMetaSerializerMap<T>]. In this particular example we can simplify keyof NodeMetaSerializerMap<T> to T["type"] (but that isn't always the case, as demonstrated by #55129). However, the checker can't further simplify NodeMetaSerializerMap<T>[T["type"]] because it is not sound to conclude that this is the same as NodeToDomSerializer<T>. Specifically, there is no guarantee that T["type"] represents a unique value for every T, so T["type"] could very well be a lossy one-way mapping.

Another way to look at it is, mapping[node.type] produces a function that is known to be safe to invoke for that node instance. But the checker only tracks T, the type of node, and there could well be other values of type T for which it isn't safe to invoke a.

@ahejlsberg
Copy link
Member

In conclusion, I'm going to close this as fixed by #57202 because we now issue an error as expected.

@ahejlsberg ahejlsberg added Fixed A PR has been merged for this issue and removed Needs Investigation This issue needs a team member to investigate its status. labels Jan 30, 2024
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

3 participants