Expand intersection reduction division strategy down to 3-member intersections #59425
Conversation
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
All the way down to 3 member intersections. This way you can only encounter a complexity error for a pick of two (or more) 1000 property types, and not three 50 property types. I played around with some other heuristics, but they ended up being overly costly for how narrow they were - pattern literals throw a wrench into our usual intersection fast path for literal types, and we *could* preserve it in cases like this one where there's a single pattern literal type common to all otherwise-primitive unions in the intersection (intersections with other literals either produce never or that literal), but that all goes away once there are two or more pattern literals, since then you need to worry about myrid ways the patterns can combine (which is to say: potentially as many ways as the power set indicates), so it didn't seem worth it for the complexity. Even `react` has *two* of these pattern index signatures (aria- and data-), not one, so such a heuristic won't help the common case anyway. I think there is still *some* splitting and pre-simplification we can do among the literal members, probably, even when there are non-primitive members, but I haven't been able to nail down a sufficiently general algorithm yet. So instead this one-liner will probably suffice for now.
typescript-bot
added
Author: Team
For Uncommitted Bug
|
@typescript-bot test this |
|
@weswigham Here are the results of running the user tests with tsc comparing Everything looks good! |
|
Hey @weswigham, the results of running the DT tests are ready. Everything looks the same! |
|
@weswigham Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@weswigham Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
jsep
reviewed
Jul 26, 2024
sandersn
reviewed
Aug 13, 2024
| c8: string; | ||
| } | ||
|
|
||
| const xyz: Pick<A | B | C, 'id'> = {id: 'id'} |
There was a problem hiding this comment.
the heuristic doesn't apply for A | B, right?
There was a problem hiding this comment.
So this heuristic is for intersections - the way it applies here is to the key types within the mapped type. Before normalization, we construct keyof A & keyof B & keyof C which are all unions of many things - and this lets us construct it as (keyof A & keyof B) & keyof C instead, so when we see that keyof A & keyof B just becomes keyof Base (essentially), we know we can do keyof Base & keyof C without exponential blowup exceeding our limits.
Definitely doesn't apply to two element intersections, though - there's no set of comparisons to divide up.
A notable thing to say is that we attempt to do literal reduction of the intersection before the pairwise breakdown, since we can do literal-only intersection combinations in linear time (we just look for literals common to every intersected union, since them intersected with one another is always never), so this more complete division is only critical for keyof types that contain pattern literal index signatures, like in this test.
sandersn
approved these changes
Aug 14, 2024
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.
So instead of 50^3 tripping a breaker, we instead at least try the 50^2 intersection first and see if it reduces any (and it does). The same error can still occur this way, but the objects need to have 1000 members instead of 50 now - a much more generous limit.
I played around with some other heuristics to find and simplify intersections of unions like this earlier (almost-primitive unions), but they ended up being overly costly for how narrow they were - pattern literals throw a wrench into our usual intersection fast path for literal types, and we could preserve it in cases like this one where there's a single pattern literal type common to all otherwise-primitive unions in the intersection (intersections of the pattern with other literals either produce never or that literal), but that all goes away once there are two or more pattern literals, since then you need to worry about myriad ways the patterns can combine (which is to say: potentially as many ways as the power set indicates), so it didn't seem worth it for the complexity. Even
reacthas two of these pattern index signatures (aria- and data-), not one, so such a heuristic won't help the common case anyway. I think there is still some splitting and pre-simplification we can do among the literal members, probably, even when there are non-primitive members, but I haven't been able to nail down a sufficiently general algorithm yet.Fixes #58898