Repository navigation
Check referenced project sources before probing their declaration outputs - #64847
Open
Guru 🦄 (guru-irl) wants to merge 2 commits into
Open
Guru 🦄 (guru-irl) wants to merge 2 commits into
Guru 🦄 (guru-irl) wants to merge 2 commits into
Conversation
Use project-reference mappings before probing declaration files and directories on disk. Cache symlinked file mappings even when declarations already exist, so realpath does not access redirected outputs. Retain filesystem fallback when a referenced source is missing. When a referenced project's declaration output is itself a symlink to another location and the source exists, the program now uses the source, as it already does for unbuilt references. Previously the symlink target's declaration file was loaded. Check ancestor links before scanning the symlink cache, and test emptiness without counting every link. This keeps external declaration lookups from scaling with unrelated workspace links. Cover built, unbuilt, source-missing and missing cases, outputs that are symlinks, and packages whose types entry names a declaration directory, using a recording filesystem and the program file lists.
typescript-automation
Bot
added
the
For Uncommitted Bug
Contributor
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Changed symlink-resolution behavior needs broader validation, and redundant link scanning remains unresolved.
1 open finding
What changed in this PR
This PR prioritizes referenced project sources over declaration-output disk checks during language-service module resolution.
Changes:
- Checks source mappings before probing declaration files and directories.
- Adds ancestor-link checks before scanning cached symlinks.
- Adds ten cases covering output availability and symlink behavior.
| File | Description |
|---|---|
| tsc/internal/compiler/projectreferencedtsfakinghost.go | Prioritizes source mappings and checks ancestor links. |
| tsc/internal/compiler/projectreferencedtsfakinghost_test.go | Tests resolution results and declaration-output access. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+224
to
+226
| if ok && existsUsingSource(tspath.RootedPath(realFile)).IsTrue() { | ||
| canMap = true | ||
| break |
The ancestor walk already finds a link whose target exists in a referenced project's sources. Use that result instead of scanning every known link again, so each lookup no longer grows with the number of links. When several ancestors are links, the nearest one is used; the scan picked any of them.
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.
There is no associated issue.
Problem
useSourceOfProjectReference, used by the language service), module resolution still checks the disk for referenced.d.tsoutputs and output folders before it maps them to the source.Change
.d.tsfiles and folders in a referenced project's output are looked up in the reference mapping before the disk..d.tsfile still falls back to the disk when its source is missing. Output folders were already reported as existing; they are now answered without the disk.node_modules, the mapping is cached, so realpath does not touch the output folder.tscandtsc -bdo not use referenced sources and are unaffected.Behavior changes
.d.tsoutput, or its output folder, is a symlink to another location and the source exists, the program now uses the source, as it already does for unbuilt references. Before, it loaded the linked.d.ts. Reading the JavaScript implementation, it also checks the disk first and loads the linked file, so this differs from it..d.tsand folder lookups now consult the reference mapping before the disk. Each uncached lookup does more work, and the folder check grows with the number of referenced projects.typesentry, resolution still checkslib.ts,lib.tsx,lib.d.ts,lib/index.tsandlib/index.tsxon disk, as before.Tests
TestProjectReferenceSourceRedirectOutputAccessesintsc/internal/compilerrecords checks of the dependency'slib/index.d.tsandlibfolder. Ten cases: four output states (built, unbuilt, source missing, both missing), each withtypespointing at a file and at a folder, plus two built cases where the output file or folder is a symlink.lib/index.d.tsandlibare not touched. With the source missing, it checks thatlib/index.d.tsis still checked on disk.go testfor./internal/compiler,./internal/module,./internal/symlinks/...,./internal/project/...and./internal/ls/...intsc/passes, andgofmtandgo vetare clean.AI disclosure