GeezerVison

FYI @nodejs/loaders @nodejs/fs

Can we add a test so we don't regress?

Of course. I'll add it next week.

Can we add a test so we don't regress?

Test's added.

@joyeecheung do you think this can land as a fix with a regression test added and then a follow-up PR can be opened for porting path.toNamespacedPath() logic and using it in here, but potentially in other places missing it as well?

Yes, with a TODO/FIXME comment.

@joyeecheung LGTY now?

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

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

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

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

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

@joyeecheung can you add the commit-queue label to try to land this again? Last time it failed to do so...

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

@StefanStojanovic @joyeecheung any chance of getting this released in one of the next patch or minor releases?

It's been a while since my original issue in Nov 2023 and users keep running into this (eg. with pnpm) and commenting that it's an open bug but the PR hasn't been merged yet.

Commit Queue failed

• Loading data for module,win: fix long path resolve #51097
  ✔ Done loading data for module,win: fix long path resolve #51097
  ----------------------------------- PR info ------------------------------------
  Title module,win: fix long path resolve (module,win: fix long path resolve #51097)
  ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile!
  Branch StefanStojanovic:mefi-resolve-long-path -> nodejs:main
  Labels c++, fs, needs-ci
  Commits 1
• module,win: fix long path resolve
  Committers 1
• StefanStojanovic
  PR-URL: module,win: fix long path resolve #51097
  Fixes: Long node_modules paths cannot be found on Windows when LongPathsEnabled enabled #50753
  Reviewed-By: Joyee Cheung
  ------------------------------ Generated metadata ------------------------------
  PR-URL: module,win: fix long path resolve #51097
  Fixes: Long node_modules paths cannot be found on Windows when LongPathsEnabled enabled #50753
  Reviewed-By: Joyee Cheung

---

⚠ Commits were pushed since the last approving review:
⚠ - module,win: fix long path resolve
ℹ This PR was created on Fri, 08 Dec 2023 13:28:00 GMT
✔ Approvals: 1
✔ - Joyee Cheung (@joyeecheung) (TSC): #51097 (review)
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2024-04-01T17:51:22Z: https://ci.nodejs.org/job/node-test-pull-request/58036/

• Querying data for job/node-test-pull-request/58036/
  ✔ Last Jenkins CI successful

---

✔ Aborted git node land session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/8596048782

@joyeecheung do you see why this couldn't land from this log?

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

Landed in 45f0dd0

It looks like the Node.js v22 PR contains the fix "module,win: fix long path resolve (by @StefanStojanovic) #51097":

• 2024-04-24, Version 22.0.0 (Current) #52505

So maybe Node.js has support for long paths on Windows now!

(make sure that you have LongPathsEnabled set to true on the Windows machine you test on)