FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Remove nonNullUnknownType by jakebailey · Pull Request #57665 · microsoft/TypeScript · GitHub

/ TypeScript Public

Remove nonNullUnknownType - #57665

Merged
Jake Bailey (jakebailey) merged 1 commit into
microsoft:mainfrom
jakebailey:remove-nonNullUnknownType
Mar 15, 2024
ConversationCommits (1)ChecksFiles changed
Merged

Remove nonNullUnknownType#57665
Jake Bailey (jakebailey) merged 1 commit into
microsoft:mainfrom
jakebailey:remove-nonNullUnknownType

Conversation

Jake Bailey (jakebailey) commented Mar 6, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

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

Jake Bailey (jakebailey) commented Mar 6, 2024

Copy link
Copy Markdown
Member Author

TypeScript Bot (@typescript-bot) test top200
TypeScript Bot (@typescript-bot) user test this
TypeScript Bot (@typescript-bot) run dt

TypeScript Bot (@typescript-bot) perf test this faster

TypeScript Bot (typescript-bot) commented Mar 6, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 (typescript-bot) commented Mar 6, 2024

Copy link
Copy Markdown
Contributor

Jake Bailey (@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 (typescript-bot) commented Mar 6, 2024

Copy link
Copy Markdown
Contributor

Hey Jake Bailey (@jakebailey), the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

TypeScript Bot (typescript-bot) commented Mar 6, 2024

Copy link
Copy Markdown
Contributor

Jake Bailey (@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

Bruce Pascoe (fatcerberus) commented Mar 6, 2024

Copy link
Copy Markdown

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

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

Mateusz Burzyński (Andarist) commented Mar 6, 2024

Copy link
Copy Markdown
Contributor

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 (typescript-bot) commented Mar 6, 2024

Copy link
Copy Markdown
Contributor

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

Everything looks good!

Jake Bailey (jakebailey) commented Mar 15, 2024

Copy link
Copy Markdown
Member Author

TypeScript Bot (@typescript-bot) user test this
TypeScript Bot (@typescript-bot) pack this

TypeScript Bot (typescript-bot) commented Mar 15, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 (typescript-bot) commented Mar 15, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hey Jake Bailey (@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".;

Jake Bailey (jakebailey) merged commit 21c7f1c into microsoft:main Mar 15, 2024
Jake Bailey (jakebailey) deleted the remove-nonNullUnknownType branch March 15, 2024 21:58

TypeScript Bot (typescript-bot) commented Mar 15, 2024

Copy link
Copy Markdown
Contributor

Jake Bailey (@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!

Jake Bailey (jakebailey) commented Mar 15, 2024

Copy link
Copy Markdown
Member Author

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

Jake Bailey (jakebailey) mentioned this pull request Mar 15, 2024
Merged
Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 16, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Gabriela Araujo Britto (gabritto) gabritto approved these changes
Wesley Wigham (weswigham) Awaiting requested review from weswigham

Assignees

Jake Bailey (jakebailey)

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL