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

Type correlation regression #54834

Closed
jfet97 opened this issue Jun 29, 2023 · 7 comments · May be fixed by #54689
Closed

Type correlation regression #54834

jfet97 opened this issue Jun 29, 2023 · 7 comments · May be fixed by #54689
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@jfet97
Copy link
Contributor

jfet97 commented Jun 29, 2023

Bug Report

🔎 Search Terms

indexed Access Inference Improvements, indexed access type, mapped type, correlated types

🕗 Version & Regression Information

When did you start seeing this bug occur?

Without Readonly it never worked, the compiler doesn't see the correlation between v[node.type] and node.
With Readonly stopped working from 5.1.3.

⏯ Playground Link

With Readonly
Without Readonly

💻 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 TypeMap = {
  [K in DropbearNode["type"]]: Extract<DropbearNode, { type: K }>;
};

type Visitor = {
  [K in keyof TypeMap]: (node: Readonly<TypeMap[K]>) => void;
  //      I am referring to this ^ one...
};

function visitNode<K extends keyof TypeMap>(
  node: Readonly<TypeMap[K]>,
  // ... and ^ this one 
  v: Visitor
) {
  v[node.type](node);
}

🙁 Actual behavior

The focus is the implementation of the visitNode function .

Without the use of Readonly in the definition of Visitor and visitNode the code does not compile in any version of TS. On the other hand, using Readonly in those definitions has worked until 5.1.3. Thanks to those Readonly TS was somehow able to see the correlation between v[node.type] and node.

P.S. It seems that v[node.type as K](node) always solves the problem, in all versions, with or without the use of Readonly.

🙂 Expected behavior

I repropose a simplified version of this example because I've recently had a chance to get my hands back on code that makes use of the pattern suggested in #47109. This is sort of a simplified version of what is suggested in that PR so I am not sure about the right behaviour.

It would be nice if TS would be able to support this simplified pattern for expressing correlations, instead of something more convoluted like this one where I think I am perfectly following what is indicated in #47109. And TS seems to be able to do it, although I don't know why Readonly was needed nor why it suddenly stopped working.

@typescript-bot
Copy link
Collaborator

The change between origin/release-5.0 and origin/release-5.1 occurred at 9769421.

@RyanCavanaugh
Copy link
Member

#53098

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jun 29, 2023
@Andarist
Copy link
Contributor

So it seems that this fixes it #54689 but I'm not entirely sure if that solution is on the right track.

@ahejlsberg
Copy link
Member

I think this somewhat accidentally worked with Readonly, but now we do a better job of preserving the higher order types and thus we detect more situations where types differ at higher order. A formulation of the problem that more closely follows the pattern in #47109 seems to work nicely:

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

type DropbearNode<K extends keyof TypeMap = keyof TypeMap> = { [P in K]: { type: P } & TypeMap[P] }[K];

type NumericLiteral = DropbearNode<"NumericLiteral">;
type StringLiteral = DropbearNode<"StringLiteral">;
type Identifier = DropbearNode<"Identifier">;
type CallExpression = DropbearNode<"CallExpression">;

type Visitor = {
  [K in keyof TypeMap]: (node: DropbearNode<K>) => void;
};

function visitNode<K extends keyof TypeMap>(node: DropbearNode<K>, v: Visitor) {
  v[node.type](node);
}

@ahejlsberg ahejlsberg added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Needs Investigation This issue needs a team member to investigate its status. labels Jul 31, 2023
@ahejlsberg ahejlsberg removed their assignment Jul 31, 2023
@typescript-bot
Copy link
Collaborator

This issue has been marked as "Working as Intended" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 3, 2023
@jfet97
Copy link
Contributor Author

jfet97 commented Aug 3, 2023

@ahejlsberg yes that solution works nicely until we need to apply modifiers to the type of node just like Readonly. Is there a solution for this? Link to playground

@Andarist
Copy link
Contributor

It works if Readonly gets pushed into DropbearNode and if we avoid wrapping it over { type: P }: TS playground.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
5 participants