Skip to content

fix: route UNC paths through win32 instead of posix - #644

Merged
alexander-akait merged 3 commits into
mainfrom
claude/enhanced-resolve-unc-paths
Jul 28, 2026
Merged

fix: route UNC paths through win32 instead of posix#644
alexander-akait merged 3 commits into
mainfrom
claude/enhanced-resolve-unc-paths

Conversation

@alexander-akait

@alexander-akait alexander-akait commented Jul 28, 2026

Copy link
Copy Markdown
Member

Summary

getType accepted only the DOS device forms (\\?\, \\.\) as windows-absolute, so a plain UNC share fell through to Normal and every helper answered it with posix semantics. path.win32 roots any path starting with two backslashes, so both forms are windows-absolute and both now take that branch.

call before after / path.win32
dirname("\\\\server\\share\\a\\b") "." \\server\share\a
join("\\\\server\\share\\a", "./index.js") \\server\share\a/index.js \\server\share\a\index.js
normalize("\\\\server\\share\\a\\..\\b") unchanged \\server\share\b
Resolver.isModule("\\\\server\\share\\a") true false

The dirname and isModule rows are the ones that bite: walking up from a share to find a package.json gave up immediately at ".", and an absolute share path was reported as a bare module request, so it was looked for in node_modules instead of being resolved as a path. Resolving anything on a network share was broken as a result.

getDosDeviceType is gone since the two forms no longer need telling apart, which also drops a call from getType; it measures marginally faster (89 ms vs 94 ms per 11M calls on a realistic mix of requests).

The helpers pick a flavor by path shape and then have to answer the way that flavor does, so test/path.test.js now asserts that directly against path.win32/path.posix — a curated list of shapes plus 5000 seeded generated ones, comparing the flavor, normalize, dirname and join (~270 ms). Reverting this fix fails 22 of them.

Every shape we answer differently on purpose is listed in one place and asserted on its own, next to what node does with it, so a divergence can only be added deliberately:

  • A leading forward slash is a posix root, however the separator after it is spelled — path.win32 reads //server/share and /\server\share as UNC roots.
  • A single leading backslash is a rooted path on Windows and an ordinary filename character everywhere else; nothing in the string tells the two apart.
  • Drive-relative paths (C:foo, no separator after the colon) stay Normal, as they already did on main. path.win32 gives them the root C: but reports them as not absolute, and PathType has no windows-relative member — classifying them as AbsoluteWin would make join treat them as absolute and drop the root, trading one misalignment for another. Worth its own change.
  • Relative, empty and internal (#…) requests, where the resolver deliberately keeps its own semantics (a ./ prefix is preserved, an empty path stays empty).

Found while reviewing #643, which fixes the restriction boundary check. That PR is independent of this one — its comparison already handles UNC — and they touch different parts of lib/util/path.js.

What kind of change does this PR introduce?

fix

Did you add tests for your changes?

Yes. test/unc-paths.test.js resolves against a UNC context through the public API — a relative request, a nested directory, an absolute share request from an unrelated context, and a description-file main — all four fail on main and pass here. They run on every host against a fake filesystem, since a real share cannot be mounted in CI and the resolver picks the path flavor from the path rather than from the host. test/path.test.js covers getType, normalize, join and dirname directly, and adds the alignment suite described above.

Does this PR introduce a breaking change?

No, unless a posix path literally starting with \\ was being resolved, which would previously have been treated as a relative filename.

If relevant, what needs to be documented once your changes are merged?

n/a

Use of AI

Written with Claude Code. It compared every helper against path.win32/path.posix before and after — a curated matrix plus 70k generated shapes — wrote the failing tests first, mutation-tested the new alignment suite to confirm it catches the regression, and benchmarked getType to confirm the hot path did not regress; I reviewed the result. npm run lint, npm run test:only and npm run test:browser all pass locally.

`getType` only accepted the DOS device forms (`\\?\`, `\\.\`) as
windows-absolute, so a plain share fell through to `Normal` and every helper
answered it with posix semantics: `dirname("\\\\server\\share\\a\\b")` was
`"."`, `join` produced the mixed `\\server\share\a/index.js`, `normalize`
left `.` and `..` segments in place, and `Resolver.isModule` reported a share
path as a bare module request, sending it to `node_modules` instead of
resolving it as a path.

`path.win32` roots any path starting with two backslashes, so both forms are
windows-absolute and both now take that branch. A single leading backslash
stays out of it: on Windows it is a rooted path, everywhere else an ordinary
filename character, and nothing in the string tells the two apart.
@changeset-bot

changeset-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 905586c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
enhanced-resolve Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.11%. Comparing base (2bcac6e) to head (5881591).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #644      +/-   ##
==========================================
- Coverage   98.12%   98.11%   -0.01%     
==========================================
  Files          49       49              
  Lines        9902    10010     +108     
==========================================
+ Hits         9716     9821     +105     
- Misses        186      189       +3     
Flag Coverage Δ
integration 98.11% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

`path.win32` reads any two leading separators as a UNC root, including the
mixed `/\` spelling. A leading forward slash is unambiguously a posix root
here, so it keeps posix semantics whatever follows it.
@codspeed-hq

codspeed-hq Bot commented Jul 28, 2026

Copy link
Copy Markdown

Merging this PR will regress 1 benchmark

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 140 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation extensions-many: 6-extension list (warm) 994.2 µs 1,344.2 µs -26.04%
Memory node-compare: node require.resolve x 1000 29.2 KB 21.2 KB +37.49%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing claude/enhanced-resolve-unc-paths (5881591) with main (d8693b6)

Open in CodSpeed

The helpers pick a flavor by path shape and then have to answer the way that
flavor does. That is now asserted directly rather than case by case: for a
curated list of shapes and for 5000 seeded generated ones, the flavor,
`normalize`, `dirname` and `join` are compared against `path.win32` or
`path.posix`, whichever node reads the path as.

Every shape we answer differently on purpose is listed in one place and
asserted on its own, together with what node does with it, so a divergence
can only be added deliberately: a leading forward slash, a single leading
backslash, a drive-relative path, and relative, empty and internal requests.
@alexander-akait
alexander-akait merged commit 10fcea0 into main Jul 28, 2026
36 of 38 checks passed
@alexander-akait
alexander-akait deleted the claude/enhanced-resolve-unc-paths branch July 28, 2026 17:21
alexander-akait pushed a commit that referenced this pull request Jul 28, 2026
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## enhanced-resolve@5.24.4

### Patch Changes

- Keep the original request resolvable when `extensionAlias` lists its
own extension. (by
[@alexander-akait](https://github.com/alexander-akait) in
[#641](#641))

- Fix string `restrictions` boundary checks: a restriction ending with a
separator no longer rejects everything inside it, restrictions are
normalized before they are compared, and a Windows path now matches the
way `path.win32` does, treating `/` and `\` as interchangeable and
comparing case-insensitively, while `\` stays a filename character in a
posix path. The same comparison backs `tsconfig` path matching. (by
[@alexander-akait](https://github.com/alexander-akait) in
[#643](#643))

- Treat a UNC path (`\\server\share\…`) as a Windows path, so it
normalizes, joins and walks up with `path.win32` semantics instead of
being taken for a bare module request. (by
[@alexander-akait](https://github.com/alexander-akait) in
[#644](#644))

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

1 participant