Repository navigation
Check package reachability before using exports in declarations - #64599
Open
Mateusz Burzyński (Andarist) wants to merge 1 commit into
Open
Mateusz Burzyński (Andarist) wants to merge 1 commit into
Mateusz Burzyński (Andarist) wants to merge 1 commit into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 1 · 1
The boolean condition on line 770 relies on operator precedence (|| vs &&) and is difficult to… · New
The comment explains the node_modules reachability requirement, but the actual check also rejects… · New
Fixes a TypeScript module specifier generation bug where package exports could return a bare specifier before validating that the importing file can actually reach the corresponding node_modules folder.
Changes:
- Reorders reachability validation to occur before consulting package.json exports when computing node module specifiers.
- Adds a compiler test case covering unreachable-package scenarios involving exports (root, subpath, and pattern exports).
- Adds baselines for both module=preserve and module=nodenext for the new test.
| File | Description |
|---|---|
| tsc/internal/modulespecifiers/specifiers.go | Moves node_modules reachability check earlier to prevent exports from bypassing validation. |
| tsc/testdata/tests/cases/compiler/declarationEmitUnreachablePackageExports.ts | New repro test for declaration emit when exports paths are unreachable from the importer. |
| tsc/testdata/baselines/reference/compiler/declarationEmitUnreachablePackageExports(module=preserve).js | Baseline output for the new test under module=preserve. |
| tsc/testdata/baselines/reference/compiler/declarationEmitUnreachablePackageExports(module=preserve).errors.txt | Baseline errors for the new test under module=preserve. |
| tsc/testdata/baselines/reference/compiler/declarationEmitUnreachablePackageExports(module=nodenext).js | Baseline output for the new test under module=nodenext. |
| tsc/testdata/baselines/reference/compiler/declarationEmitUnreachablePackageExports(module=nodenext).errors.txt | Baseline errors for the new test under module=nodenext. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
Comment thread
tsc/internal/modulespecifiers/specifiers.go
Comment on lines
+769
to
+770
|
|
||
| if !stringutil.HasPrefix(info.SourceDirectory, pathToTopLevelNodeModules, caseSensitive) || len(globalTypingsCacheLocation) > 0 && stringutil.HasPrefix(globalTypingsCacheLocation, pathToTopLevelNodeModules, caseSensitive) { |
Comment thread
tsc/internal/modulespecifiers/specifiers.go
Comment on lines
+765
to
+772
| // Check reachability before consulting package.json exports, which can return | ||
| // a bare specifier directly. An exported path is only usable if the importing | ||
| // file can reach the package through node_modules. | ||
| pathToTopLevelNodeModules := pathObj.FileName[0:parts.TopLevelNodeModulesIndex] | ||
|
|
||
| if !stringutil.HasPrefix(info.SourceDirectory, pathToTopLevelNodeModules, caseSensitive) || len(globalTypingsCacheLocation) > 0 && stringutil.HasPrefix(globalTypingsCacheLocation, pathToTopLevelNodeModules, caseSensitive) { | ||
| return "" | ||
| } |
Copilot started reviewing on behalf of
Mateusz Burzyński (Andarist)
October 2, 2026 17:46
View session
Copilot stopped reviewing on behalf of
Mateusz Burzyński (Andarist) due to an error
October 2, 2026 17:47
Wesley Wigham (weswigham)
approved these changes
Oct 6, 2026
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.
fixes #64590
The issue was causedd by an early return~, the code sequence was roughly was:
So the step 2 bypassed the step 3's validation~ entirely. The fix simply relies on reordering the steps listed above. It now validates the reachability before checking if the exports have the specifier.