When an intersection is going to produce an expression too complex error... #42772
Conversation
This file contains 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
…ror, eagerly perform reductions in a last-ditch effort to avoid the error
|
@typescript-bot perf test this |
|
Heya @weswigham, I've started to run the parallelized community code test suite on this PR at 757bee7. You can monitor the build here. |
|
Heya @weswigham, I've started to run the parallelized Definitely Typed test suite on this PR at 757bee7. You can monitor the build here. |
|
Heya @weswigham, I've started to run the extended test suite on this PR at 757bee7. You can monitor the build here. |
|
Heya @weswigham, I've started to run the tarball bundle task on this PR at 757bee7. You can monitor the build here. |
|
Heya @weswigham, I've started to run the perf test suite on this PR at 757bee7. You can monitor the build here. Update: The results are in! |
weswigham
commented
Feb 12, 2021
| // This is relevant for, eg, when looking at `(HTMLElement | null) & (SVGElement | null) & ... & undefined` where _usually_ | ||
| // we'd allow for tons of garbage intermediate types like `null & SVGElement` to exist; but nobody ever really actually _wants_ | ||
| // that, IMO. Those types can still exist in the type system; just... not when working with unions and intersections with massive | ||
| // cross-product growth potential. |
There was a problem hiding this comment.
RE: This comment and associated logic. If we did this in all scenarios, it would be a breaking change; however since we only do it when we would have previously issued a Expression produces a union type that is too complex to represent error, it is not. I think we've had a desire to try to eliminate structurally branded primitives from the type system for awhile, and I think this is just one small corner where we can actually remove them and be pretty safe, without worrying about either breaking people or needing to provide an alternative.
|
Hey @weswigham, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
@weswigham Here they are:Comparison Report - master..42772
System
Hosts
Scenarios
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
sandersn
added
the
For Milestone Bug
|
Unfortunately, we never finished reviewing this PR. It is pretty old now, so I'm going to close it to reduce the number of open PRs. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
eagerly perform reductions in a last-ditch effort to avoid the error. I've wanted to do something like this since #29949 was opened, and since we added
getReducedTypeand associated machinery, alongside machinery for bailing when expressions are producing unions/intersections are too complex. I feel like we now have a scenario where it's justifiable to invoke the reduction machinery earlier than we normally do in order to try to save us from resorting to an "Expression produces a union type that is too complex to represent" error, since that error is ultimately heuristic and, in fact, the union produced is not too complex if significant reductions take place.#29949 has had acceptable perf for awhile, it just also had a complexity error in it, so could already be considered "fixed". This really fixes #29949, IMO, by finally removing that "expression too complex" error once
react's types are adjusted to make the tworefobject types mutually exclusive by adding acurrent?: undefinedproperty to theRefCallbacktype and the callback inForwardedRef.This shouldn't impact perf generally, since this fallback heuristic only takes effect when we would already issue an error.