Match Strada's behavior to allow satisfying recursive indexed access#4430
Open
RyanCavanaugh wants to merge 1 commit into
Open
Match Strada's behavior to allow satisfying recursive indexed access#4430RyanCavanaugh wants to merge 1 commit into
RyanCavanaugh wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Ports the TypeScript-Go checker’s base-constraint caching and recursion handling to more closely match upstream TypeScript (“Strada”), fixing a regression where recursive indexed-access/conditional constraints were treated as circular too early and caused constraint satisfaction failures (TypeScript#63568).
Changes:
- Split base-constraint caching into
immediateBaseConstraintvsresolvedBaseConstraint, and key circularity tracking off the immediate cache. - Refactor
getResolvedBaseConstraint/computeBaseConstraintto mirror Strada’s structure and recursion flow, including a targeted tweak for indexed-access constraints inside distributive conditional constraints. - Add a regression test and update baselines (including reducing one previously-emitted “Excessive stack depth” diagnostic in
infiniteConstraints).
Show a summary per file
| File | Description |
|---|---|
internal/checker/checker.go |
Reworks base-constraint computation/caching and circularity tracking to align with Strada and fix premature circularity. |
internal/checker/types.go |
Adds immediateBaseConstraint to ConstrainedType alongside resolvedBaseConstraint. |
internal/checker/utilities.go |
Updates call site for the new getResolvedBaseConstraint signature. |
testdata/tests/cases/compiler/recursiveMappedTypeDefaultConstraint.ts |
New regression repro covering recursive mapped/default constraint satisfaction. |
testdata/baselines/reference/compiler/recursiveMappedTypeDefaultConstraint.* |
Adds baselines for the new compiler test. |
testdata/baselines/reference/submodule/compiler/infiniteConstraints.errors.txt* |
Updates submodule baseline to reflect diagnostic changes from the new constraint behavior. |
Copilot's findings
- Files reviewed: 9/9 changed files
- Comments generated: 0
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
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.
Authored by copilot
Fixes microsoft/TypeScript#63568
Analysis
The regression came from Corsa not matching Strada’s base-constraint cache model.
In Strada, base-constraint resolution has two distinct caches:
immediateBaseConstraint, cached on everyTyperesolvedBaseConstraint, cached on instantiable/union/intersection typesThe key Strada implementation is
getResolvedBaseConstraint. It computesresolvedBaseConstraintby calling an innergetImmediateBaseConstraint, and recursive base-constraint walks go through an innergetBaseConstrainthelper. Circularity tracking is also keyed toTypeSystemPropertyName.ImmediateBaseConstraint, andresolutionTargetHasPropertychecksimmediateBaseConstraintatchecker.ts#L11525-L11526.Corsa had collapsed this into a single
resolvedBaseConstraintcache and passed an explicit recursion stack throughgetResolvedBaseConstraint(t, stack). That made conditional/indexed-access constraint recovery diverge in recursive cases like:In the reduced repro, the checker needs to keep advancing the conditional/indexed-access constraint chain far enough to see the
values: stringshape. The old Corsa model treated one of those recursive conditional constraints as circular too early, soWrapped<FromSchema<...>>failed theTypeconstraint even though Strada accepts the type parameter default.Fix
This ports Corsa’s base-constraint machinery closer to Strada
The main changes are:
Added
immediateBaseConstraintto Corsa’sConstrainedType, matching Strada’s separate immediate/resolved cache split:types.ts#L6452[internal/checker/types.go](https://github.com/microsoft/typescript-go/compare/main...RyanCavanaugh:fix63568?expand=1#)Renamed the type-resolution property from resolved-base-constraint tracking to immediate-base-constraint tracking:
[checker.ts#L1331](https://github.com/microsoft/typescript-go/compare/main...RyanCavanaugh:fix63568?expand=1#),[checker.ts#L11525-L11526](https://github.com/microsoft/typescript-go/compare/main...RyanCavanaugh:fix63568?expand=1#)[internal/checker/checker.go](https://github.com/microsoft/typescript-go/compare/main...RyanCavanaugh:fix63568?expand=1#)Refactored Corsa’s
getResolvedBaseConstraintto mirror Strada’s structure:getImmediateBaseConstraintgetBaseConstraintresolvedBaseConstraint = getImmediateBaseConstraint(t)[checker.ts#L15322-L15367](https://github.com/microsoft/typescript-go/compare/main...RyanCavanaugh:fix63568?expand=1#)[internal/checker/checker.go](https://github.com/microsoft/typescript-go/compare/main...RyanCavanaugh:fix63568?expand=1#)Updated
computeBaseConstraintto use the innergetBaseConstrainthelper, matching the Strada flow through type parameters, unions/intersections, index types, indexed accesses, substitutions, and tuples:[checker.ts#L15369-L15440](https://github.com/microsoft/typescript-go/compare/main...RyanCavanaugh:fix63568?expand=1#)[internal/checker/checker.go](https://github.com/microsoft/typescript-go/compare/main...RyanCavanaugh:fix63568?expand=1#)Kept Corsa’s existing safeguard for pathological recursive conditional constraints, but adjusted indexed-access check types to use the regular constraint path when appropriate. This preserves the existing infinite-constraint behavior while allowing the #63568 recursive mapped/default constraint case to resolve like Strada.
Added a regression test:
[testdata/tests/cases/compiler/recursiveMappedTypeDefaultConstraint.ts](https://github.com/microsoft/typescript-go/compare/main...RyanCavanaugh:fix63568?expand=1#)The baseline update also improves the existing
infiniteConstraintssubmodule output by removing one excessive-stack-depth diagnostic while retaining the intended indexing error.