Skip to content

Remove nonNullUnknownType #57665

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

jakebailey
Copy link
Member

@jakebailey jakebailey commented Mar 6, 2024

As far as I can tell (and test), this unknown variant is no longer needed. Probably due to the changes to how unknown is more or less {} | undefined | null.

This effectively finishes reverting #45575; most of it was removed in #49119 (as expected). By the numbers, #45575 added the declaration of and 5 uses of nonNullUnknownType, #49119 removed 3 references, and this PR removes the 2 remaining references and declaration.

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Mar 6, 2024
@jakebailey
Copy link
Member Author

@typescript-bot test top200
@typescript-bot user test this
@typescript-bot run dt

@typescript-bot perf test this faster

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 6, 2024

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

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

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the user test suite comparing main and refs/pull/57665/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 @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
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
Angular - node (v18.15.0, x64)
Memory used 295,557k (± 0.01%) 295,578k (± 0.01%) ~ 295,540k 295,602k p=0.229 n=6
Parse Time 2.66s (± 0.19%) 2.66s (± 0.19%) ~ 2.66s 2.67s p=1.000 n=6
Bind Time 0.83s (± 0.00%) 0.83s (± 0.66%) ~ 0.82s 0.83s p=0.071 n=6
Check Time 8.23s (± 0.10%) 8.20s (± 0.12%) -0.02s (- 0.28%) 8.19s 8.22s p=0.008 n=6
Emit Time 7.11s (± 0.21%) 7.10s (± 0.29%) ~ 7.09s 7.14s p=0.325 n=6
Total Time 18.83s (± 0.08%) 18.80s (± 0.14%) -0.04s (- 0.19%) 18.77s 18.82s p=0.015 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used 193,340k (± 0.92%) 193,193k (± 0.99%) ~ 191,417k 195,016k p=0.378 n=6
Parse Time 1.36s (± 1.26%) 1.36s (± 0.40%) ~ 1.35s 1.36s p=0.675 n=6
Bind Time 0.72s (± 0.00%) 0.72s (± 0.00%) ~ 0.72s 0.72s p=1.000 n=6
Check Time 9.35s (± 0.42%) 9.36s (± 0.73%) ~ 9.26s 9.44s p=0.688 n=6
Emit Time 2.61s (± 0.42%) 2.61s (± 0.66%) ~ 2.58s 2.63s p=0.672 n=6
Total Time 14.04s (± 0.28%) 14.05s (± 0.50%) ~ 13.96s 14.14s p=0.872 n=6
Monaco - node (v18.15.0, x64)
Memory used 347,368k (± 0.00%) 347,338k (± 0.01%) -31k (- 0.01%) 347,309k 347,358k p=0.020 n=6
Parse Time 2.47s (± 0.33%) 2.48s (± 0.61%) ~ 2.46s 2.49s p=0.930 n=6
Bind Time 0.93s (± 0.00%) 0.93s (± 0.56%) ~ 0.92s 0.93s p=0.174 n=6
Check Time 6.97s (± 0.43%) 6.93s (± 0.34%) ~ 6.91s 6.96s p=0.061 n=6
Emit Time 4.07s (± 0.58%) 4.07s (± 0.57%) ~ 4.04s 4.10s p=1.000 n=6
Total Time 14.45s (± 0.30%) 14.41s (± 0.26%) ~ 14.36s 14.47s p=0.225 n=6
TFS - node (v18.15.0, x64)
Memory used 302,754k (± 0.01%) 302,767k (± 0.00%) ~ 302,754k 302,776k p=0.199 n=6
Parse Time 2.01s (± 0.58%) 2.02s (± 1.97%) ~ 1.95s 2.07s p=0.514 n=6
Bind Time 1.00s (± 1.03%) 1.00s (± 0.41%) ~ 1.00s 1.01s p=0.924 n=6
Check Time 6.36s (± 0.27%) 6.35s (± 0.31%) ~ 6.31s 6.36s p=0.357 n=6
Emit Time 3.60s (± 0.11%) 3.60s (± 0.42%) ~ 3.59s 3.63s p=0.858 n=6
Total Time 12.97s (± 0.17%) 12.97s (± 0.32%) ~ 12.89s 13.01s p=1.000 n=6
material-ui - node (v18.15.0, x64)
Memory used 511,236k (± 0.01%) 511,226k (± 0.01%) ~ 511,181k 511,357k p=0.575 n=6
Parse Time 2.66s (± 0.45%) 2.66s (± 0.65%) ~ 2.63s 2.68s p=0.558 n=6
Bind Time 0.98s (± 0.42%) 0.99s (± 0.90%) ~ 0.98s 1.00s p=0.086 n=6
Check Time 17.25s (± 0.80%) 17.28s (± 0.34%) ~ 17.22s 17.38s p=0.747 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 20.90s (± 0.69%) 20.93s (± 0.29%) ~ 20.85s 21.03s p=0.630 n=6
mui-docs - node (v18.15.0, x64)
Memory used 2,296,328k (± 0.00%) 2,296,330k (± 0.00%) ~ 2,296,306k 2,296,361k p=0.810 n=6
Parse Time 11.93s (± 0.59%) 12.07s (± 0.69%) +0.14s (+ 1.17%) 11.95s 12.15s p=0.031 n=6
Bind Time 2.65s (± 0.46%) 2.65s (± 0.32%) ~ 2.63s 2.65s p=0.867 n=6
Check Time 102.11s (± 0.63%) 102.02s (± 0.54%) ~ 101.34s 102.92s p=1.000 n=6
Emit Time 0.32s (± 1.27%) 0.32s (± 1.60%) ~ 0.32s 0.33s p=0.595 n=6
Total Time 117.01s (± 0.52%) 117.05s (± 0.48%) ~ 116.45s 118.04s p=0.689 n=6
self-build-src - node (v18.15.0, x64)
Memory used 2,394,496k (± 0.01%) 2,394,356k (± 0.01%) ~ 2,394,019k 2,394,530k p=0.378 n=6
Parse Time 5.04s (± 0.40%) 5.07s (± 1.24%) ~ 4.98s 5.15s p=0.295 n=6
Bind Time 1.89s (± 1.13%) 1.88s (± 0.78%) ~ 1.87s 1.91s p=0.741 n=6
Check Time 33.58s (± 0.29%) 33.67s (± 0.22%) ~ 33.60s 33.76s p=0.077 n=6
Emit Time 2.67s (± 1.27%) 2.66s (± 1.05%) ~ 2.62s 2.69s p=0.810 n=6
Total Time 43.20s (± 0.29%) 43.29s (± 0.25%) ~ 43.14s 43.45s p=0.298 n=6
self-compiler - node (v18.15.0, x64)
Memory used 414,407k (± 0.01%) 414,441k (± 0.01%) ~ 414,378k 414,497k p=0.173 n=6
Parse Time 2.81s (± 0.39%) 2.81s (± 1.05%) ~ 2.78s 2.86s p=0.934 n=6
Bind Time 1.06s (± 0.84%) 1.07s (± 0.38%) ~ 1.06s 1.07s p=0.086 n=6
Check Time 15.17s (± 0.53%) 15.17s (± 0.28%) ~ 15.09s 15.21s p=0.873 n=6
Emit Time 1.11s (± 1.79%) 1.11s (± 1.78%) ~ 1.07s 1.12s p=0.742 n=6
Total Time 20.15s (± 0.41%) 20.15s (± 0.33%) ~ 20.06s 20.25s p=0.936 n=6
vscode - node (v18.15.0, x64)
Memory used 2,859,965k (± 0.00%) 2,859,948k (± 0.00%) ~ 2,859,822k 2,859,994k p=0.810 n=6
Parse Time 10.76s (± 0.28%) 10.78s (± 0.49%) ~ 10.71s 10.85s p=0.332 n=6
Bind Time 3.44s (± 0.44%) 3.44s (± 0.30%) ~ 3.42s 3.45s p=0.869 n=6
Check Time 61.04s (± 0.22%) 61.19s (± 0.57%) ~ 60.70s 61.73s p=0.297 n=6
Emit Time 16.28s (± 0.99%) 16.86s (± 8.21%) ~ 16.25s 19.69s p=0.810 n=6
Total Time 91.51s (± 0.24%) 92.26s (± 1.65%) ~ 91.19s 95.28s p=0.298 n=6
webpack - node (v18.15.0, x64)
Memory used 397,000k (± 0.01%) 397,111k (± 0.02%) +111k (+ 0.03%) 397,035k 397,192k p=0.008 n=6
Parse Time 3.14s (± 0.55%) 3.12s (± 0.55%) ~ 3.10s 3.15s p=0.059 n=6
Bind Time 1.37s (± 1.07%) 1.38s (± 0.99%) ~ 1.36s 1.40s p=0.401 n=6
Check Time 13.96s (± 0.34%) 13.98s (± 0.38%) ~ 13.92s 14.05s p=0.520 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 18.47s (± 0.25%) 18.47s (± 0.31%) ~ 18.41s 18.55s p=1.000 n=6
xstate - node (v18.15.0, x64)
Memory used 513,150k (± 0.01%) 513,109k (± 0.01%) ~ 513,070k 513,142k p=0.092 n=6
Parse Time 3.28s (± 0.32%) 3.27s (± 0.42%) ~ 3.25s 3.29s p=1.000 n=6
Bind Time 1.54s (± 0.00%) 1.54s (± 0.64%) ~ 1.53s 1.56s p=1.000 n=6
Check Time 2.86s (± 0.78%) 2.85s (± 0.44%) ~ 2.84s 2.87s p=0.255 n=6
Emit Time 0.08s (± 0.00%) 0.08s (± 0.00%) ~ 0.08s 0.08s p=1.000 n=6
Total Time 7.76s (± 0.28%) 7.75s (± 0.16%) ~ 7.72s 7.75s p=0.392 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)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@fatcerberus
Copy link

Probably due to the changes to how unknown is more or less {} | undefined | null.

Oh no - I hope it's not distributive like boolean 👀

@Andarist
Copy link
Contributor

Andarist commented Mar 6, 2024

Oh no - I hope it's not distributive like boolean 👀

It's not :D although internally such a type kinda exists (unknownUnionType) - I don't think it's particularly special though, it's just a literal internal creation of a union that contains those 3 types

@typescript-bot
Copy link
Collaborator

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

Everything looks good!

@jakebailey
Copy link
Member Author

@typescript-bot user test this
@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 15, 2024

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

Command Status Results
user test this ✅ Started ✅ Results
pack this ✅ Started ✅ Results

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 15, 2024

Hey @jakebailey, 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/160449/artifacts?artifactName=tgz&fileId=EBAF0A0102E43BF68F69F9828C9B1C0EA5D2370B5487E56CA4D2969AA0AEE1CD02&fileName=/typescript-5.5.0-insiders.20240315.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/pr-build@5.5.0-pr-57665-10".;

@jakebailey jakebailey merged commit 21c7f1c into microsoft:main Mar 15, 2024
@jakebailey jakebailey deleted the remove-nonNullUnknownType branch March 15, 2024 21:58
@typescript-bot
Copy link
Collaborator

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

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Unknown failure"

Otherwise...

Everything looks good!

@jakebailey
Copy link
Member Author

(I intentionally merged before the above result; I was just testing that a recent change in the error deltas repo was working.)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants