Only infer readonly tuples for const type parameters when constraints permit
#55229
Conversation
typescript-bot
added
Author: Team
For Uncommitted Bug
|
|
||
| declare function fn1<const T extends { foo: unknown[] }[]>(...args: T): T; | ||
|
|
||
| fn1({ foo: ["hello", 123] }, { foo: [true]}); |
There was a problem hiding this comment.
maybe worth adding some weirdo test based on reverse mapped types as well?
declare function invoke<
const T extends readonly { key: string }[],
const T2 extends {
[K in keyof T]: T[K]["key"];
}
>(f: (...args: T2) => void, ...args: T): T2;
const res = invoke((a, b) => {}, { key: "A" }, { key: "B" });This works fine with the changes here but it's more esoteric and could not work if different internal checks would be used or something
There was a problem hiding this comment.
Not so sure about this, seems like an orthogonal thing.
src/compiler/checker.ts
Outdated
| @@ -23925,7 +23925,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
| callback(getTypeAtPosition(source, i), getTypeAtPosition(target, i)); | |||
| } | |||
| if (targetRestType) { | |||
| callback(getRestTypeAtPosition(source, paramCount), targetRestType); | |||
| callback(getRestTypeAtPosition(source, paramCount, /*readonly*/ isConstTypeVariable(targetRestType) && !isMutableArrayOrTuple(getBaseConstraintOrType(targetRestType))), targetRestType); | |||
There was a problem hiding this comment.
IIUC this won't work correctly with union constraints, for example:
declare function invoke<const T extends [string, unknown] | [unknown, string]>(
f: (...args: T) => void,
...args: T
): T;
const res = invoke((a: string, b: string) => {}, "hello", "world");There was a problem hiding this comment.
Yeah, this should probably use some(...) to distribute over unions.
|
@typescript-bot test this |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at f6833cd. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the extended test suite on this PR at f6833cd. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at f6833cd. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at f6833cd. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the diff-based top-repos suite on this PR at f6833cd. You can monitor the build here. Update: The results are in! |
|
@ahejlsberg Here are the results of running the user test suite comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Something interesting changed - please have a look. Details
|
|
@ahejlsberg Here are the results of running the top-repos suite comparing Everything looks good! |
|
Hey @ahejlsberg, the results of running the DT tests are ready. |
|
@typescript-bot test this |
|
Heya @ahejlsberg, I've started to run the extended test suite on this PR at f1104cc. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at f1104cc. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at f1104cc. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at f1104cc. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the diff-based top-repos suite on this PR at f1104cc. You can monitor the build here. Update: The results are in! |
|
@ahejlsberg Here are the results of running the user test suite comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Something interesting changed - please have a look. Details
|
|
@ahejlsberg Here are the results of running the top-repos suite comparing Everything looks good! |
|
@typescript-bot test this |
|
Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at 30281ac. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 30281ac. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the tsc-only perf test suite on this PR at 30281ac. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the diff-based top-repos suite on this PR at 30281ac. You can monitor the build here. Update: The results are in! |
|
@ahejlsberg Here they are:
CompilerComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@ahejlsberg Here are the results of running the user test suite comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Something interesting changed - please have a look. Details
|
|
@ahejlsberg Here are the results of running the top-repos suite comparing Something interesting changed - please have a look. Details
|
|
Hey @ahejlsberg, the results of running the DT tests are ready. |
typescript-bot
added
For Milestone Bug
|
Tests and performance are unaffected except for the new errors in declare function foo<T extends unknown[] | readonly unknown[]>(args: T): T;
foo(["hello", "world"] as const); // ["hello", "world"]We now infer a mutable array type when an The simple fix is to change the type ArgumentsTuple = readonly [any, ...unknown[]]The mutable tuple in the current definition is redundant anyway. |
|
Thanks @ahejlsberg! |
|
but the on RHS const foo = [1] as const; // apply readonlyon LHS (I consider function parameter as LHS) function bar<const T extends unknown[]>(a: T): T // not apply readonlybut if we use const foo = [1] satisfies unknown[]; // doesn't apply readonly
function bar<satisfies T extends unknown[]>(a: T): T // doesn't apply readonly
const foo = [1] as const satisfies unknown[]>; // okay, now apply readonly
function bar<satisfies T extends readonly unknown[]>(a: T): T // okay, now apply readonly |
|
That type was updated in |
…ts permit (microsoft#55229) Co-authored-by: Mateusz Burzyński <[email protected]>
|
Oh, this is great! It's too bad that this didn't make it into the TS5.3 Release Notes. |
Expands on #55034 to interpret
constmodifier on type parameters to mean "as const as possible without violating constraints". For example:Previously the call to
fa1would inferunknown[]because the compiler would first produce the candidatereadonly ["hello", 42], but then realize that isn't assignable tounknown[]and therefore go with theunknown[]constraint. With the changes in this PR, the compiler appropriately adjusts its inference to match the constraint.Fixes #51931.