Skip to content

Conversation

Zzzen
Copy link
Contributor

@Zzzen Zzzen commented Sep 3, 2023

Fixes #34974

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Sep 3, 2023
@jakebailey
Copy link
Member

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 3, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 3, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 3, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 3, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

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

There were infrastructure failures potentially unrelated to your change:

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

Otherwise...

Everything looks good!

@typescript-bot
Copy link
Collaborator

@jakebailey
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,271k (± 0.01%) 300,263k (± 0.01%) ~ 300,246k 300,305k p=0.336 n=6
Parse Time 3.01s (± 0.27%) 3.01s (± 0.28%) ~ 3.00s 3.02s 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.33s (± 0.24%) 9.31s (± 0.23%) ~ 9.28s 9.34s p=0.164 n=6
Emit Time 7.63s (± 0.20%) 7.62s (± 0.27%) ~ 7.59s 7.64s p=0.324 n=6
Total Time 20.90s (± 0.19%) 20.87s (± 0.13%) ~ 20.84s 20.90s p=0.145 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used 193,923k (± 0.02%) 193,939k (± 0.02%) ~ 193,860k 193,988k p=0.689 n=6
Parse Time 1.58s (± 0.00%) 1.58s (± 0.00%) ~ 1.58s 1.58s p=1.000 n=6
Bind Time 0.79s (± 0.00%) 0.80s (± 0.65%) +0.01s (+ 0.84%) 0.79s 0.80s p=0.025 n=6
Check Time 9.92s (± 0.26%) 9.95s (± 0.50%) ~ 9.88s 10.03s p=0.145 n=6
Emit Time 2.73s (± 0.31%) 2.74s (± 0.36%) ~ 2.73s 2.75s p=0.734 n=6
Total Time 15.03s (± 0.23%) 15.06s (± 0.26%) ~ 15.01s 15.13s p=0.107 n=6
Monaco - node (v16.17.1, x64)
Memory used 347,199k (± 0.00%) 347,205k (± 0.00%) ~ 347,191k 347,229k p=0.199 n=6
Parse Time 2.69s (± 0.38%) 2.69s (± 0.33%) ~ 2.68s 2.70s p=0.673 n=6
Bind Time 0.99s (± 0.00%) 0.99s (± 0.00%) ~ 0.99s 0.99s p=1.000 n=6
Check Time 7.92s (± 0.22%) 7.93s (± 0.25%) ~ 7.90s 7.96s p=0.567 n=6
Emit Time 4.27s (± 0.12%) 4.25s (± 0.44%) ~ 4.23s 4.28s p=0.079 n=6
Total Time 15.86s (± 0.17%) 15.86s (± 0.11%) ~ 15.83s 15.88s p=0.627 n=6
TFS - node (v16.17.1, x64)
Memory used 301,174k (± 0.00%) 301,176k (± 0.00%) ~ 301,157k 301,195k p=1.000 n=6
Parse Time 2.17s (± 0.56%) 2.16s (± 0.63%) ~ 2.15s 2.19s p=0.170 n=6
Bind Time 1.09s (± 5.33%) 1.11s (± 0.37%) ~ 1.11s 1.12s p=0.673 n=6
Check Time 7.25s (± 0.56%) 7.25s (± 0.23%) ~ 7.22s 7.26s p=1.000 n=6
Emit Time 3.98s (± 0.40%) 3.98s (± 0.40%) ~ 3.97s 4.01s p=0.675 n=6
Total Time 14.49s (± 0.75%) 14.50s (± 0.23%) ~ 14.46s 14.55s p=0.376 n=6
material-ui - node (v16.17.1, x64)
Memory used 479,470k (± 0.00%) 479,469k (± 0.00%) ~ 479,448k 479,483k p=1.000 n=6
Parse Time 3.15s (± 0.16%) 3.15s (± 0.31%) ~ 3.14s 3.17s p=0.386 n=6
Bind Time 0.91s (± 0.00%) 0.91s (± 0.00%) ~ 0.91s 0.91s p=1.000 n=6
Check Time 17.77s (± 0.19%) 17.84s (± 0.34%) ~ 17.77s 17.91s p=0.065 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 21.82s (± 0.16%) 21.90s (± 0.25%) +0.07s (+ 0.34%) 21.84s 21.96s p=0.034 n=6
xstate - node (v16.17.1, x64)
Memory used 542,826k (± 0.01%) 542,851k (± 0.01%) ~ 542,804k 542,925k p=0.936 n=6
Parse Time 3.69s (± 0.22%) 3.70s (± 0.20%) ~ 3.69s 3.71s p=0.383 n=6
Bind Time 1.40s (± 4.59%) 1.38s (± 4.42%) ~ 1.33s 1.46s p=0.801 n=6
Check Time 3.25s (± 2.68%) 3.28s (± 2.55%) ~ 3.16s 3.34s p=0.746 n=6
Emit Time 0.08s (± 4.99%) 0.08s (± 0.00%) ~ 0.08s 0.08s p=0.405 n=6
Total Time 8.42s (± 0.44%) 8.43s (± 0.34%) ~ 8.38s 8.46s p=0.376 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

tsserver

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-UnionsTSServer - node (v16.17.1, x64)
Req 1 - updateOpen 2,489ms (± 0.15%) 2,491ms (± 0.07%) ~ 2,488ms 2,493ms p=0.681 n=6
Req 2 - geterr 5,940ms (± 0.23%) 5,946ms (± 0.35%) ~ 5,915ms 5,966ms p=0.575 n=6
Req 3 - references 344ms (± 0.48%) 344ms (± 0.40%) ~ 342ms 346ms p=0.867 n=6
Req 4 - navto 277ms (± 0.37%) 278ms (± 0.62%) ~ 276ms 281ms p=0.737 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 (±10.32%) 82ms (± 7.59%) ~ 76ms 93ms p=0.510 n=6
CompilerTSServer - node (v16.17.1, x64)
Req 1 - updateOpen 2,624ms (± 0.84%) 2,612ms (± 0.55%) ~ 2,601ms 2,640ms p=0.170 n=6
Req 2 - geterr 4,776ms (± 0.19%) 4,770ms (± 0.15%) ~ 4,762ms 4,784ms p=0.195 n=6
Req 3 - references 354ms (± 2.22%) 350ms (± 0.21%) ~ 349ms 351ms p=0.150 n=6
Req 4 - navto 269ms (± 0.19%) 270ms (± 0.19%) ~ 269ms 270ms p=0.311 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 79ms (± 0.52%) 79ms (± 0.52%) ~ 78ms 79ms p=0.218 n=6
xstateTSServer - node (v16.17.1, x64)
Req 1 - updateOpen 2,710ms (± 0.18%) 2,711ms (± 0.13%) ~ 2,705ms 2,714ms p=0.629 n=6
Req 2 - geterr 1,965ms (± 1.03%) 1,970ms (± 0.52%) ~ 1,957ms 1,988ms p=0.936 n=6
Req 3 - references 139ms (± 1.87%) 137ms (± 3.04%) ~ 131ms 142ms p=0.687 n=6
Req 4 - navto 352ms (± 0.34%) 352ms (± 0.34%) ~ 351ms 354ms p=1.000 n=6
Req 5 - completionInfo count 2,071 (± 0.00%) 2,071 (± 0.00%) ~ 2,071 2,071 p=1.000 n=6
Req 5 - completionInfo 323ms (± 1.62%) 321ms (± 1.62%) ~ 314ms 326ms p=0.572 n=6
System info unknown
Hosts
  • node (v16.17.1, x64)
Scenarios
  • CompilerTSServer - node (v16.17.1, x64)
  • Compiler-UnionsTSServer - node (v16.17.1, x64)
  • xstateTSServer - node (v16.17.1, 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 (v16.17.1, x64)
Execution time 156.36ms (± 0.15%) 156.40ms (± 0.15%) +0.04ms (+ 0.03%) 154.96ms 158.64ms p=0.006 n=600
tsserver-startup - node (v16.17.1, x64)
Execution time 230.44ms (± 0.14%) 231.19ms (± 0.13%) +0.75ms (+ 0.33%) 229.75ms 235.79ms p=0.000 n=600
tsserverlibrary-startup - node (v16.17.1, x64)
Execution time 235.25ms (± 0.12%) 236.10ms (± 0.17%) +0.86ms (+ 0.36%) 234.66ms 249.11ms p=0.000 n=600
typescript-startup - node (v16.17.1, x64)
Execution time 236.17ms (± 0.15%) 236.03ms (± 0.15%) -0.13ms (- 0.06%) 234.32ms 243.19ms p=0.000 n=600
System info unknown
Hosts
  • node (v16.17.1, x64)
Scenarios
  • tsc-startup - node (v16.17.1, x64)
  • tsserver-startup - node (v16.17.1, x64)
  • tsserverlibrary-startup - node (v16.17.1, x64)
  • typescript-startup - node (v16.17.1, 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-repos suite comparing main and refs/pull/55613/merge:

Everything looks good!

@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.

};

const getName2 = (person?: Person): string => {
return isString(person?.name) ? person?.name : '';
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be changed to this?

Suggested change
return isString(person?.name) ? person?.name : '';
return isString(person?.name) ? person.name : '';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both should be fine. If I recall correctly, optional chaining does not alter the type of the expression it is applied to.

Copy link
Contributor

@Andarist Andarist Sep 3, 2023

Choose a reason for hiding this comment

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

Ye, right - i thought that previously the error was also raised within the truthy branch of the conditional expression (as in that the obj wasnt being proven to be non-nullable) but it seems that this part was already working ok

type Person = { name: string; }

const getName1 = (person?: Person): string => {
return typeof person?.name === 'string' ? person?.name : '';
Copy link
Contributor

Choose a reason for hiding this comment

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

similarly here:

Suggested change
return typeof person?.name === 'string' ? person?.name : '';
return typeof person?.name === 'string' ? person.name : '';

@fatcerberus
Copy link

It must be getting too close to spooky season because I read the issue title as "Fix issue with ... type gourd" 🎃

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
Archived in project
Development

Successfully merging this pull request may close these issues.

Optional chaining, proper infer in type guard
6 participants