Repository navigation
[api] Reparse an extended config cached under a different resolution stack - #64825
Open
Christopher Plieger (cplieger) wants to merge 1 commit into
Open
Christopher Plieger (cplieger) wants to merge 1 commit into
Christopher Plieger (cplieger) wants to merge 1 commit into
Conversation
Open
1 of 6 tasks
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Stack-sensitive invalidation can cause conflicting recursive cache locks during concurrent snapshot creation.
1 open finding
What changed in this PR
Addresses #64824 by making extended-config cache validation sensitive to the resolution stack, following the circular-extends fix in #64755.
Changes:
- Invalidates cached configs when relevant resolution-stack membership changes.
- Adds a regression test for opening a project that extends into a cached cycle.
| File | Description |
|---|---|
tsc/internal/project/extendedconfigcache.go |
Adds resolution-stack information to cache validation. |
tsc/internal/api/session_apistate_test.go |
Tests sequential project opening into a circular extends chain. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+52
to
+53
| if slices.Contains(args.ResolutionStack, args.ConfigFS.CaseSensitivity().PathKey(fileName.AsPath())) { | ||
| _, _ = hasher.WriteString(fileName.AsString()) |
This branch has not been deployed
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.

When a config in a circular
extendschain is parsed,getExtendedConfigparses the config that closes the cycle without the extended config cache, because it is already on the resolution stack. The cached entry of the config that extends it still lists it inExtendedSourceFiles, though nothing acquired a cache entry for it. A second project that extends into the cycle reuses that cached entry, because its validity hash covers only file contents, and inherits the uncached path.Snapshot.Clonethen callsAddOwneron that path and panics withOwnerCache.AddOwner: entry not found.Impact: a repository with a circular
extendschain and another config that extends into it makescreateSnapshotfail with a Go panic whenever the cyclic project was opened first, wheretsc -preports TS18000 for each project, so an analyzer on the API fails on that repository depending on the order it opens its projects in.The extended config cache's
hashnow also writes the name of each extended file that is on the current resolution stack, compared byPathKeyas the circularity check does. An entry reached with a different set of its files on the stack is parsed again under that stack, and that parse acquires every file it lists. A config outside a cycle never has one of its extended files on its resolution stack, so its hash is the same as before and cache sharing is unchanged. TS18000 is reported as before.The CLI's own extended config cache, used by
tsc -pandtsc -b, has the same order dependence for configs inside anextendscycle, but it cannot panic and matches TypeScript 5.9's behavior, so this change leaves it alone.TestOpenProjectsExtendingIntoCircularExtendsReportCircularityopenscycle, whosetsconfig.jsonanda.jsonextend each other, and thenouter, which extendscycle/a.json, in one session. It expects TS18000 as the only config file parsing diagnostic ofouter, and onmainthe secondhandleCreateSnapshotcall panics. With atscbuilt from this branch, the reproduction in the issue reports TS18000 in both orders.TS18000 still prints its
{0}placeholder unfilled, which has a separate cause and is left out of this change.This follows #64754 and #64755. I met it while building deadset-ts, the TypeScript analyzer of deadset, on the TypeScript 7 API.
An AI coding agent wrote this patch. I have read, built and tested it and will handle the review.
Backlogmilestone (required)mainbranchnpx hereby testnpx hereby lintnpx hereby check:formatFixes #64824