Skip to content

Missing reduction step in a union of intersections with bounded type parameter #48718

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
jfet97 opened this issue Apr 15, 2022 · 4 comments
Closed
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@jfet97
Copy link
Contributor

jfet97 commented Apr 15, 2022

Bug Report

πŸ”Ž Search Terms

reduction, generic, union, intersection

πŸ•— Version & Regression Information

This bug is also the 'Nightly' version. Versions prior to 4.6.2 give even more errors.

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

type NumericLiteral = {
  value: number;
  type: "NumericLiteral";
};
type StringLiteral = {
  value: string;
  type: "StringLiteral";
};
type Identifier = {
  name: string;
  type: "Identifier";
};
type CallExpression = {
  name: string;
  arguments: DropbearNode[];
  type: "CallExpression";
};

type DropbearNode =
  | NumericLiteral
  | StringLiteral
  | Identifier
  | CallExpression;

type Visitor = {
  [K in DropbearNode["type"]]: (node: Extract<DropbearNode, { type: K }>) => void
};

function visitNode<K extends DropbearNode['type']>(node: Extract<DropbearNode, { type: K }>, v: Visitor) {
   v[node.type](node) // error
}

πŸ™ Actual behavior

typeof node.type is inferred as (K & "NumericLiteral") | (K & "StringLiteral") | (K & "Identifier") | (K & "CallExpression") that it is equal to K itself considering that K extends "NumericLiteral" | "StringLiteral" | "Identifier" | "CallExpression".
Something goes really wrong when I try to use node.type as an index, maybe because its type doesn't perform well if used as an index to access the Visitor type.

πŸ™‚ Expected behavior

typeof node.type inferred as K because it is simpler to interact with and doesn't give problems if used as an index.

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Apr 15, 2022
@RyanCavanaugh
Copy link
Member

given that K extends DropbearNode['type'] = "NumericLiteral" | "StringLiteral" | "Identifier" | "CallExpression"

It's not clear how we'd come to that conclusion in a tractable amount of time. From the outside you can see why that's happening, but that broader perspective isn't present during the evaluation of node.type.

If you think you can fix this feel free to send a PR, but I don't think this is something we're likely to be able to address given that the proposed reduction rule would have to be able to figure out that it's inside a context where the type parameter is effectively bounded and that the expansion is in a place where it's safe to "undo" it -- the posited reduction rule is, I think, capable of causing incompleteness in other currently-working programs.

@jfet97
Copy link
Contributor Author

jfet97 commented Apr 16, 2022

@RyanCavanaugh @jcalz thinking about it, I wonder if we have some sort of regression here because the following seems to be unsound but accepted by TS 4.6.2 or above:

function visitNode2<K extends DropbearNode['type']>(
  node1: Readonly<Extract<DropbearNode, { type: K }>>, 
  node2: Readonly<Extract<DropbearNode, { type: K }>>, 
  v: Visitor) {   
   v[node1.type as K](node2)
}

Playground

The cast of the node1.type to K shouldn't be enough to assure that node2 is the right node:

declare const v: Visitor

declare const node1: NumericLiteral
declare const node2: CallExpression

visitNode<"NumericLiteral" | "CallExpression">(node1, node2, v)

In the worst case K could be the full union "NumericLiteral" | "StringLiteral" | "Identifier" | "CallExpression", so TS versions like 4.5.5 or the previous ones correctly required the argument to always be an intersection of the nodes' types.

@RyanCavanaugh
Copy link
Member

This was an intentional trade-off to enable the pattern where you have a single value to infer the type parameter from; see https://devblogs.microsoft.com/typescript/announcing-typescript-4-6/#indexed-access-inference-improvements. visitNode2 should use two type parameters in this case.

@Andarist
Copy link
Contributor

This design limitation has been lifted by #56515 πŸ˜‰

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

3 participants