Skip to content

Avoid false symlink mappings for physical dependencies - #4813

Open
platypii wants to merge 1 commit into
microsoft:mainfrom
platypii:fix/jsdoc-private-alias-symlink-cache
Open

Avoid false symlink mappings for physical dependencies#4813
platypii wants to merge 1 commit into
microsoft:mainfrom
platypii:fix/jsdoc-private-alias-symlink-cache

Conversation

@platypii

Copy link
Copy Markdown

Summary

Fixes declaration emit incorrectly reusing an unrelated JSDoc import when referencing a type from a physically installed dependency.

Related issue: microsoft/TypeScript#63705
Reproduction: https://github.com/platypii/typescript-7-jsdoc-private-alias-repro

Root cause

For a normal, non-symlinked package, ResolvePackageDirectory returns a resolved package directory with an empty OriginalPath.

GetSymlinkCache appended package.json to that empty path before calling ProcessResolution. This produced the relative path package.json, which was then resolved against the project directory. As a result, the symlink cache incorrectly recorded the project root as a symlink to the dependency.

The module specifier generator could consequently treat a project source file as an alternative location for the dependency's type and reuse an unrelated JSDoc module specifier. This produced declarations such as:

schema: import("../src/types.js").SchemaElement[];

even though that module does not export SchemaElement.

Fix

Only add package-directory resolutions to the symlink cache when OriginalPath is non-empty. An empty OriginalPath means there is no symlink mapping to record.

A regression test verifies that declaration emit instead references the type's public external source:

schema: import("external/src/types.js").SchemaElement[];

Reproduction note

The standalone reproduction contains .npmrc with install-links=true only because it models the dependency using a local file: package, which npm would otherwise install as a symlink.

Registry dependencies are normally installed as physical directories under node_modules, so users do not need this npm setting to encounter the bug in a real project.

Tests

  • go test ./internal/testrunner -run 'TestLocal/jsDeclarationEmitDoesNotReuseUnrelatedJSDocImport' -count=1
  • go test ./internal/compiler ./internal/modulespecifiers ./internal/symlinks

Copilot AI review requested due to automatic review settings July 31, 2026 22:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Ready to approve

The targeted guard matches resolver semantics and the regression test validates the corrected declaration output.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Prevents false symlink mappings for physical dependencies, fixing invalid module specifiers during declaration emit.

Changes:

  • Requires a non-empty OriginalPath before recording package symlinks.
  • Adds a compiler regression test and expected declaration baseline.
File summaries
File Description
internal/compiler/program.go Guards symlink-cache insertion for physical packages.
testdata/tests/cases/compiler/jsDeclarationEmitDoesNotReuseUnrelatedJSDocImport.ts Reproduces the declaration-emit regression.
testdata/baselines/reference/compiler/jsDeclarationEmitDoesNotReuseUnrelatedJSDocImport.js Verifies the correct external module specifier.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@platypii

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree company="Hyperparam"

@jakebailey

Copy link
Copy Markdown
Member

@typescript-bot test it

@typescript-automation

typescript-automation Bot commented Aug 1, 2026

Copy link
Copy Markdown

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
test top400 ✅ Started ✅ Results
perf test this faster ✅ Started 👀 Results

@typescript-automation

Copy link
Copy Markdown

@jakebailey
The results of the perf run you requested are in!

Here they are:

tsc

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-Unions - native
Errors 41 41 ~ ~ ~ p=1.000 n=12
Symbols 119,733 119,733 ~ ~ ~ p=1.000 n=12
Types 99,032 99,032 ~ ~ ~ p=1.000 n=12
Memory Used 151,575k (± 0.55%) 151,191k (± 0.50%) ~ 149,349k 152,958k p=0.514 n=12
Memory Allocs 2,262,999 (± 0.00%) 2,263,013 (± 0.00%) ~ 2,262,788 2,263,222 p=1.000 n=12
Config Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Parse Time 0.041s (± 2.48%) 0.041s (± 3.57%) ~ 0.037s 0.045s p=0.630 n=12
Bind Time 0.013s (±13.84%) 0.011s (±10.36%) ~ 0.010s 0.016s p=0.058 n=12
Check Time 0.728s (± 0.50%) 0.733s (± 0.65%) ~ 0.724s 0.746s p=0.052 n=12
Emit Time 0.287s (± 1.43%) 0.284s (± 1.23%) ~ 0.272s 0.289s p=0.354 n=12
Total Time 1.076s (± 0.62%) 1.076s (± 0.42%) ~ 1.063s 1.087s p=0.744 n=12
angular-1 - native
Errors 3 3 ~ ~ ~ p=1.000 n=12
Symbols 871,832 (± 0.10%) 871,444 (± 0.12%) ~ 869,245 874,737 p=0.410 n=12
Types 263,962 (± 0.00%) 263,962 (± 0.00%) ~ 263,958 263,964 p=0.946 n=12
Memory Used 806,333k (± 0.06%) 806,622k (± 0.09%) ~ 805,096k 808,875k p=0.713 n=12
Memory Allocs 13,123,773 (± 0.07%) 13,113,231 (± 0.02%) -10,541 (- 0.08%) 13,109,557 13,118,536 p=0.003 n=12
Config Time 0.016s (± 1.75%) 0.016s (± 1.51%) ~ 0.016s 0.017s p=1.000 n=12
Parse Time 0.279s (± 1.91%) 0.281s (± 2.91%) ~ 0.260s 0.295s p=0.541 n=12
Bind Time 0.062s (± 7.00%) 0.064s (±10.72%) ~ 0.058s 0.094s p=0.508 n=12
Check Time 0s 0s ~ ~ ~ p=1.000 n=12
Emit Time 1.699s (± 1.17%) 1.707s (± 1.31%) ~ 1.663s 1.780s p=0.542 n=12
Total Time 2.068s (± 1.29%) 2.081s (± 1.25%) ~ 2.034s 2.172s p=0.355 n=12
mui-docs - native
Errors 11,277 (± 0.02%) 11,275 (± 0.04%) ~ 11,258 11,282 p=0.656 n=12
Symbols 4,387,605 4,387,605 ~ ~ ~ p=1.000 n=12
Types 1,540,346 1,540,346 ~ ~ ~ p=1.000 n=12
Memory Used 4,965,332k (± 0.03%) 4,965,286k (± 0.03%) ~ 4,961,511k 4,969,062k p=0.713 n=12
Memory Allocs 45,307,999 (± 0.03%) 45,311,607 (± 0.04%) ~ 45,283,888 45,384,616 p=1.000 n=12
Config Time 0.016s (± 1.14%) 0.016s ~ ~ ~ p=1.000 n=12
Parse Time 0.558s (± 1.47%) 0.546s (± 2.92%) ~ 0.502s 0.576s p=0.354 n=12
Bind Time 0.002s 0.002s ~ ~ ~ p=1.000 n=12
Check Time 16.235s (± 0.48%) 16.322s (± 0.44%) ~ 16.132s 16.449s p=0.131 n=12
Emit Time 0.464s (± 3.03%) 0.457s (± 3.54%) ~ 0.440s 0.518s p=0.896 n=12
Total Time 17.980s (± 0.48%) 18.070s (± 0.43%) ~ 17.846s 18.225s p=0.213 n=12
self-build-src - native
Errors 0 0 ~ ~ ~ p=1.000 n=12
Symbols 1,394,491 1,394,491 ~ ~ ~ p=1.000 n=12
Types 442,212 442,212 ~ ~ ~ p=1.000 n=12
Memory Used 1,639,173k (± 0.19%) 1,631,002k (± 0.35%) -8,171k (- 0.50%) 1,614,692k 1,649,259k p=0.008 n=12
Memory Allocs 97,107,646 (± 0.02%) 97,048,008 (± 0.04%) -59,638 (- 0.06%) 96,914,686 97,167,654 p=0.002 n=12
Config Time 0.003s (±11.47%) 0.003s (±16.58%) ~ 0.002s 0.004s p=0.856 n=12
Parse Time 0.197s (± 2.12%) 0.198s (± 2.44%) ~ 0.187s 0.209s p=0.745 n=12
Bind Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Check Time 2.038s (± 0.32%) 2.042s (± 0.32%) ~ 2.029s 2.069s p=0.523 n=12
Emit Time 0.307s (± 3.72%) 0.304s (± 2.28%) ~ 0.285s 0.320s p=0.832 n=12
Total Time 27.360s (± 0.68%) 27.463s (± 0.55%) ~ 27.009s 27.852s p=0.178 n=12
self-compiler - native
Errors 0 0 ~ ~ ~ p=1.000 n=12
Symbols 337,834 337,834 ~ ~ ~ p=1.000 n=12
Types 199,561 199,561 ~ ~ ~ p=1.000 n=12
Memory Used 318,357k (± 0.03%) 318,436k (± 0.07%) ~ 318,123k 319,480k p=1.000 n=12
Memory Allocs 4,659,410 (± 0.01%) 4,659,115 (± 0.01%) ~ 4,657,799 4,660,311 p=0.478 n=12
Config Time 0.001s 0.001s ~ ~ ~ p=1.000 n=12
Parse Time 0.113s (± 4.01%) 0.118s (± 3.57%) ~ 0.107s 0.126s p=0.138 n=12
Bind Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Check Time 1.219s (± 0.69%) 1.216s (± 0.56%) ~ 1.199s 1.235s p=0.560 n=12
Emit Time 0.167s (± 6.68%) 0.167s (± 6.29%) ~ 0.145s 0.203s p=0.832 n=12
Total Time 1.551s (± 0.93%) 1.553s (± 0.81%) ~ 1.525s 1.589s p=0.921 n=12
ts-pre-modules - native
Errors 87 87 ~ ~ ~ p=1.000 n=12
Symbols 302,246 302,246 ~ ~ ~ p=1.000 n=12
Types 181,594 181,594 ~ ~ ~ p=1.000 n=12
Memory Used 274,440k (± 0.02%) 274,451k (± 0.02%) ~ 274,209k 274,624k p=0.504 n=12
Memory Allocs 1,661,250 (± 0.01%) 1,661,364 (± 0.02%) ~ 1,660,747 1,662,430 p=0.630 n=12
Config Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Parse Time 0.099s (± 4.88%) 0.097s (± 3.63%) ~ 0.087s 0.108s p=0.504 n=12
Bind Time 0.038s (±12.84%) 0.035s (± 9.46%) ~ 0.029s 0.045s p=0.337 n=12
Check Time 0.984s (± 0.67%) 0.987s (± 0.52%) ~ 0.974s 1.003s p=0.451 n=12
Emit Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Total Time 1.135s (± 0.71%) 1.133s (± 0.47%) ~ 1.122s 1.152s p=0.831 n=12
vscode - native
Errors 0 0 ~ ~ ~ p=1.000 n=12
Symbols 7,631,101 7,631,101 ~ ~ ~ p=1.000 n=12
Types 2,654,831 2,654,831 ~ ~ ~ p=1.000 n=12
Memory Used 5,083,535k (± 0.02%) 5,083,555k (± 0.02%) ~ 5,081,390k 5,085,425k p=0.799 n=12
Memory Allocs 38,640,128 (± 0.04%) 38,639,360 (± 0.03%) ~ 38,624,138 38,680,420 p=0.932 n=12
Config Time 0.059s (± 0.55%) 0.059s (± 0.56%) ~ 0.058s 0.059s p=1.000 n=12
Parse Time 1.510s (± 1.64%) 1.491s (± 1.85%) ~ 1.381s 1.543s p=0.318 n=12
Bind Time 0.306s (± 0.43%) 0.325s (±12.82%) ~ 0.302s 0.536s p=0.916 n=12
Check Time 9.734s (± 0.37%) 9.688s (± 0.92%) ~ 9.284s 9.753s p=0.193 n=12
Emit Time 2.792s (± 0.85%) 3.009s (±10.31%) ~ 2.724s 4.426s p=0.098 n=12
Total Time 14.416s (± 0.24%) 14.587s (± 1.67%) ~ 14.350s 15.709s p=0.247 n=12
webpack - native
Errors 2 2 ~ ~ ~ p=1.000 n=12
Symbols 196,816 196,816 ~ ~ ~ p=1.000 n=12
Types 340 340 ~ ~ ~ p=1.000 n=12
Memory Used 230,680k (± 0.07%) 230,616k (± 0.06%) ~ 230,464k 230,994k p=1.000 n=12
Memory Allocs 975,530 (± 0.43%) 973,514 (± 0.15%) ~ 970,621 977,348 p=0.755 n=12
Config Time 0.008s 0.008s ~ ~ ~ p=1.000 n=12
Parse Time 0.192s (± 2.52%) 0.192s (± 2.75%) ~ 0.176s 0.202s p=0.660 n=12
Bind Time 0s 0s ~ ~ ~ p=1.000 n=12
Check Time 0s 0s ~ ~ ~ p=1.000 n=12
Emit Time 0.053s (±13.24%) 0.054s (±16.07%) ~ 0.043s 0.088s p=0.559 n=12
Total Time 0.253s (± 2.63%) 0.255s (± 2.06%) ~ 0.242s 0.273s p=0.504 n=12
xstate-main - native
Errors 0 0 ~ ~ ~ p=1.000 n=12
Symbols 1,066,055 1,066,055 ~ ~ ~ p=1.000 n=12
Types 392,847 392,847 ~ ~ ~ p=1.000 n=12
Memory Used 633,785k (± 0.02%) 633,830k (± 0.01%) ~ 633,637k 634,099k p=0.443 n=12
Memory Allocs 4,919,482 (± 0.03%) 4,918,990 (± 0.04%) ~ 4,916,388 4,924,654 p=0.478 n=12
Config Time 0.003s (± 6.23%) 0.003s ~ ~ ~ p=1.000 n=12
Parse Time 0.154s (± 3.11%) 0.153s (± 3.01%) ~ 0.142s 0.164s p=0.619 n=12
Bind Time 0.043s (±12.71%) 0.053s (±14.73%) ~ 0.035s 0.070s p=0.086 n=12
Check Time 1.276s (± 0.54%) 1.277s (± 0.56%) ~ 1.266s 1.302s p=0.898 n=12
Emit Time 0.001s 0.001s ~ ~ ~ p=1.000 n=12
Total Time 1.484s (± 0.67%) 1.493s (± 0.50%) ~ 1.472s 1.508s p=0.155 n=12
System info unknown
Hosts
  • native
Scenarios
  • Compiler-Unions - native
  • angular-1 - native
  • mui-docs - native
  • self-build-src - native
  • self-compiler - native
  • ts-pre-modules - native
  • vscode - native
  • webpack - native
  • xstate-main - native
Benchmark Name Iterations
Current pr 12
Baseline baseline 12

Developer Information:

Download Benchmarks

@typescript-automation

Copy link
Copy Markdown

@jakebailey Here are the results of running the top 400 repos with tsc comparing main and refs/pull/4813/merge:

Everything looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants