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

[WIP] Contextual narrowing to object by DanielRosenwasser · Pull Request #48576 · microsoft/TypeScript · GitHub

/ TypeScript Public

[WIP] Contextual narrowing to object - #48576

Closed
Daniel Rosenwasser (DanielRosenwasser) wants to merge 6 commits into
mainfrom
contextualNarrowingToObject
ConversationCommits (6)ChecksFiles changed
Closed

[WIP] Contextual narrowing to object#48576
Daniel Rosenwasser (DanielRosenwasser) wants to merge 6 commits into
mainfrom
contextualNarrowingToObject

Conversation

Daniel Rosenwasser (DanielRosenwasser) commented Apr 5, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Related: #48468

Daniel Rosenwasser (DanielRosenwasser) changed the title Contextual narrowing to object [WIP] Contextual narrowing to object Apr 5, 2022
Closed
TypeScript Bot (typescript-bot) added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 5, 2022

Daniel Rosenwasser (DanielRosenwasser) commented Apr 27, 2022

Copy link
Copy Markdown
Member Author

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

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view
Open

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

Daniel Rosenwasser (DanielRosenwasser) commented Apr 27, 2022

Copy link
Copy Markdown
Member Author

Seems like there's a slowdown from this - maybe a fluke?

TypeScript Bot (@typescript-bot) perf test faster

TypeScript Bot (typescript-bot) commented Apr 27, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the abridged perf test suite on this PR at 1e2eb03. You can monitor the build here.

Update: The results are in!

Daniel Rosenwasser (DanielRosenwasser) commented Apr 27, 2022
View reviewed changes
Comment thread src/compiler/checker.ts Outdated
someType(type, isGenericTypeWithUnionConstraint) ||
// When the contextual type is 'unknown', we may need to narrow for compatibility with non-null targets.
// This allows some parity with a constraint of '{} | null | undefined'.
(type.flags & TypeFlags.Instantiable) && contextualType && isEmptyObjectType(contextualType);

Daniel Rosenwasser (DanielRosenwasser) Apr 27, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should this be gated on strictNullChecks?

Anders Hejlsberg (ahejlsberg) reviewed Apr 27, 2022
View reviewed changes
Comment thread src/compiler/checker.ts Outdated

TypeScript Bot (typescript-bot) commented Apr 27, 2022

Copy link
Copy Markdown
Contributor

Daniel Rosenwasser (@DanielRosenwasser)
The results of the perf run you requested are in!

Here they are:

Comparison Report - main..48576

Metric main 48576 Delta Best Worst
Angular - node (v14.15.1, x64)
Memory used 333,421k (± 0.01%) 333,544k (± 0.01%) +124k (+ 0.04%) 333,489k 333,606k
Parse Time 2.03s (± 0.43%) 2.04s (± 0.57%) +0.01s (+ 0.54%) 2.02s 2.07s
Bind Time 0.87s (± 0.74%) 0.87s (± 0.56%) +0.00s (+ 0.23%) 0.86s 0.88s
Check Time 5.63s (± 0.41%) 5.73s (± 0.46%) +0.11s (+ 1.88%) 5.67s 5.78s
Emit Time 6.30s (± 0.51%) 6.31s (± 0.73%) +0.01s (+ 0.14%) 6.21s 6.42s
Total Time 14.83s (± 0.33%) 14.95s (± 0.47%) +0.13s (+ 0.85%) 14.80s 15.09s
Compiler-Unions - node (v14.15.1, x64)
Memory used 192,431k (± 0.41%) 192,049k (± 0.14%) -383k (- 0.20%) 191,370k 192,339k
Parse Time 0.85s (± 0.58%) 0.85s (± 0.70%) 0.00s ( 0.00%) 0.85s 0.87s
Bind Time 0.56s (± 1.57%) 0.56s (± 0.40%) +0.00s (+ 0.18%) 0.56s 0.57s
Check Time 7.50s (± 0.49%) 7.56s (± 0.43%) +0.06s (+ 0.76%) 7.50s 7.65s
Emit Time 2.52s (± 1.05%) 2.50s (± 0.40%) -0.02s (- 0.79%) 2.49s 2.53s
Total Time 11.44s (± 0.38%) 11.48s (± 0.30%) +0.04s (+ 0.31%) 11.41s 11.56s
Monaco - node (v14.15.1, x64)
Memory used 325,495k (± 0.00%) 325,586k (± 0.01%) +91k (+ 0.03%) 325,511k 325,626k
Parse Time 1.58s (± 0.79%) 1.58s (± 0.94%) -0.00s (- 0.32%) 1.56s 1.62s
Bind Time 0.78s (± 0.47%) 0.78s (± 0.64%) -0.00s (- 0.13%) 0.77s 0.79s
Check Time 5.56s (± 0.49%) 5.62s (± 0.50%) +0.05s (+ 0.99%) 5.57s 5.69s
Emit Time 3.32s (± 1.07%) 3.31s (± 0.72%) -0.01s (- 0.27%) 3.27s 3.36s
Total Time 11.25s (± 0.44%) 11.29s (± 0.38%) +0.04s (+ 0.37%) 11.20s 11.40s
TFS - node (v14.15.1, x64)
Memory used 289,058k (± 0.01%) 289,197k (± 0.01%) +139k (+ 0.05%) 289,101k 289,262k
Parse Time 1.37s (± 0.65%) 1.37s (± 1.67%) +0.00s (+ 0.37%) 1.35s 1.43s
Bind Time 0.73s (± 0.94%) 0.72s (± 0.50%) -0.00s (- 0.28%) 0.72s 0.73s
Check Time 5.17s (± 0.25%) 5.31s (± 0.41%) +0.14s (+ 2.63%) 5.26s 5.35s
Emit Time 3.52s (± 2.29%) 3.50s (± 1.84%) -0.01s (- 0.40%) 3.40s 3.63s
Total Time 10.78s (± 0.72%) 10.91s (± 0.69%) +0.13s (+ 1.19%) 10.76s 11.08s
material-ui - node (v14.15.1, x64)
Memory used 447,685k (± 0.06%) 447,773k (± 0.06%) +87k (+ 0.02%) 446,631k 447,945k
Parse Time 1.87s (± 0.56%) 1.88s (± 0.64%) +0.01s (+ 0.37%) 1.84s 1.90s
Bind Time 0.70s (± 0.93%) 0.70s (± 0.80%) -0.00s (- 0.00%) 0.69s 0.71s
Check Time 12.98s (± 0.67%) 13.18s (± 0.64%) +0.19s (+ 1.49%) 13.02s 13.47s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.55s (± 0.61%) 15.75s (± 0.49%) +0.21s (+ 1.32%) 15.59s 16.01s
xstate - node (v14.15.1, x64)
Memory used 535,906k (± 0.00%) 535,912k (± 0.00%) +7k (+ 0.00%) 535,876k 535,960k
Parse Time 2.59s (± 0.35%) 2.59s (± 0.36%) +0.00s (+ 0.04%) 2.58s 2.62s
Bind Time 1.14s (± 0.70%) 1.15s (± 0.66%) +0.00s (+ 0.26%) 1.13s 1.16s
Check Time 1.51s (± 0.46%) 1.53s (± 0.49%) +0.02s (+ 1.06%) 1.51s 1.54s
Emit Time 0.07s (± 0.00%) 0.07s (± 0.00%) 0.00s ( 0.00%) 0.07s 0.07s
Total Time 5.33s (± 0.30%) 5.34s (± 0.27%) +0.01s (+ 0.24%) 5.31s 5.38s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-210-generic
Architecturex64
Available Memory16 GB
Available Memory2 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v14.15.1, x64)
Scenarios
  • Angular - node (v14.15.1, x64)
  • Compiler-Unions - node (v14.15.1, x64)
  • Monaco - node (v14.15.1, x64)
  • TFS - node (v14.15.1, x64)
  • material-ui - node (v14.15.1, x64)
  • xstate - node (v14.15.1, x64)
Benchmark Name Iterations
Current 48576 10
Baseline main 10

Developer Information:

Download Benchmark

Daniel Rosenwasser (DanielRosenwasser) commented Apr 28, 2022

Copy link
Copy Markdown
Member Author

TypeScript Bot (@typescript-bot) pack this
TypeScript Bot (@typescript-bot) test this
TypeScript Bot (@typescript-bot) user test this
TypeScript Bot (@typescript-bot) run dt
TypeScript Bot (@typescript-bot) perf test faster

This comment was marked as outdated.

Sign in to view

TypeScript Bot (typescript-bot) commented Apr 28, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the parallelized Definitely Typed test suite on this PR at ab3f21e. You can monitor the build here.

TypeScript Bot (typescript-bot) commented Apr 28, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the extended test suite on this PR at ab3f21e. You can monitor the build here.

TypeScript Bot (typescript-bot) commented Apr 28, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the tarball bundle task on this PR at ab3f21e. You can monitor the build here.

TypeScript Bot (typescript-bot) commented Apr 28, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the parallelized community code test suite on this PR at ab3f21e. You can monitor the build here.

TypeScript Bot (typescript-bot) commented Apr 28, 2022

Copy link
Copy Markdown
Contributor

Daniel Rosenwasser (@DanielRosenwasser)
The results of the perf run you requested are in!

Here they are:

Comparison Report - main..48576

Metric main 48576 Delta Best Worst
Angular - node (v14.15.1, x64)
Memory used 333,420k (± 0.01%) 333,410k (± 0.01%) -10k (- 0.00%) 333,358k 333,446k
Parse Time 2.03s (± 0.77%) 2.03s (± 0.47%) -0.01s (- 0.34%) 2.01s 2.05s
Bind Time 0.88s (± 0.45%) 0.88s (± 0.42%) -0.00s (- 0.46%) 0.87s 0.88s
Check Time 5.63s (± 0.60%) 5.60s (± 0.41%) -0.03s (- 0.59%) 5.56s 5.65s
Emit Time 6.34s (± 1.13%) 6.27s (± 0.58%) -0.07s (- 1.10%) 6.20s 6.34s
Total Time 14.88s (± 0.66%) 14.78s (± 0.33%) -0.11s (- 0.74%) 14.68s 14.88s
Compiler-Unions - node (v14.15.1, x64)
Memory used 192,516k (± 0.37%) 192,801k (± 0.51%) +285k (+ 0.15%) 191,559k 195,438k
Parse Time 0.85s (± 0.47%) 0.85s (± 0.47%) 0.00s ( 0.00%) 0.84s 0.86s
Bind Time 0.56s (± 0.71%) 0.56s (± 0.59%) -0.00s (- 0.18%) 0.55s 0.57s
Check Time 7.51s (± 0.38%) 7.48s (± 0.74%) -0.03s (- 0.33%) 7.40s 7.67s
Emit Time 2.53s (± 1.00%) 2.52s (± 1.06%) -0.01s (- 0.24%) 2.46s 2.59s
Total Time 11.45s (± 0.37%) 11.41s (± 0.60%) -0.03s (- 0.30%) 11.27s 11.65s
Monaco - node (v14.15.1, x64)
Memory used 325,497k (± 0.01%) 325,523k (± 0.00%) +27k (+ 0.01%) 325,496k 325,544k
Parse Time 1.58s (± 0.82%) 1.56s (± 0.44%) -0.01s (- 0.76%) 1.55s 1.58s
Bind Time 0.78s (± 0.86%) 0.78s (± 0.75%) 0.00s ( 0.00%) 0.77s 0.79s
Check Time 5.56s (± 0.47%) 5.54s (± 0.44%) -0.01s (- 0.23%) 5.50s 5.60s
Emit Time 3.33s (± 0.71%) 3.34s (± 0.73%) +0.01s (+ 0.27%) 3.28s 3.39s
Total Time 11.24s (± 0.48%) 11.22s (± 0.38%) -0.02s (- 0.13%) 11.12s 11.30s
TFS - node (v14.15.1, x64)
Memory used 289,074k (± 0.01%) 289,061k (± 0.01%) -14k (- 0.00%) 289,033k 289,110k
Parse Time 1.37s (± 1.84%) 1.35s (± 0.66%) -0.01s (- 0.88%) 1.34s 1.37s
Bind Time 0.72s (± 0.66%) 0.73s (± 1.43%) +0.01s (+ 0.69%) 0.72s 0.76s
Check Time 5.21s (± 0.33%) 5.19s (± 0.44%) -0.01s (- 0.21%) 5.15s 5.25s
Emit Time 3.54s (± 2.00%) 3.52s (± 2.17%) -0.02s (- 0.68%) 3.38s 3.63s
Total Time 10.84s (± 0.89%) 10.79s (± 0.75%) -0.04s (- 0.39%) 10.63s 10.91s
material-ui - node (v14.15.1, x64)
Memory used 447,798k (± 0.00%) 447,694k (± 0.06%) -103k (- 0.02%) 446,660k 447,862k
Parse Time 1.87s (± 0.43%) 1.87s (± 0.31%) -0.00s (- 0.11%) 1.86s 1.88s
Bind Time 0.70s (± 0.74%) 0.70s (± 0.83%) -0.00s (- 0.57%) 0.69s 0.71s
Check Time 13.05s (± 0.82%) 13.12s (± 0.79%) +0.07s (+ 0.52%) 12.86s 13.32s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.62s (± 0.69%) 15.68s (± 0.66%) +0.06s (+ 0.38%) 15.44s 15.88s
xstate - node (v14.15.1, x64)
Memory used 535,897k (± 0.00%) 535,924k (± 0.00%) +27k (+ 0.01%) 535,903k 535,968k
Parse Time 2.61s (± 0.63%) 2.60s (± 0.68%) -0.01s (- 0.23%) 2.57s 2.66s
Bind Time 1.15s (± 1.02%) 1.14s (± 0.52%) -0.01s (- 0.61%) 1.13s 1.16s
Check Time 1.51s (± 0.49%) 1.51s (± 0.72%) +0.00s (+ 0.13%) 1.49s 1.54s
Emit Time 0.07s (± 0.00%) 0.07s (± 0.00%) 0.00s ( 0.00%) 0.07s 0.07s
Total Time 5.34s (± 0.40%) 5.33s (± 0.32%) -0.01s (- 0.15%) 5.31s 5.38s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-210-generic
Architecturex64
Available Memory16 GB
Available Memory2 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v14.15.1, x64)
Scenarios
  • Angular - node (v14.15.1, x64)
  • Compiler-Unions - node (v14.15.1, x64)
  • Monaco - node (v14.15.1, x64)
  • TFS - node (v14.15.1, x64)
  • material-ui - node (v14.15.1, x64)
  • xstate - node (v14.15.1, x64)
Benchmark Name Iterations
Current 48576 10
Baseline main 10

Developer Information:

Download Benchmark

Daniel Rosenwasser (DanielRosenwasser) commented Apr 28, 2022

Copy link
Copy Markdown
Member Author

RWC reveals a few interesting things:

  1. Substituting unknown for the error messages is definitely a bit confusing.

  2. This gets worse with unions - e.g.

    let f = <T, U extends UsefulType>(x: T | U): object => x

    So whereas this used to report that T | U wasn't assignable to object, it now reports unknown isn't assignable to object.

Going to rerun with latest changes.

Anders Hejlsberg (ahejlsberg) commented Apr 28, 2022

Copy link
Copy Markdown
Member

Hmm, yeah, I don't much like the error message changes in the RWC baselines. The unknown makes it seem like the expression doesn't have a type at all.

Daniel Rosenwasser (DanielRosenwasser) commented Apr 28, 2022

Copy link
Copy Markdown
Member Author

Discussed with Anders Hejlsberg (@ahejlsberg) offline - this PR is a bit too corner-casey and produces worse messages when a type isn't narrowed. We're interested in seeing if we can leverage smarter intersection types with {} while narrowing generics, which is a change that will have to wait for 4.8 at the least. For now we'll have to live with the break, which people can get around by adding a constraint of {} | null | undefined.

Daniel Rosenwasser (DanielRosenwasser) deleted the contextualNarrowingToObject branch April 28, 2022 20:01
This was referenced May 4, 2022
Closed
Closed
Closed
Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 22, 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

Anders Hejlsberg (ahejlsberg) ahejlsberg left review comments
Wesley Wigham (weswigham) Awaiting requested review from weswigham
Copilot code review Copilot
Copilot will automatically review once the pull request is marked ready for review

Assignees

Daniel Rosenwasser (DanielRosenwasser)

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.

3 participants

Back | FazBrowse Home | New Git URL