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

Only infer readonly tuples for const type parameters when constraints permit #55229

Merged
merged 10 commits into from
Aug 26, 2023

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Aug 1, 2023

Expands on #55034 to interpret const modifier on type parameters to mean "as const as possible without violating constraints". For example:

declare function fa1<const T extends unknown[]>(args: T): T;
declare function fa2<const T extends readonly unknown[]>(args: T): T;

fa1(["hello", 42]);  // ["hello", 42]
fa2(["hello", 42]);  // readonly ["hello", 42]

Previously the call to fa1 would infer unknown[] because the compiler would first produce the candidate readonly ["hello", 42], but then realize that isn't assignable to unknown[] and therefore go with the unknown[] constraint. With the changes in this PR, the compiler appropriately adjusts its inference to match the constraint.

Fixes #51931.


declare function fn1<const T extends { foo: unknown[] }[]>(...args: T): T;

fn1({ foo: ["hello", 123] }, { foo: [true]});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not so sure about this, seems like an orthogonal thing.

@@ -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);
Copy link
Contributor

@Andarist Andarist Aug 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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");

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this should probably use some(...) to distribute over unions.

@ahejlsberg
Copy link
Member Author

@typescript-bot test this
@typescript-bot user test this inline
@typescript-bot run dt
@typescript-bot perf test faster
@typescript-bot test top100

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 2, 2023

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!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 2, 2023

Heya @ahejlsberg, I've started to run the extended test suite on this PR at f6833cd. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 2, 2023

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!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 2, 2023

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!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 2, 2023

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!

@typescript-bot
Copy link
Collaborator

@ahejlsberg Here are the results of running the user test suite comparing main and refs/pull/55229/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Unknown failure"
  • 1 instance of "Package install failed"

Otherwise...

Something interesting changed - please have a look.

Details

rxjs-src

/mnt/ts_downloads/rxjs-src/build.sh

  • [NEW] error TS2428: All declarations of 'WeakMap' must have identical type parameters.
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
  • [MISSING] error TS2428: All declarations of 'WeakMap' must have identical type parameters.
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)

@typescript-bot
Copy link
Collaborator

@ahejlsberg Here are the results of running the top-repos suite comparing main and refs/pull/55229/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

Hey @ahejlsberg, the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

@ahejlsberg
Copy link
Member Author

@typescript-bot test this
@typescript-bot user test this inline
@typescript-bot run dt
@typescript-bot perf test faster
@typescript-bot test top100

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 3, 2023

Heya @ahejlsberg, I've started to run the extended test suite on this PR at f1104cc. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 3, 2023

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!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 3, 2023

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!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 3, 2023

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!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 3, 2023

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!

@typescript-bot
Copy link
Collaborator

@ahejlsberg Here are the results of running the user test suite comparing main and refs/pull/55229/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Unknown failure"
  • 1 instance of "Package install failed"

Otherwise...

Something interesting changed - please have a look.

Details

rxjs-src

/mnt/ts_downloads/rxjs-src/build.sh

  • [NEW] error TS2428: All declarations of 'WeakMap' must have identical type parameters.
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
  • [MISSING] error TS2428: All declarations of 'WeakMap' must have identical type parameters.
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(63,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)

@typescript-bot
Copy link
Collaborator

@ahejlsberg Here are the results of running the top-repos suite comparing main and refs/pull/55229/merge:

Everything looks good!

@ahejlsberg
Copy link
Member Author

@typescript-bot test this
@typescript-bot user test this inline
@typescript-bot run dt
@typescript-bot perf test faster
@typescript-bot test top100

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 24, 2023

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!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 24, 2023

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!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 24, 2023

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!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 24, 2023

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!

@typescript-bot
Copy link
Collaborator

@ahejlsberg
The results of the perf run you requested are in!

Here they are:

Compiler

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Angular - node (v16.17.1, x64)
Memory used 300,232k (± 0.00%) 300,255k (± 0.01%) +23k (+ 0.01%) 300,234k 300,281k p=0.045 n=6
Parse Time 3.03s (± 0.25%) 3.03s (± 0.21%) ~ 3.02s 3.04s p=0.718 n=6
Bind Time 0.93s (± 0.00%) 0.93s (± 0.00%) ~ 0.93s 0.93s p=1.000 n=6
Check Time 9.31s (± 0.43%) 9.29s (± 0.18%) ~ 9.26s 9.30s p=0.324 n=6
Emit Time 7.63s (± 0.28%) 7.65s (± 0.31%) ~ 7.61s 7.67s p=0.124 n=6
Total Time 20.89s (± 0.16%) 20.89s (± 0.19%) ~ 20.83s 20.95s p=1.000 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used 194,893k (± 0.75%) 194,896k (± 0.74%) ~ 193,949k 197,088k p=0.575 n=6
Parse Time 1.58s (± 0.26%) 1.58s (± 0.33%) ~ 1.58s 1.59s p=0.595 n=6
Bind Time 0.80s (± 0.51%) 0.80s (± 0.51%) ~ 0.79s 0.80s p=1.000 n=6
Check Time 9.91s (± 0.16%) 9.93s (± 0.43%) ~ 9.86s 9.99s p=0.126 n=6
Emit Time 2.74s (± 0.30%) 2.74s (± 0.36%) ~ 2.72s 2.75s p=0.604 n=6
Total Time 15.02s (± 0.14%) 15.06s (± 0.30%) ~ 14.98s 15.10s p=0.170 n=6
Monaco - node (v16.17.1, x64)
Memory used 347,128k (± 0.01%) 347,152k (± 0.01%) ~ 347,096k 347,177k p=0.335 n=6
Parse Time 2.69s (± 0.20%) 2.69s (± 0.28%) ~ 2.68s 2.70s p=0.476 n=6
Bind Time 0.99s (± 0.41%) 0.99s (± 0.00%) ~ 0.99s 0.99s p=0.405 n=6
Check Time 7.91s (± 0.13%) 7.90s (± 0.26%) ~ 7.87s 7.93s p=0.279 n=6
Emit Time 4.25s (± 0.26%) 4.26s (± 0.25%) +0.01s (+ 0.35%) 4.25s 4.28s p=0.048 n=6
Total Time 15.84s (± 0.11%) 15.84s (± 0.19%) ~ 15.80s 15.89s p=0.685 n=6
TFS - node (v16.17.1, x64)
Memory used 301,144k (± 0.00%) 301,161k (± 0.00%) +18k (+ 0.01%) 301,145k 301,180k p=0.020 n=6
Parse Time 2.18s (± 0.82%) 2.17s (± 0.58%) ~ 2.16s 2.19s p=0.357 n=6
Bind Time 1.12s (± 0.75%) 1.11s (± 0.89%) ~ 1.09s 1.12s p=0.340 n=6
Check Time 7.22s (± 0.33%) 7.20s (± 0.21%) ~ 7.18s 7.22s p=0.254 n=6
Emit Time 3.99s (± 0.49%) 3.99s (± 0.49%) ~ 3.97s 4.02s p=0.607 n=6
Total Time 14.50s (± 0.17%) 14.48s (± 0.18%) ~ 14.44s 14.51s p=0.288 n=6
material-ui - node (v16.17.1, x64)
Memory used 479,446k (± 0.00%) 479,451k (± 0.00%) ~ 479,429k 479,467k p=0.415 n=6
Parse Time 3.15s (± 0.16%) 3.15s (± 0.24%) ~ 3.14s 3.16s p=0.784 n=6
Bind Time 0.91s (± 0.00%) 0.91s (± 0.00%) ~ 0.91s 0.91s p=1.000 n=6
Check Time 17.80s (± 0.19%) 17.78s (± 0.26%) ~ 17.73s 17.86s p=0.261 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 21.86s (± 0.15%) 21.84s (± 0.23%) ~ 21.79s 21.92s p=0.376 n=6
xstate - node (v16.17.1, x64)
Memory used 542,842k (± 0.01%) 542,841k (± 0.01%) ~ 542,769k 542,976k p=0.936 n=6
Parse Time 3.70s (± 0.27%) 3.71s (± 0.23%) ~ 3.70s 3.72s p=0.734 n=6
Bind Time 1.34s (± 0.39%) 1.34s (± 0.30%) ~ 1.33s 1.34s p=0.595 n=6
Check Time 3.32s (± 0.45%) 3.33s (± 0.58%) ~ 3.30s 3.35s p=0.323 n=6
Emit Time 0.08s (± 0.00%) 0.08s (± 0.00%) ~ 0.08s 0.08s p=1.000 n=6
Total Time 8.44s (± 0.26%) 8.45s (± 0.26%) ~ 8.42s 8.47s p=0.416 n=6
System info unknown
Hosts
  • node (v16.17.1, x64)
Scenarios
  • Angular - node (v16.17.1, x64)
  • Compiler-Unions - node (v16.17.1, x64)
  • Monaco - node (v16.17.1, x64)
  • TFS - node (v16.17.1, x64)
  • material-ui - node (v16.17.1, x64)
  • xstate - node (v16.17.1, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
Collaborator

@ahejlsberg Here are the results of running the user test suite comparing main and refs/pull/55229/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Unknown failure"
  • 2 instances of "Package install failed"

Otherwise...

Something interesting changed - please have a look.

Details

rxjs-src

/mnt/ts_downloads/rxjs-src/build.sh

  • [NEW] error TS2428: All declarations of 'WeakMap' must have identical type parameters.
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-55229/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
  • [MISSING] error TS2428: All declarations of 'WeakMap' must have identical type parameters.
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.collection.d.ts(62,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.iterable.d.ts(162,11)
    • /home/vsts/work/1/s/typescript-main/lib/lib.es2015.symbol.wellknown.d.ts(140,11)

@typescript-bot
Copy link
Collaborator

@ahejlsberg Here are the results of running the top-repos suite comparing main and refs/pull/55229/merge:

Something interesting changed - please have a look.

Details

vercel/swr

5 of 13 projects failed to build with the old tsc and were ignored

test/type/tsconfig.json

@typescript-bot
Copy link
Collaborator

Hey @ahejlsberg, the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

@typescript-bot typescript-bot added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Aug 26, 2023
@ahejlsberg
Copy link
Member Author

Tests and performance are unaffected except for the new errors in vercel/swr. The new errors occur in circumstances similar to

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 [...] as const array literal is contextually typed by a type parameter with a constraint that includes a mutable array type. Previously we'd always infer a readonly array type (which often would fail to be assignable to the constraint). The mutable array type is a more specific type than the readonly array type, so this shouldn't really cause issues. However, the vercel/swr code checks for exact type matches, thus any change is a break.

The simple fix is to change the vercel/swr code here to just

type ArgumentsTuple = readonly [any, ...unknown[]]

The mutable tuple in the current definition is redundant anyway.

@ahejlsberg ahejlsberg merged commit 753c463 into main Aug 26, 2023
19 checks passed
PR Backlog automation moved this from Needs merge to Done Aug 26, 2023
@ahejlsberg ahejlsberg deleted the fix55034 branch August 26, 2023 15:20
@rauchg
Copy link

rauchg commented Aug 26, 2023

Thanks @ahejlsberg!

@t7yang
Copy link

t7yang commented Aug 27, 2023

but the const keyword would be a little bit confuse.

on RHS

const foo = [1] as const; // apply readonly

on LHS (I consider function parameter as LHS)

function bar<const T extends unknown[]>(a: T): T // not apply readonly

but if we use satisfies instead of const on LHS, semantics will be more consistent.

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

@shuding
Copy link

shuding commented Aug 27, 2023

That type was updated in vercel/swr v2.2.2. Thanks @ahejlsberg (and @Andarist for opening the PR)!

snovader pushed a commit to mestro-se/TypeScript that referenced this pull request Sep 23, 2023
…ts permit (microsoft#55229)

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
@jcalz
Copy link
Contributor

jcalz commented Dec 11, 2023

Oh, this is great! It's too bad that this didn't make it into the TS5.3 Release Notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
Archived in project
PR Backlog
  
Done
Development

Successfully merging this pull request may close these issues.

const type parameters don't work with mutable array constraints
9 participants