Add --stableTypeOrdering for TS7 ordering compat #63084
Open
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
typescript-bot
added
Author: Team
For Uncommitted Bug
Collaborator
|
Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page. Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up. |
Member
Author
|
The flag is disabled; checking perf to verify that it isn't harmful. @typescript-bot perf test this faster |
Collaborator
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new --stableTypeOrdering compiler flag intended to make type/symbol/node ordering deterministic (TS7 ordering compatibility) to support concurrent checking in the tsgo port, at a performance cost.
Changes:
- Introduces the
stableTypeOrderingcompiler option (CLI +CompilerOptions) and its help/diagnostic text. - Reorders
TypeFlags(and adjusts related flag groupings) to support a deterministic sort strategy. - Implements stable ordering logic in the checker (type/symbol/node comparators and stable sorting in several flows).
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/baselines/reference/tsc/commandLine/help-all.js | Updates --help --all baseline to include --stableTypeOrdering. |
| tests/baselines/reference/api/typescript.d.ts | Updates public API baselines for reordered TypeFlags and new CompilerOptions.stableTypeOrdering. |
| src/compiler/types.ts | Reorders TypeFlags, tweaks ObjectFlags.ObjectTypeKindMask, adds stableTypeOrdering to CompilerOptions. |
| src/compiler/diagnosticMessages.json | Adds the option description message used by the CLI help output. |
| src/compiler/core.ts | Makes binarySearchKey tolerant of non-Comparison comparer returns; exports compareComparableValues. |
| src/compiler/commandLineParser.ts | Registers the new stableTypeOrdering option for CLI/config parsing. |
| src/compiler/checker.ts | Adds stable ordering machinery and wires it behind compilerOptions.stableTypeOrdering. |
Comment on lines
54150
to
54154
| switch (kind1) { | ||
| case TypeMapKind.Simple: { | ||
| const c = compareTypes(m1.source, (m2 as typeof m1).source); | ||
| if (c !== 0) { | ||
| return c; |
There was a problem hiding this comment.
TypeMapKind includes Deferred, Function, and Composite (see src/compiler/types.ts:7076+), but compareTypeMappers doesn’t handle them. For those kinds, the switch falls through and returns 0, causing many distinct mappers to compare equal and forcing later fallback to unstable IDs. Add deterministic comparisons for the missing kinds (and consider handling Composite similarly to Merged).
Comment on lines
18075
to
18079
| return stableTypeOrdering ? binarySearch(types, type, identity, compareTypes) >= 0 : binarySearch(types, type, getTypeId, compareValues) >= 0; | ||
| } | ||
|
|
||
| function insertType(types: Type[], type: Type): boolean { | ||
| const index = binarySearch(types, type, getTypeId, compareValues); | ||
| const index = stableTypeOrdering ? binarySearch(types, type, identity, compareTypes) : binarySearch(types, type, getTypeId, compareValues); |
There was a problem hiding this comment.
binarySearch expects a Comparer returning Comparison (-1/0/1), but compareTypes returns arbitrary numbers (e.g. ID differences). This is a type error and also violates the Comparer contract. Wrap with Math.sign/toComparison at the call site (or change compareTypes to return Comparison by using compareValues/compareComparableValues and converting numeric deltas).
Member
Author
There was a problem hiding this comment.
This is true, but I updated binarySearch to math.Sign, which is theoretically sufficient, although I could define a compareTypes helper that does that
Comment on lines
54102
to
54106
| for (let i = 0; i < (t1.labeledElementDeclarations?.length ?? 0); i++) { | ||
| const c = compareElementLabels(t1.labeledElementDeclarations![i], t2.labeledElementDeclarations![i]); | ||
| if (c !== 0) { | ||
| return c; | ||
| } |
There was a problem hiding this comment.
Tuple label comparison only iterates up to t1.labeledElementDeclarations?.length and doesn’t account for t2 having more (or fewer) labels, so tuples can compare equal despite differing label metadata and then fall back to unstable IDs. Compare the label-declaration lengths first (or iterate up to max(len1, len2) and treat missing as undefined).
Collaborator
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
jakebailey
commented
Feb 4, 2026
| if (c !== 0) { | ||
| return c; | ||
| } | ||
| c = (t1 as IndexType).flags - (t2 as IndexType).flags; |
Member
Author
There was a problem hiding this comment.
Per microsoft/typescript-go#2663 I'm pretty sure this should be a different set of flags
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Yoinked out of #62556 / tsgo-port.
The Go port of the compiler does concurrent checking with multiple checkers. But because they can visit nodes/types/symbols in a different order, their results are not stable when compared to each other due to lazy ID assignment.
The way the port addresses this is to enforce a strict ordering of types, symbols, and nodes, that does not rely on IDs. This causes changes in behavior, however, since the ordering is not a defined part of the language.
This PR adds a flag that enables the same algorithm that TS7 has, at the cost of perf.
On main, this flag is defaulted to false, of course, but it'd be flipped on in tsgo-port (which is more convenient than maintaining a patch).
This requires
TypeFlagsto change order, but this is not a part of the API per se; nobody should depend on the exact values of these as they are not declaredconstin the published.d.ts.