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

Regression with branded types in TypeScript 4.2 #42939

Closed
TylorS opened this issue Feb 24, 2021 · 9 comments · Fixed by #43183
Closed

Regression with branded types in TypeScript 4.2 #42939

TylorS opened this issue Feb 24, 2021 · 9 comments · Fixed by #43183
Assignees
Labels
Breaking Change Would introduce errors in existing code Fix Available A PR has been opened for this issue Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@TylorS
Copy link

TylorS commented Feb 24, 2021

Bug Report

When attempting to put together some conditional types to extract information about branded types, TS 4.2 is no longer able to correctly infer your intent.

type Branded<T, A> = A & { __brand: T }
type BrandedValue<A> = A extends Branded<unknown, infer R> ? R : never 

// Expected to be 'string' but is 'string & { __brand: T }' in 4.2+
type T1 = BrandedValue< Branded< 'GUID', string > >

🔎 Search Terms

  • Branded Types
  • Conditional Types
  • TypeScript 4.2

🕗 Version & Regression Information

  • This changed between versions 4.1.5 and 4.2 beta

⏯ Playground Link

Working example in 4.1.5

https://www.typescriptlang.org/play?#code/C4TwDgpgBAQgTgQwHYBMIoDwBUA0UCCAfFALwFQBkUA3lAPp0BGiqAXFFlAL4BQPokWCxQB5AGYYipKAG18AXSgQAHsAioAzrPjI0mAJZIxEOFABKeAK5IA1kgD2AdySFFAfnNR2SCADcTUPzg0Dqo6ABqCAA2lhCSxGT4SqrqKFqhehjWdk5IeIbGpmbEHmZeUD7+pnwC0ADilvoo0hnoGABEDU3teBrAcIYA5oRBgl0oGS3C4hjjI7VQ45Ex0GStKMuxs40oI0A

Not working in 4.2
https://www.typescriptlang.org/play?ts=4.2.0-beta#code/C4TwDgpgBAQgTgQwHYBMIoDwBUA0UCCAfFALwFQBkUA3lAPp0BGiqAXFFlAL4BQPokWCxQB5AGYYipKAG18AXSgQAHsAioAzrPjI0mAJZIxEOFABKeAK5IA1kgD2AdySFFAfnNR2SCADcTUPzg0Dqo6ABqCAA2lhCSxGT4SqrqKFqhehjWdk5IeIbGpmbEHmZeUD7+pnwC0ADilvoo0hnoGABEDU3teBrAcIYA5oRBgl0oGS3C4hjjI7VQ45Ex0GStKMuxs40oI0A

Not working in Nightly
https://www.typescriptlang.org/play?ts=4.3.0-dev.20210224#code/C4TwDgpgBAQgTgQwHYBMIoDwBUA0UCCAfFALwFQBkUA3lAPp0BGiqAXFFlAL4BQPokWCxQB5AGYYipKAG18AXSgQAHsAioAzrPjI0mAJZIxEOFABKeAK5IA1kgD2AdySFFAfnNR2SCADcTUPzg0Dqo6ABqCAA2lhCSxGT4SqrqKFqhehjWdk5IeIbGpmbEHmZeUD7+pnwC0ADilvoo0hnoGABEDU3teBrAcIYA5oRBgl0oGS3C4hjjI7VQ45Ex0GStKMuxs40oI0A

🙁 Actual behavior

The brand is not removed from the return type

🙂 Expected behavior

The brand should be removed from the return type

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Feb 24, 2021
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.2.2 milestone Feb 24, 2021
@RyanCavanaugh
Copy link
Member

@weswigham this one might require a hotfix if we see multiple reports

@ahejlsberg
Copy link
Member

Hang on, I'm about to post an explanation of what's going on...

@ahejlsberg ahejlsberg assigned ahejlsberg and unassigned weswigham Feb 24, 2021
@ahejlsberg
Copy link
Member

TL;DR answer is to change the BrandedValue type to use BrandOf<A> instead of unknown:

type Branded<T, A> = A & { __brand: T };
type BrandOf<A> = [A] extends [Branded<infer R, unknown>] ? R : never;
type BrandedValue<A> = A extends Branded<BrandOf<A>, infer R> ? R : never;

This helps type inference better recognize that we're inferring between two types with identical { __brand: xxx } brands, allowing the remainder of the type to be extracted. The change fixes the issue in 4.2 and generally cause the BrandedValue type to work with types that are structurally compatible with Branded<T, A>.

Longer explanation...

First, the example in the original post above actually works in 4.2. However, the examples in the playground link are broken as indicated, which is an effect of our improved type alias preservation.

The type alias

type Guid = Branded<"Guid", string>;

used to simply reference the intersection type string & { __brand: "Guid" } with the associated alias Branded<"Guid", string>. Internally in the compiler, the type references Guid and Branded<"Guid", string> would reference the exact same intersection type object and the type would always display as Branded<"Guid", string>.

In 4.2, Guid actually becomes a distinct intersection type string & { __brand: "Guid" } with the associated alias Guid. This type is structurally compatible with Branded<"Guid", string>, but it is not the exact same type object (because it has a different alias associated with it). The new representation allows us to preserve and display the Guid alias (which is good), but it also means that type inference can't rely on inferring between instantiations of the same type alias--and this is where things go wrong in your example.

Some examples to illustrate the issue:

type Branded<T, A> = A & { __brand: T };
type BrandOf<A> = [A] extends [Branded<infer R, unknown>] ? R : never;
type BrandedValue<A> = A extends Branded<unknown, infer R> ? R : never;

type Guid = Branded<"Guid", string>;

type T1 = BrandedValue<Branded<'GUID', string>>;       // Works in 4.1 and 4.2
type T2 = BrandedValue<Guid>;                          // Works in 4.1, fails in 4.2
type T3 = BrandedValue<string & { __brand: "Guid" }>;  // Fails in 4.1 and 4.2

With the change to BrandedValue I suggested above, all of these examples work in both 4.1 and 4.2.

@ahejlsberg ahejlsberg added Breaking Change Would introduce errors in existing code 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 Feb 24, 2021
@TylorS
Copy link
Author

TylorS commented Feb 24, 2021

Hey @ahejlsberg I really appreciate the workaround and the detailed explanation, it makes perfect sense. 🙏

Editor-focused improvements, despite being very welcomed, requiring code changes does feel a little like this could be a leak of implementation details. Was this really the intended behavior when trying to preserve aliases? I couldn't find mention of this behavior in #35654 or #42149

@RyanCavanaugh
Copy link
Member

When you have an inference of the form

type T = U extends V & infer W ? W : never;

There is not one exact-right answer for W. In fact, any subtype of V is a valid answer for infer W, and our choice amongst those is sometimes reliant on heuristics around "what you intended". You should think of an inference position here as possibly returning any of those types.

For example:

type Point2d = { x: number, y: number };
type Point3d = { x: number, y: number, z: number };

type T = Point3d extends Point2d & infer W ? W : never;

Here, T is Point3d, not { z: number }. However, if you wrote this as

type Point3d = { x: number, y: number } & { z: number };

Then you'd see T = { z: number }

Or in this form, we go back to inferring the whole type:

type Point3d = { x: number } & { z: number, y: number };

type T = Point3d extends Point2d & infer W ? W : never;

Importantly, none of these answers are wrong.

The "peeling off" heuristic behavior of intersections in inference positions is delicate and it's best to try to avoid relying on it too heavily. It's expected that indirections (including things that weren't previously considered indirections) can thwart that process. It's a nice-to-have when it works but is not a correctness deficit when it doesn't.

@TylorS
Copy link
Author

TylorS commented Feb 24, 2021

The subtyping issue makes sense from a theoretical standpoint, thanks for that explanation @RyanCavanaugh.

I still feel a portion of my point was missed. It's fine that there are multiple answers, but this broke code by changing which answer TypeScript picks. Since this is a hidden choice from programmers, that would be my definition of an implementation detail. I'll concede here though...I'm happy enough there's a workaround for this particular issue and to understand why it's happening.

Less related to the original question, and more specific to the last paragraph regarding the "peeling off" behavior of intersections. I've seen and written several open-source libraries making pretty heavy use of this type-inference w/ intersections as a way to track requirements (as a record) that can be added/subtracted from. They all pretty much run into these issues you describe but are generally workable. Are there any proposals/features on the horizon to actually improve this ability?

@weswigham
Copy link
Member

#33290 or #33038 probably.

@TylorS
Copy link
Author

TylorS commented Feb 24, 2021

Ooh thanks for those links, I wasn't aware of either of those 🙏

But I meant more generally regarding records and not just brands, sorry I wasn't more clear. Variadic kinds from way back is the one thing that comes to mind which could have maybe solved this example like

type Point3d = { x: number } & { z: number, y: number };

// I don't actually know what syntax would have worked here
type T = Point3d extends { ...Point2d, ...infer W } ? W : never;

@typescript-bot
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Would introduce errors in existing code Fix Available A PR has been opened for this issue Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants