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

Fixed accidental undefined omissions in union props sourced from index types under noUncheckedIndexedAccess #61233

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Andarist
Copy link
Contributor

fixes #61225

@Andarist Andarist force-pushed the fix/missing-type-omission-union-prop branch from 2964f64 to 464abcf Compare February 20, 2025 09:49
@jakebailey
Copy link
Member

@typescript-bot test it

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 20, 2025

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
test top400 ✅ Started ✅ Results
user test this ✅ Started ✅ Results
run dt ✅ Started ✅ Results
perf test this faster ✅ Started 👀 Results

@typescript-bot
Copy link
Collaborator

Hey @jakebailey, the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the user tests with tsc comparing main and refs/pull/61233/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

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

Here they are:

tsc

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-Unions - node (v18.15.0, x64)
Errors 34 34 ~ ~ ~ p=1.000 n=6
Symbols 62,390 62,390 ~ ~ ~ p=1.000 n=6
Types 50,395 50,395 ~ ~ ~ p=1.000 n=6
Memory used 193,518k (± 0.74%) 193,673k (± 0.77%) ~ 192,905k 196,685k p=0.575 n=6
Parse Time 1.30s (± 0.90%) 1.31s (± 0.75%) ~ 1.30s 1.32s p=0.157 n=6
Bind Time 0.73s 0.73s ~ ~ ~ p=1.000 n=6
Check Time 9.73s (± 0.65%) 9.75s (± 0.40%) ~ 9.69s 9.79s p=0.574 n=6
Emit Time 2.73s (± 0.56%) 2.72s (± 0.66%) ~ 2.70s 2.75s p=0.570 n=6
Total Time 14.48s (± 0.52%) 14.51s (± 0.39%) ~ 14.41s 14.56s p=0.629 n=6
angular-1 - node (v18.15.0, x64)
Errors 37 37 ~ ~ ~ p=1.000 n=6
Symbols 948,488 948,488 ~ ~ ~ p=1.000 n=6
Types 411,006 411,006 ~ ~ ~ p=1.000 n=6
Memory used 1,224,187k (± 0.00%) 1,224,188k (± 0.00%) ~ 1,224,105k 1,224,273k p=0.936 n=6
Parse Time 6.64s (± 0.49%) 6.64s (± 0.98%) ~ 6.55s 6.69s p=0.809 n=6
Bind Time 1.88s (± 0.62%) 1.89s (± 0.72%) ~ 1.87s 1.90s p=0.562 n=6
Check Time 31.76s (± 0.16%) 31.81s (± 0.19%) ~ 31.71s 31.89s p=0.126 n=6
Emit Time 15.20s (± 0.43%) 15.24s (± 0.53%) ~ 15.14s 15.35s p=0.521 n=6
Total Time 55.48s (± 0.09%) 55.57s (± 0.24%) ~ 55.36s 55.75s p=0.172 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,347,424 2,347,424 ~ ~ ~ p=1.000 n=6
Types 845,493 845,493 ~ ~ ~ p=1.000 n=6
Memory used 2,120,429k (± 0.00%) 2,120,394k (± 0.00%) ~ 2,120,319k 2,120,543k p=0.471 n=6
Parse Time 7.26s (± 0.19%) 7.27s (± 0.17%) ~ 7.26s 7.29s p=1.000 n=6
Bind Time 2.46s (± 1.08%) 2.46s (± 0.79%) ~ 2.44s 2.49s p=0.452 n=6
Check Time 72.42s (± 1.38%) 72.67s (± 1.17%) ~ 70.96s 73.20s p=0.689 n=6
Emit Time 0.15s (± 5.07%) 0.14s (± 3.60%) ~ 0.14s 0.15s p=0.247 n=6
Total Time 82.28s (± 1.17%) 82.55s (± 0.99%) ~ 80.89s 83.05s p=0.689 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,226,478 1,226,480 +2 (+ 0.00%) ~ ~ p=0.001 n=6
Types 266,963 266,963 ~ ~ ~ p=1.000 n=6
Memory used 2,356,465k (± 0.01%) 2,357,096k (± 0.03%) ~ 2,356,134k 2,357,783k p=0.093 n=6
Parse Time 5.22s (± 1.21%) 5.22s (± 0.51%) ~ 5.18s 5.25s p=0.470 n=6
Bind Time 1.77s (± 0.83%) 1.77s (± 1.45%) ~ 1.74s 1.81s p=1.000 n=6
Check Time 35.26s (± 0.23%) 35.27s (± 0.27%) ~ 35.15s 35.38s p=0.810 n=6
Emit Time 2.98s (± 1.32%) 2.95s (± 0.89%) ~ 2.91s 2.99s p=0.109 n=6
Total Time 45.23s (± 0.25%) 45.24s (± 0.24%) ~ 45.07s 45.35s p=1.000 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,226,478 1,226,480 +2 (+ 0.00%) ~ ~ p=0.001 n=6
Types 266,963 266,963 ~ ~ ~ p=1.000 n=6
Memory used 3,032,329k (± 9.77%) 3,152,667k (± 0.03%) ~ 3,151,674k 3,154,442k p=0.575 n=6
Parse Time 6.95s (± 0.98%) 6.96s (± 0.87%) ~ 6.91s 7.06s p=0.936 n=6
Bind Time 2.14s (± 1.53%) 2.16s (± 1.58%) ~ 2.12s 2.22s p=0.334 n=6
Check Time 42.92s (± 0.49%) 42.91s (± 0.25%) ~ 42.80s 43.06s p=0.936 n=6
Emit Time 3.44s (± 1.79%) 3.53s (± 2.04%) +0.09s (+ 2.52%) 3.41s 3.62s p=0.045 n=6
Total Time 55.45s (± 0.37%) 55.56s (± 0.17%) ~ 55.41s 55.69s p=0.378 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 262,584 262,586 +2 (+ 0.00%) ~ ~ p=0.001 n=6
Types 106,832 106,832 ~ ~ ~ p=1.000 n=6
Memory used 440,369k (± 0.02%) 440,361k (± 0.01%) ~ 440,286k 440,423k p=0.873 n=6
Parse Time 3.52s (± 0.75%) 3.55s (± 1.09%) ~ 3.51s 3.60s p=0.377 n=6
Bind Time 1.32s (± 0.42%) 1.32s (± 0.75%) ~ 1.31s 1.33s p=0.662 n=6
Check Time 18.91s (± 0.31%) 18.87s (± 0.35%) ~ 18.80s 18.97s p=0.377 n=6
Emit Time 1.51s (± 0.90%) 1.51s (± 2.17%) ~ 1.46s 1.54s p=1.000 n=6
Total Time 25.25s (± 0.20%) 25.24s (± 0.46%) ~ 25.12s 25.38s p=0.682 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 70 70 ~ ~ ~ p=1.000 n=6
Symbols 226,113 226,113 ~ ~ ~ p=1.000 n=6
Types 94,488 94,488 ~ ~ ~ p=1.000 n=6
Memory used 371,336k (± 0.05%) 371,310k (± 0.02%) ~ 371,217k 371,371k p=0.688 n=6
Parse Time 2.86s (± 0.87%) 2.89s (± 1.47%) ~ 2.82s 2.95s p=0.196 n=6
Bind Time 1.59s (± 1.77%) 1.59s (± 1.80%) ~ 1.56s 1.63s p=0.572 n=6
Check Time 16.48s (± 0.42%) 16.46s (± 0.45%) ~ 16.33s 16.55s p=0.520 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 20.93s (± 0.38%) 20.95s (± 0.56%) ~ 20.72s 21.02s p=0.375 n=6
vscode - node (v18.15.0, x64)
Errors 1 1 ~ ~ ~ p=1.000 n=6
Symbols 3,227,811 3,227,811 ~ ~ ~ p=1.000 n=6
Types 1,094,612 1,094,612 ~ ~ ~ p=1.000 n=6
Memory used 3,303,920k (± 0.01%) 3,303,995k (± 0.03%) ~ 3,303,198k 3,305,767k p=0.810 n=6
Parse Time 14.35s (± 0.59%) 14.22s (± 0.51%) -0.13s (- 0.91%) 14.08s 14.27s p=0.030 n=6
Bind Time 4.61s (± 0.73%) 4.85s (±11.71%) ~ 4.56s 6.01s p=0.377 n=6
Check Time 89.98s (± 3.16%) 88.74s (± 0.69%) ~ 88.36s 89.95s p=0.873 n=6
Emit Time 27.20s (± 7.38%) 27.73s (± 8.54%) ~ 22.91s 29.09s p=0.230 n=6
Total Time 136.15s (± 0.96%) 135.53s (± 0.94%) ~ 132.95s 136.32s p=0.936 n=6
webpack - node (v18.15.0, x64)
Errors 2 2 ~ ~ ~ p=1.000 n=6
Symbols 293,866 293,866 ~ ~ ~ p=1.000 n=6
Types 119,628 119,628 ~ ~ ~ p=1.000 n=6
Memory used 447,126k (± 0.01%) 447,178k (± 0.03%) ~ 446,981k 447,383k p=0.471 n=6
Parse Time 4.08s (± 1.19%) 4.11s (± 0.97%) ~ 4.05s 4.16s p=0.377 n=6
Bind Time 1.77s (± 1.59%) 1.77s (± 1.07%) ~ 1.75s 1.79s p=0.935 n=6
Check Time 18.74s (± 0.11%) 18.83s (± 0.26%) +0.09s (+ 0.47%) 18.77s 18.90s p=0.005 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 24.58s (± 0.23%) 24.71s (± 0.23%) +0.12s (+ 0.49%) 24.62s 24.79s p=0.013 n=6
xstate-main - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 558,084 558,084 ~ ~ ~ p=1.000 n=6
Types 186,415 186,415 ~ ~ ~ p=1.000 n=6
Memory used 495,085k (± 0.01%) 495,050k (± 0.01%) ~ 495,016k 495,079k p=0.470 n=6
Parse Time 3.43s (± 0.63%) 3.41s (± 0.80%) ~ 3.38s 3.45s p=0.254 n=6
Bind Time 1.19s (± 0.53%) 1.19s (± 1.01%) ~ 1.18s 1.21s p=0.673 n=6
Check Time 19.58s (± 0.28%) 19.58s (± 0.30%) ~ 19.49s 19.65s p=0.936 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 24.19s (± 0.25%) 24.18s (± 0.28%) ~ 24.08s 24.26s p=0.872 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Compiler-Unions - node (v18.15.0, x64)
  • angular-1 - node (v18.15.0, x64)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-build-src-public-api - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • ts-pre-modules - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate-main - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the top 400 repos with tsc comparing main and refs/pull/61233/merge:

Everything looks good!

@@ -15034,6 +15034,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
propTypes.push(type);
}
addRange(propTypes, indexTypes);
if (indexTypes && compilerOptions.noUncheckedIndexedAccess) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe I'm just not familiar with this part of the code, but how can this get away with being unconditionally added when this flag is enabled? Are there code paths above this that are missing this too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The preceding branch requires !indexTypes and this flag is only relevant to existing indexTypes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Not started
Development

Successfully merging this pull request may close these issues.

Unions seem to sidestep --noUncheckedIndexedAccess and lose the possibility of undefined
3 participants