microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.16k stars 12.38k forks source link

Inference from rest parameters has strange inconsistent results #37193

Open DanielRosenwasser opened 4 years ago

DanielRosenwasser commented 4 years ago
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:

Actual

Playground link

Andarist commented 1 year ago

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 https://github.com/microsoft/TypeScript/issues/46020 and https://github.com/microsoft/TypeScript/issues/44143 (according to my local experiment).

Andarist commented 1 month ago

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. 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
  2. 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 commented 4 weeks ago

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 https://github.com/microsoft/TypeScript/issues/53398