fix(compiler): resolve packages located on UNC paths - #11998
Open
Scott Mc (scooter12) wants to merge 1 commit into
Open
Scott Mc (scooter12) wants to merge 1 commit into
Scott Mc (scooter12) wants to merge 1 commit into
Conversation
`pathToFileURL` returned `file://${path}`. For a UNC path
`//server/share/pkg` this gave `file:////server/share/pkg`. `resolvePath`
normalized the export target to `file:///server/share/pkg/...`, so the
`startsWith(packageUrl)` check failed and `tsp compile` threw
`INVALID_MODULE_EXPORT_TARGET`.
`pathToFileURL` now returns `file://server/share/pkg` for UNC paths, the
same as Node's `url.pathToFileURL`. `fileURLToPath` restores the leading
`//` when the authority is not a drive letter.
Fixes microsoft#11997
|
Azure Pipelines: Successfully started running 1 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused implementation and regression tests cover the reported UNC resolution failure.
Pull request overview
Fixes compiler module resolution for packages and imports located on Windows UNC paths.
Changes:
- Corrects UNC path-to-file-URL conversion and reverse conversion.
- Adds utility and module-resolution tests covering POSIX, drive-letter, and UNC paths.
- Adds the compiler changelog entry.
File summaries
| File | Description |
|---|---|
packages/compiler/src/module-resolver/utils.ts |
Fixes UNC URL conversion. |
packages/compiler/test/module-resolver/utils.test.ts |
Tests URL conversion and round trips. |
packages/compiler/test/module-resolver/module-resolver.test.ts |
Tests UNC package and import resolution. |
.chronus/changes/fix-11997-compiler-unc-module-resolution-2026-09-17.md |
Documents the compiler bug fix. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
commit: |
Contributor
|
All changed packages have been documented.
Show changes
|
Timothee Guerin (timotheeguerin)
left a comment
Member
There was a problem hiding this comment.
Scott Mc (@scooter12) Thanks for the fix, I think this looks good just need to accept the CLA for the checks to pass #11998 (comment)
Author
|
@microsoft-github-policy-service agree |
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 #11997
Problem
tsp compilefails withINVALID_MODULE_EXPORT_TARGETwhen the project is on a Windows network share. A mapped drive letter fails too, because the path resolves to\\server\share\....pathToFileURLinmodule-resolver/utils.tsreturnedfile://${path}. For a UNC path//server/share/pkgthis gavefile:////server/share/pkg(4 slashes).resolvePathnormalized the export target tofile:///server/share/pkg/...(3 slashes). TheresolvedTarget.startsWith(packageUrl)check inresolve-package-target.tsthen failed.Fix
One file, two functions:
pathToFileURLreturnsfile://server/share/pkgfor a UNC path. This is the same form as Node'surl.pathToFileURL, andpath-utilsalready handlesfile://server/as a root.fileURLToPathrestores the leading//when the authority is not a drive letter.Other paths give the same result as before.
Tests
utils.test.ts:pathToFileURLandfileURLToPathfor POSIX and UNC paths, and a round trip for POSIX, drive letter, and UNC paths.module-resolver.test.ts: a package withexportson a UNC path, and a project withimportson a UNC path. These use the existingmkFshelper.5 of the new tests fail without the fix. 3 of them fail with the same
INVALID_MODULE_EXPORT_TARGET/INVALID_MODULE_IMPORT_TARGETerror as the issue.