module,win: fix long path resolve by huseyinacacak-janea · Pull Request #53294 · nodejs/node · GitHub

Several issues were encountered with module loading due to extended path lengths on Windows. These have been addressed with corrections implemented across four distinct code locations, accompanied by the addition of a corresponding test case for each to ensure functionality.
Additionally, I've moved es-module/test-GH-50753.js to es-module/test-esm-long-path-win.js as the tests cover more cases than specified in that issue.

Fixes: #50753

@nodejs/platform-windows

I actually need more time to take a look, I think the implementation will solve the problem but I want to test in which cases we should include the UNC path, since we didn't do this in all paths in the toNamespacedPath.

There are some conditions to include the UNC that you are not following:

node/lib/path.js

Lines 632 to 645 in 58711c2

    
           if (StringPrototypeCharCodeAt(resolvedPath, 0) === CHAR_BACKWARD_SLASH) { 
        
             // Possible UNC root 
        
             if (StringPrototypeCharCodeAt(resolvedPath, 1) === CHAR_BACKWARD_SLASH) { 
        
               const code = StringPrototypeCharCodeAt(resolvedPath, 2); 
        
               if (code !== CHAR_QUESTION_MARK && code !== CHAR_DOT) { 
        
                 // Matched non-long UNC root, convert the path to a long UNC path 
        
                 return `\\\\?\\UNC\\${StringPrototypeSlice(resolvedPath, 2)}`; 
        
               } 
        
             } 
        
           } else if ( 
        
             isWindowsDeviceRoot(StringPrototypeCharCodeAt(resolvedPath, 0)) && 
        
             StringPrototypeCharCodeAt(resolvedPath, 1) === CHAR_COLON && 
        
             StringPrototypeCharCodeAt(resolvedPath, 2) === CHAR_BACKWARD_SLASH 
        
           ) {

Although the condition could be matched, I think it would be better to wait for the migration of toNamespacedPath to C++.

Maybe we could include the ToNamespacePath port in this PR, and then leave that other PR just to remove the JS version and rewrite the places they are used.

@anonrig What do you think?

CI: https://ci.nodejs.org/job/node-test-pull-request/59635/

I prefer to land this after the toNamespacedPath migration to C++. Unfortunately, I don't have the time to pursue this at the moment (afaict, there is only one failing test). I appreciate if you could took over that PR and apply these changes in a follow up PR.

I prefer to land this after the toNamespacedPath migration to C++. Unfortunately, I don't have the time to pursue this at the moment (afaict, there is only one failing test). I appreciate if you could took over that PR and apply these changes in a follow up PR.

in that case, can you convert the blocking "request changes" into a simple suggestion/friendly ask? I don't think this PR should be blocked on a PR that is not being currently worked on

CI: https://ci.nodejs.org/job/node-test-pull-request/59644/

I prefer to land this after the toNamespacedPath migration to C++. Unfortunately, I don't have the time to pursue this at the moment (afaict, there is only one failing test). I appreciate if you could took over that PR and apply these changes in a follow up PR.

Sure, no problem, I'll take over your PR and start working on it.

CI: https://ci.nodejs.org/job/node-test-pull-request/60050/

I prefer to land this after the toNamespacedPath migration to C++. Unfortunately, I don't have the time to pursue this at the moment (afaict, there is only one failing test). I appreciate if you could took over that PR and apply these changes in a follow up PR.

@anonrig I've rebased this PR to use toNamespacedPath. Could you please review it?

Is there anything else I can do to help this PR move forward?

Suggested change

      
                          env->isolate(), file_path.c_str(), strlen(file_path.c_str()))
          
                          env->isolate(), file_path.c_str(), file_path.size())

Fixed. Thanks.

ditto

Fixed.

Suggested change

      
                                path_value.ToString().back() ==
          
                                path_value.ToStringView().ends_with

Fixed.

ditto

Fixed.

The previous line is not needed path_value.ToString() creates an unnecessary string.

Can you also remove this in other places?

Fixed.

I've fixed the suggestions. Is there anything else I can do to help this PR move forward?

@anonrig Is there anything else we can do to help this PR move forward? I'm having many issues with long paths on Windows and am eager to get this fixed.

CI: https://ci.nodejs.org/job/node-test-pull-request/60944/

CI: https://ci.nodejs.org/job/node-test-pull-request/60972/

CI: https://ci.nodejs.org/job/node-test-pull-request/60974/

The failing test in the CI is a known flaky test.

CI: https://ci.nodejs.org/job/node-test-pull-request/60987/

Landed in 37f9eca

PR-URL: #53294 Fixes: #50753 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>

The old comment blamed a Node bug: package.json read silently failing past MAX_PATH, so `{"type":"module"}` was missed and ESM parsed as CJS. That was fixed in Node 22.7.0 (nodejs/node#53294, which namespaces fs, ESM resolution and package.json reads) and this package requires >=22.9.0, so the mechanism the comment described cannot be why we need the setting. The reason that does hold up is headroom, measured on this lockfile: the longest path relative to the repo root is 115 characters hoisted against 190 isolated (at Windows' default virtualStoreDirMaxLength of 60). An ordinary C:\Users\<first>.<last>\IdeaProjects\tomtom-traffic-analytics-mcp checkout is 63 characters, leaving isolated ~6 before MAX_PATH — a OneDrive-redirected Documents folder or a worktree subdirectory exceeds it. Enabling long paths instead needs an admin registry value plus a longPathAware manifest node.exe does not ship (nodejs/node#40641), which a public repo cannot require. Also records what the setting does and does not cost, since that was the review question. It costs strict isolation: an undeclared dependency resolves locally but breaks in the published bundle. It costs nothing measurable otherwise — 321 MB and equal install times either way (isolated 4.86/6.65/ 5.12s against hoisted 5.82/5.77/5.11s, warm store, frozen lockfile), because the store dedup comes from packageImportMethod, not from nodeLinker. Comment only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Jun 3, 2024

huseyinacacak-janea force-pushed the huseyin-11801-long-path-error branch from 3ee742c to 27a4505 Compare June 3, 2024 12:11

H4ad previously approved these changes Jun 3, 2024

View reviewed changes

StefanStojanovic added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 3, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 3, 2024

anonrig requested changes Jun 3, 2024

View reviewed changes

huseyinacacak-janea force-pushed the huseyin-11801-long-path-error branch from 27a4505 to 42b88bf Compare July 1, 2024 09:31

module,win: fix long path resolve

14a1de0

huseyinacacak-janea force-pushed the huseyin-11801-long-path-error branch from 42b88bf to 14a1de0 Compare July 2, 2024 05:37

StefanStojanovic added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 2, 2024

H4ad approved these changes Jul 2, 2024

View reviewed changes

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 3, 2024

H4ad requested a review from anonrig July 15, 2024 12:01

anonrig reviewed Jul 15, 2024

View reviewed changes

fixup! module,win: fix long path resolve

dae0bfe

anonrig reviewed Jul 15, 2024

View reviewed changes

Comment thread src/node_file.cc

fixup! module,win: fix long path resolve

0e2c739

anonrig approved these changes Aug 7, 2024

View reviewed changes

anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 7, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 7, 2024

H4ad added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Aug 10, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 10, 2024

nodejs-github-bot merged commit 37f9eca into nodejs:main Aug 10, 2024

targos pushed a commit that referenced this pull request Aug 14, 2024

module,win: fix long path resolve

e676d98

PR-URL: #53294 Fixes: #50753 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>

RafaelGSS mentioned this pull request Aug 19, 2024

v22.7.0 proposal #54452

Merged

This was referenced Sep 9, 2024

Upgrade to Node.js 22.7.0 karlhorky/node-js-max_path-windows-bug#2

Open

Long node_modules paths cannot be found on Windows when LongPathsEnabled enabled #50753

Closed

BalaM314 mentioned this pull request Sep 19, 2024

Node does not determine that a file is an ES module when the package.json is in a long path #54304

Closed

targos added the dont-land-on-v20.x label Sep 21, 2024

joyeecheung mentioned this pull request Jan 23, 2025

[v20.x backport] src: move package_json_reader cache to c++ #56590

Closed

wonu mentioned this pull request Feb 28, 2026

Subpath imports (#) fail on Windows when package.json path >= 260 chars (MAX_PATH) #62043

Closed

DanielForniessoria-TomTom mentioned this pull request Aug 18, 2026

chore: migrate to pnpm and Biome, and harden the MCPB bundle build tomtom-international/tomtom-traffic-analytics-mcp#37

Merged

17 tasks

-                    switch (FilePathIsFile(env, file_path)) {
+                    Local<Value> local_file_path =
+                        Buffer::Copy(
+                            env->isolate(), file_path.c_str(), strlen(file_path.c_str()))

-                  switch (FilePathIsFile(env, file_path)) {
+                  Local<Value> local_file_path =
+                      Buffer::Copy(
+                          env->isolate(), file_path.c_str(), strlen(file_path.c_str()))

+                // Check if the path has a trailing slash. If so, add it after
+                // ToNamespacedPath() as it will be deleted by ToNamespacedPath()
+                bool slashCheck = !path_value.ToString().empty() &&
+                                  path_value.ToString().back() ==

+                // Check if the path has a trailing slash. If so, add it after
+                // ToNamespacedPath() as it will be deleted by ToNamespacedPath()
+                bool slashCheck = !path_value.ToString().empty() &&
+                                  path_value.ToString().back() ==

                 bool slashCheck = !path_value.ToString().empty() &&
-                                  path_value.ToString().back() ==
-                                      std::filesystem::path::preferred_separator;
+                                  path_value.ToStringView().ends_with(

	env->isolate(), file_path.c_str(), strlen(file_path.c_str()))
	env->isolate(), file_path.c_str(), file_path.size())

	path_value.ToString().back() ==
	path_value.ToStringView().ends_with

module,win: fix long path resolve by huseyinacacak-janea · Pull Request #53294 · nodejs/node · GitHub

Uh oh!

Uh oh!

module,win: fix long path resolve - #53294

module,win: fix long path resolve#53294nodejs-github-bot merged 3 commits intonodejs:mainnodejs/node:mainfrom JaneaSystems:huseyin-11801-long-path-errorJaneaSystems/node:huseyin-11801-long-path-errorCopy head branch name to clipboard

Conversation

huseyinacacak-janea commented Jun 3, 2024

Uh oh!

targos commented Jun 3, 2024

Uh oh!

H4ad commented Jun 3, 2024

Uh oh!

nodejs-github-bot commented Jun 3, 2024

Uh oh!

anonrig left a comment

Choose a reason for hiding this comment

Uh oh!

MoLow commented Jun 3, 2024

Uh oh!

nodejs-github-bot commented Jun 4, 2024

Uh oh!

huseyinacacak-janea commented Jun 5, 2024

Uh oh!

nodejs-github-bot commented Jul 4, 2024

Uh oh!

huseyinacacak-janea commented Jul 5, 2024

Uh oh!

huseyinacacak-janea commented Jul 15, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

huseyinacacak-janea commented Jul 26, 2024

Uh oh!

QuantumQuin commented Aug 7, 2024

Uh oh!

nodejs-github-bot commented Aug 7, 2024

Uh oh!

nodejs-github-bot commented Aug 8, 2024

Uh oh!

nodejs-github-bot commented Aug 8, 2024

Uh oh!

huseyinacacak-janea commented Aug 9, 2024

Uh oh!

nodejs-github-bot commented Aug 9, 2024

Uh oh!

nodejs-github-bot commented Aug 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

module,win: fix long path resolve#53294
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
JaneaSystems:huseyin-11801-long-path-error