Skip to content

Keep returned (and yielded) literal types as const when their types using const type variables #56859

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
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Andarist
Copy link
Contributor

@Andarist Andarist commented Dec 23, 2023

fixes #56858
closes #53813

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Dec 23, 2023
@typescript-bot
Copy link
Collaborator

The TypeScript team hasn't accepted the linked issue #53813. If you can get it accepted, this PR will have a better chance of being reviewed.

@sandersn sandersn requested a review from ahejlsberg January 3, 2024 14:32
@typescript-bot typescript-bot removed the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jan 3, 2024
@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Jan 3, 2024
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

The change seems sensible but we should probably review it at a design meeting.

@sandersn
Copy link
Member

sandersn commented Jan 5, 2024

My notes from the design meeting:

This improves the apparent consistency so much that it's worth taking. The implementation needs to be right of course.

To check that:

  • run all test suites,
  • test multiply-nested cases
  • test overloads
  • test async functions, generators, async generators

Also decide whether this is the right place to implement it; could be that it's better to put it in getWidenedType. (@weswigham brought this concern up but seemed to be believe that its current place is better.)

@sandersn
Copy link
Member

sandersn commented Jan 5, 2024

@typescript-bot test top100
@typescript-bot user test this
@typescript-bot run dt
@typescript-bot perf test this
@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 5, 2024

Heya @sandersn, I've started to run the tarball bundle task on this PR at 78831c2. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 5, 2024

Heya @sandersn, I've started to run the diff-based top-repos suite on this PR at 78831c2. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 5, 2024

Heya @sandersn, I've started to run the parallelized Definitely Typed test suite on this PR at 78831c2. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 5, 2024

Heya @sandersn, I've started to run the regular perf test suite on this PR at 78831c2. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 5, 2024

Heya @sandersn, I've started to run the diff-based user code test suite on this PR at 78831c2. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 5, 2024

Hey @sandersn, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/159283/artifacts?artifactName=tgz&fileId=CCBB4994699BAF8077FFA5F10F4D9FE2A4243C14A9540A067A3112B12804D78702&fileName=/typescript-5.4.0-insiders.20240105.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

@sandersn sandersn assigned weswigham and unassigned ahejlsberg Jan 5, 2024
@sandersn sandersn requested review from weswigham and removed request for ahejlsberg January 5, 2024 19:31
@typescript-bot
Copy link
Collaborator

@sandersn
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 (v18.15.0, x64)
Memory used 295,476k (± 0.01%) 295,467k (± 0.01%) ~ 295,424k 295,517k p=0.810 n=6
Parse Time 2.65s (± 0.19%) 2.65s (± 0.40%) ~ 2.63s 2.66s p=0.794 n=6
Bind Time 0.82s (± 0.50%) 0.82s (± 0.50%) ~ 0.82s 0.83s p=1.000 n=6
Check Time 8.17s (± 0.26%) 8.16s (± 0.10%) ~ 8.15s 8.17s p=0.565 n=6
Emit Time 7.11s (± 0.21%) 7.11s (± 0.29%) ~ 7.08s 7.14s p=0.745 n=6
Total Time 18.75s (± 0.12%) 18.73s (± 0.12%) ~ 18.72s 18.78s p=0.216 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used 191,523k (± 0.02%) 192,472k (± 1.23%) ~ 191,414k 197,303k p=0.810 n=6
Parse Time 1.36s (± 0.77%) 1.35s (± 1.69%) ~ 1.31s 1.38s p=0.802 n=6
Bind Time 0.72s (± 0.00%) 0.72s (± 0.00%) ~ 0.72s 0.72s p=1.000 n=6
Check Time 9.25s (± 0.28%) 9.25s (± 0.26%) ~ 9.21s 9.27s p=0.935 n=6
Emit Time 2.62s (± 0.24%) 2.61s (± 0.45%) ~ 2.60s 2.63s p=0.068 n=6
Total Time 13.94s (± 0.22%) 13.92s (± 0.38%) ~ 13.83s 13.99s p=0.810 n=6
Monaco - node (v18.15.0, x64)
Memory used 347,397k (± 0.00%) 347,419k (± 0.01%) ~ 347,369k 347,453k p=0.090 n=6
Parse Time 2.46s (± 0.31%) 2.46s (± 0.54%) ~ 2.44s 2.48s p=0.548 n=6
Bind Time 0.93s (± 0.56%) 0.92s (± 0.56%) ~ 0.92s 0.93s p=0.311 n=6
Check Time 6.88s (± 0.35%) 6.90s (± 0.47%) ~ 6.85s 6.93s p=0.170 n=6
Emit Time 4.05s (± 0.37%) 4.04s (± 0.34%) ~ 4.03s 4.07s p=0.242 n=6
Total Time 14.31s (± 0.26%) 14.32s (± 0.29%) ~ 14.27s 14.38s p=0.683 n=6
TFS - node (v18.15.0, x64)
Memory used 302,737k (± 0.01%) 302,718k (± 0.00%) -19k (- 0.01%) 302,700k 302,737k p=0.045 n=6
Parse Time 2.01s (± 1.08%) 2.00s (± 0.92%) ~ 1.98s 2.02s p=0.459 n=6
Bind Time 1.00s (± 1.03%) 1.00s (± 1.09%) ~ 0.99s 1.02s p=0.546 n=6
Check Time 6.28s (± 0.55%) 6.28s (± 0.19%) ~ 6.27s 6.30s p=0.568 n=6
Emit Time 3.59s (± 0.62%) 3.57s (± 0.40%) ~ 3.55s 3.59s p=0.139 n=6
Total Time 12.88s (± 0.29%) 12.85s (± 0.29%) ~ 12.81s 12.92s p=0.072 n=6
material-ui - node (v18.15.0, x64)
Memory used 506,810k (± 0.00%) 506,791k (± 0.00%) ~ 506,764k 506,817k p=0.230 n=6
Parse Time 2.59s (± 0.63%) 2.58s (± 0.53%) ~ 2.57s 2.61s p=0.867 n=6
Bind Time 0.99s (± 0.00%) 0.99s (± 0.76%) ~ 0.98s 1.00s p=0.598 n=6
Check Time 16.93s (± 0.40%) 16.95s (± 0.31%) ~ 16.85s 17.00s p=0.368 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 20.51s (± 0.36%) 20.52s (± 0.20%) ~ 20.45s 20.57s p=0.748 n=6
xstate - node (v18.15.0, x64)
Memory used 512,865k (± 0.01%) 512,795k (± 0.01%) -69k (- 0.01%) 512,730k 512,865k p=0.031 n=6
Parse Time 3.27s (± 0.19%) 3.27s (± 0.27%) ~ 3.26s 3.28s p=1.000 n=6
Bind Time 1.54s (± 0.53%) 1.54s (± 0.36%) ~ 1.53s 1.54s p=0.859 n=6
Check Time 2.82s (± 1.08%) 2.81s (± 0.76%) ~ 2.77s 2.83s p=0.519 n=6
Emit Time 0.07s (± 5.69%) 0.07s (± 0.00%) ~ 0.07s 0.07s p=0.405 n=6
Total Time 7.71s (± 0.44%) 7.69s (± 0.40%) ~ 7.64s 7.73s p=0.630 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Angular - node (v18.15.0, x64)
  • Compiler-Unions - node (v18.15.0, x64)
  • Monaco - node (v18.15.0, x64)
  • TFS - node (v18.15.0, x64)
  • material-ui - node (v18.15.0, x64)
  • xstate - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

tsserver

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-UnionsTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 2,338ms (± 0.33%) 2,346ms (± 0.84%) ~ 2,327ms 2,378ms p=0.748 n=6
Req 2 - geterr 5,393ms (± 1.11%) 5,401ms (± 0.99%) ~ 5,366ms 5,506ms p=0.521 n=6
Req 3 - references 326ms (± 1.28%) 325ms (± 1.05%) ~ 322ms 331ms p=0.747 n=6
Req 4 - navto 276ms (± 1.14%) 277ms (± 1.01%) ~ 271ms 278ms p=0.652 n=6
Req 5 - completionInfo count 1,356 (± 0.00%) 1,356 (± 0.00%) ~ 1,356 1,356 p=1.000 n=6
Req 5 - completionInfo 86ms (± 5.64%) 85ms (± 4.64%) ~ 83ms 93ms p=1.000 n=6
CompilerTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 2,483ms (± 0.93%) 2,486ms (± 0.65%) ~ 2,461ms 2,500ms p=0.630 n=6
Req 2 - geterr 4,138ms (± 1.99%) 4,190ms (± 1.91%) ~ 4,084ms 4,254ms p=0.298 n=6
Req 3 - references 338ms (± 1.82%) 337ms (± 1.04%) ~ 333ms 341ms p=1.000 n=6
Req 4 - navto 285ms (± 0.88%) 284ms (± 0.37%) ~ 282ms 285ms p=0.139 n=6
Req 5 - completionInfo count 1,518 (± 0.00%) 1,518 (± 0.00%) ~ 1,518 1,518 p=1.000 n=6
Req 5 - completionInfo 84ms (± 6.80%) 85ms (± 6.44%) ~ 78ms 90ms p=0.466 n=6
xstateTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 2,604ms (± 0.88%) 2,599ms (± 0.69%) ~ 2,575ms 2,617ms p=0.376 n=6
Req 2 - geterr 1,728ms (± 2.64%) 1,721ms (± 1.55%) ~ 1,689ms 1,762ms p=0.689 n=6
Req 3 - references 115ms (± 9.20%) 113ms (±10.21%) ~ 102ms 123ms p=1.000 n=6
Req 4 - navto 367ms (± 1.21%) 365ms (± 0.83%) ~ 361ms 369ms p=0.361 n=6
Req 5 - completionInfo count 2,073 (± 0.00%) 2,073 (± 0.00%) ~ 2,073 2,073 p=1.000 n=6
Req 5 - completionInfo 310ms (± 1.88%) 310ms (± 1.27%) ~ 303ms 314ms p=0.574 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • CompilerTSServer - node (v18.15.0, x64)
  • Compiler-UnionsTSServer - node (v18.15.0, x64)
  • xstateTSServer - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Startup

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
tsc-startup - node (v18.15.0, x64)
Execution time 153.24ms (± 0.21%) 153.09ms (± 0.18%) -0.14ms (- 0.09%) 152.01ms 155.15ms p=0.000 n=600
tsserver-startup - node (v18.15.0, x64)
Execution time 229.09ms (± 0.18%) 228.88ms (± 0.16%) -0.21ms (- 0.09%) 227.63ms 234.32ms p=0.000 n=600
tsserverlibrary-startup - node (v18.15.0, x64)
Execution time 230.73ms (± 0.21%) 230.71ms (± 0.20%) ~ 228.64ms 241.90ms p=0.848 n=600
typescript-startup - node (v18.15.0, x64)
Execution time 230.19ms (± 0.19%) 230.09ms (± 0.18%) ~ 228.52ms 233.27ms p=0.075 n=600
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • tsc-startup - node (v18.15.0, x64)
  • tsserver-startup - node (v18.15.0, x64)
  • tsserverlibrary-startup - node (v18.15.0, x64)
  • typescript-startup - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
Collaborator

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

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Package install failed"

Otherwise...

Something interesting changed - please have a look.

Details

puppeteer

packages/browsers/test/src/tsconfig.json

@typescript-bot
Copy link
Collaborator

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

@typescript-bot
Copy link
Collaborator

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

Everything looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
Status: Waiting on author
5 participants