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

[5.4 regression] Conditional type changes behavior with 5.4-beta #57221

Closed
danvk opened this issue Jan 29, 2024 · 5 comments
Closed

[5.4 regression] Conditional type changes behavior with 5.4-beta #57221

danvk opened this issue Jan 29, 2024 · 5 comments
Labels
Not a Defect This behavior is one of several equally-correct options

Comments

@danvk
Copy link
Contributor

danvk commented Jan 29, 2024

πŸ”Ž Search Terms

5.4 regression

πŸ•— Version & Regression Information

  • This changed between versions 5.3.3 and 5.4-beta

⏯ Playground Link

https://www.typescriptlang.org/play?ts=5.4.0-dev.20240129#code/KYDwDg9gTgLgBDAnmYcCiIZQIYGMYBKEArjMAArY4C2AzgDwAqcoZAdgCa1y1YCWbAOYA+OAF4e-IS0zBO3RgFgAUHDgB+OAWC5oHerygDBAGklGhwlWoBccZqzlc4AAwAkAbwEAzYFDgA+gDKMFQwAL42nj5+cJQ04QD00Wy+-tq84S7WGnAecADaANZwAnFU2NRwAD5wRcCIEN7omDj4RKQUFXT0GTDCALp2hsZw4Tl2DrLyrilpgSFhkXOx8ZVZOZr5xaVs5TRD5qPjqnB2HuEA3CoqHDoANlSo3sRs+HwQe-cCRUwy7M4RpYABQ5KiCWjnHJqMDYGAAC0m11O4TgADI4MCLv8nNwMFg8IQSGQ1j1GKItnBYTRaOpzqjUecqd1IS0Ce1iV0aUxROEAJQmFR8uwANwgfA4yJUiUSLCgUGguwQ3AArAA6ADMmrg2E4cHVABY1QAGAC0ACNgKEVN82EUscyEXYAOSJGzeCAQN3mqjOszUyqs7H85EyuUK-xlGCqzWasxsCDhxVRmNGs2W0JquBBeEke4cOCWnV7PwRtU2n4O2FOuCu92e71URJ+5k0pkeiAujvOsZjPnIoA

πŸ’» Code

export type ExtractRouteParams<T extends string> = string extends T
  ? Record<string, string>
  : T extends `${infer _Start}:${infer Param}/${infer Rest}`
  ? { [k in Param | keyof ExtractRouteParams<Rest>]: string }
  : T extends `${infer _Start}:${infer Param}`
  ? { [k in Param]: string }
  : {};

declare function link<T extends string>(
  args: {
    path: T;
  } & ({} extends ExtractRouteParams<T> ? { params?: {} } : { params: ExtractRouteParams<T> }),
): void;

// error in ts 5.3.3 and 5.4.0-beta
link({ path: '/:foo/:bar', params: {} });
// error in ts 5.3.3, no error in ts 5.4.0-beta
link({ path: '/:foo/:bar/', params: { foo: 'foo' } });

πŸ™ Actual behavior

The last line is only an error in TS 5.3.3, not TS 5.4-beta.

πŸ™‚ Expected behavior

I'd expect it to be an error in both (foo isn't specified).

Additional information about the issue

Putting NoInfer in the conditional type makes the expected error reappear:

- } & ({} extends ExtractRouteParams<T> ? { params?: {} } : { params: ExtractRouteParams<T> }),
+ } & ({} extends ExtractRouteParams<NoInfer<T>> ? { params?: {} } : { params: ExtractRouteParams<NoInfer<T>> }),

So perhaps this is WAI? Just wanted to flag it as a change I noticed from 5.3.3 to 5.4-beta.

@Andarist
Copy link
Contributor

bisected to #56515

@Andarist
Copy link
Contributor

I can confirm that inferToMappedType is now able to infer more because the constraintType that it receives is a type parameter (T) and not an intersection like before (T & string). The mentioned intersection was reduced to just T as per #56515 so I think this is working as intended.

The NoInfer looks like a good workaround to this issue because that's precisely what it is useful for - it limits the inference sources and #56515 made the second argument a viable inference source here.

@B4nan
Copy link

B4nan commented Jan 31, 2024

I feel like this is the same problem that breaks the inference in MikroORM for the populate hints (and many other things that work on the same basis), it starts to break with 5.4.0-dev.20231121, previously I was able to infer string literals and now I only get string which breaks other types. I'll try to work on a playground when I have some spare time and open a separate issue, but the types are pretty complex so it might take some time.

If this is considered WAI, I am honestly scared... 🀣 The use case on our end is more complex than just validation, we need inference so the types can be used in the return values.

Will work on the repro and open a new issue soon'ish, just wanted to bring this up, as I feel a storm is coming (already got a report about this some time ago).

edit: solved via mikro-orm/mikro-orm#5197

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Feb 1, 2024

The difference here is in the type parameter inference. In 5.4 we get

"foo" | "/:foo/:bar/"

whereas in 5.3 it's only

"/:foo/:bar/"

Generic inference has always been willing, in other cases, to produce a string literal union:

declare function choose<T extends string>(x: T, y: T): T;
// p: "a" | "b"
const p = choose("a" as const, "b" as const);

so it's at least somewhat apparent that the 5.3 behavior is the incorrect (or at least inconsistent) one.

@RyanCavanaugh RyanCavanaugh added the Not a Defect This behavior is one of several equally-correct options label Feb 1, 2024
@typescript-bot
Copy link
Collaborator

This issue has been marked as "Not a Defect" 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 Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not a Defect This behavior is one of several equally-correct options
Projects
None yet
Development

No branches or pull requests

5 participants