Skip to content

Inference from rest parameters has strange inconsistent results #37193

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

Open
DanielRosenwasser opened this issue Mar 3, 2020 · 3 comments
Open
Labels
Needs Investigation This issue needs a team member to investigate its status.
Milestone

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Mar 3, 2020

declare function foo(...x: readonly any[]): any

type FooParams = Parameters<typeof foo>;

declare function bar(...x: readonly number[]): any

type BarParams = Parameters<typeof bar>

Expected:

  • FooParams is any[] or readonly any[]
  • BarParams is number[] or readonly number[]

Actual

  • FooParams is unknown[]
  • BarParams is never

Playground link

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Mar 4, 2020
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.9.1 milestone Mar 4, 2020
@RyanCavanaugh RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label May 20, 2020
@Andarist
Copy link
Contributor

Andarist commented Mar 6, 2023

This might be a hard thing to fix unless you would be OK with some breakage. The problem is in the getInferredTypeParameterConstraint:
https://github.dev/microsoft/TypeScript/blob/43cc362cef72e5fa1372f59736a9c4b55d85def0/src/compiler/checker.ts#L14949-L14955

Because of this we end up checking if readonly number[] is assignable to unknown[] in the getInferredType:
https://github.dev/microsoft/TypeScript/blob/43cc362cef72e5fa1372f59736a9c4b55d85def0/src/compiler/checker.ts#L25022-L25028

Since this isn't assignable we pick the constraint over the inferred type.

It's not easy to change this inferred constraint to readonly unknown[] (nor to readonly unknown[] | unknown[]) since people might pass this infer type parameter to other types in places where any[]/unknown[] is expected. I find it usually better to use readonly unknown[] over unknown[] for my constraints - it's better since usually I was to allow both mutable and readonly arrays. However, not everybody does that (and I'm often lazy too, and don't include the readonly modifier ;p). So changing that would likely break a lot of types.

Alternatively, getInferredType could have some heuristics to ignore the inferred constraint but I don't know what that would look like exactly. I experimented with a couple of things and actually got some promising results for this. A couple of branches in getInferredTypeParameterConstraint are just matching things syntactically. Maybe there is a difference between a constraint that has to be used for substitution types and the one that has to be tested against in getInferredType?

Note that I would consider those to have a semantic and not a syntactic match (and this case is also inferrable through getInferredTypeParameterConstraint):

type Foo<T extends string, U extends T> = [T, U];
type Bar<T> = T extends Foo<infer X, infer Y> ? Foo<X, Y> : never;

Using this "prefer inferred type over inferred syntactical constraint" logic could also fix #46020 and #44143 (according to my local experiment).

@RyanCavanaugh RyanCavanaugh removed the Rescheduled This issue was previously scheduled to an earlier milestone label Mar 6, 2023
@Andarist
Copy link
Contributor

Andarist commented Aug 14, 2024

I'm one year more experienced now 😉 and I don't see how this can be fixed without breaking something.

The problem is that the implied constraint of those rest params is unknown[] so naturally an inferred result of readonly any[] doesn't satisfy it. On such an occasion the constraint is picked as the inferred type. This results in an unexpected behavior for the user.

What can be done about it? I see 2 possible solutions.

  1. change the implied constraint for rest params to readonly unknown[]. This would allow both mutable and readonly arrays. It sounds good, right? The problem is that now this code would fail:
type AcceptArr<T extends unknown[]> = T
type A<T> = T extends (...rest: infer P) => any ? AcceptArr<P> : never
  1. allow readonly arrays to satisfy this constraint check (by introducing a special internal type). This would result in problems at instantiation time because the type parameter's constraint within the conditional type would still be unknown[] but yet then that type could be instantiated with a type that doesn't satisfy that constraint

I think only option 1 is viable here but it's a breaking change.

@Andarist
Copy link
Contributor

There is also an option number 3. Rest parameters could strip readonly-ness from instantiations. This would feel consistent with rests in tuples:

type Test<T extends readonly unknown[]> = T extends readonly [...infer R] ? R : never;

type Result1 = Test<readonly any[]>;
//   ^? type Result1 = any[]
type Result2 = Test<readonly number[]>;
//   ^? type Result2 = number[]

It would also be consistent with the proposed fix for #53398

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

4 participants