Repository navigation
Avoid resolving property types that cannot be unit types when looking for key properties - #65056
Mateusz Burzyński (Andarist) wants to merge 4 commits into
Conversation
… for key properties
|
TypeScript Bot (@typescript-bot) test it |
|
Hey Jake Bailey (@jakebailey), this PR was updated less than two minutes before this command. Please wait until it has been unchanged for two minutes and try again. |
|
TypeScript Bot (@typescript-bot) test it |
|
Hey Jake Bailey (@jakebailey), the results of running the DT tests are ready. Everything looks the same! |
|
Jake Bailey (@jakebailey) Here are the results of running the user tests with tsc comparing Everything looks good! |
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Jake Bailey (@jakebailey) Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
There was a problem hiding this comment.
🟡 Changes recommended
The guard prematurely rejects unresolved generic variadic tuples that can normalize to never, potentially preventing key-property map creation and bypassing the discriminant shortcut in removeSubtypes.
1 open issue
What changed in this PR
This experiment aims to reduce unnecessary property-type resolution when the native TypeScript checker searches for unit-typed discriminant properties.
Changes:
- Adds a shared predicate that skips methods and certain instantiated or mapped properties.
- Uses the predicate in key-property discovery and union subtype reduction.
| File | Description |
|---|---|
tsc/internal/checker/relater.go |
Defines the predicate and applies it to key-property candidate selection. |
tsc/internal/checker/checker.go |
Reuses the predicate when selecting discriminants for subtype reduction. |
Review effort: Balanced
…le that can normalize to never
There was a problem hiding this comment.
🟡 Changes recommended
The shortcut’s preserved literal-property exceptions lack compiler regression tests covering both key-property mapping with at least ten union constituents and subtype reduction.
1 open issue
1 resolved since last review
Review effort: Balanced
| if core.Some(uninstantiated.Distributed(), func(t *Type) bool { | ||
| return t.flags&TypeFlagsObject != 0 && t.objectFlags&ObjectFlagsMapped == 0 && !isGenericTupleType(t) | ||
| }) { |
There was a problem hiding this comment.
this is perf-only and we can't really easily test that
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
|
TypeScript Bot (@typescript-bot) perf test this faster |
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
TypeScript Bot (@typescript-bot) perf test this faster |
|
Wesley Wigham (@weswigham) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // A method is typed as a function type only when none of the flags getTypeOfSymbol checks before it are set | ||
| if prop.Flags()&(ast.SymbolFlagsMethod|ast.SymbolFlagsAccessor|ast.SymbolFlagsVariable|ast.SymbolFlagsProperty) == ast.SymbolFlagsMethod { | ||
| return false | ||
| } |
There was a problem hiding this comment.
Why is this not just an early bailout on if prop.Flags()&ast.SymbolFlagsMethod == 0? Why do the other symbol types carve out exceptions?
There was a problem hiding this comment.
they were not needed, I simplified this now - the only flag trily coexisting with ast.SymbolFlagsMethod is ast.SymbolFlagsProperty. And even then the symbol is still a method and thus not a unit type so the bailout is safe
| return "" | ||
| } | ||
|
|
||
| // Reports whether a property has a unit type, without resolving the types of methods, which are function types, or |
There was a problem hiding this comment.
The comment is slightly misleading, right? You may still resolve the type.
There was a problem hiding this comment.
fixed


No description provided.