Appending node_modules/.bin for each subdirectory of the current folder to PATH variable (specifically for monorepos)

I thought this one was hard/next to impossible to do safely for Windows because of the short limit of the path variable there? There was a comment to this effect on an earlier thread.

Appending node_modules/.bin for each subdirectory of the current folder to PATH variable (specifically for monorepos)

I thought this one was hard/next to impossible to do safely for Windows because of the short limit of the path variable there? There was a comment to this effect on an earlier thread.

I found the following solution online, might fix it:

• Create a new system environment variable, say 'NEWPATH'
• Assign the bin directory location to 'NEWPATH'
• Now append '; %NEWPATH%' to the PATH environment variable

Since the path part is potentially problematic, should it be a separate PR? The traversing up the tree part is straightforward and could just land I'd think.

Also, checking if the node_modules/.bin directory for each ancestor actually exists before adding it to PATH could possibly alleviate the issue.

Also, checking if the node_modules/.bin directory for each ancestor actually exists before adding it to PATH could possibly alleviate the issue.

This would reduce the performance by a significant margin, and isn't in par with npm (but that's not a supportive argument just a fact)

This would reduce the performance by a significant margin

It's a very minor cost compared to the other work that's being done, and skipping these checks would really just offload the work to the subprocesses being run (such as the shell), which would have to look through all of these nonexistent paths for binaries to execute, so it doesn't prevent the performance cost.

Appending node_modules/.bin for each subdirectory of the current folder to PATH variable (specifically for monorepos)

I think you mean each parent directory, not each sub directory?

I haven't checked the implementation, but you can probably get away with only adding node_modules/.bin to the path for every parent directory you find containing a package.json file, and can also further check if {dir}/node_modules/.bin is a directory. It adds some stats, but it'll help keep the PATH env within limits, and if you don't do those stats, then the OS will end up doing them anyway when it goes to look up executables. (This is close to what npm does.)

Also, the node_modules/.bin should be pre-pended to the PATH env, not appended. Eg, if I have eslint installed globally, and a different version in the current project, I want the one in the closest containing project that has it, not the one at /usr/local/bin/eslint.

@isaacs Thanks for the comment!

I think you mean each parent directory, not each sub directory?

Yes, I've updated the CLI to use ancestor keyword.

I haven't checked the implementation, but you can probably get away with only adding node_modules/.bin to the path for every parent directory you find containing a package.json file, and can also further check if {dir}/node_modules/.bin is a directory. It adds some stats, but it'll help keep the PATH env within limits, and if you don't do those stats, then the OS will end up doing them anyway when it goes to look up executables. (This is close to what npm does.)

Why do you think checking package_json is good? I'm currently checking if node_modules/.bin is a directory.

Also, the node_modules/.bin should be pre-pended to the PATH env, not appended. Eg, if I have eslint installed globally, and a different version in the current project, I want the one in the closest containing project that has it, not the one at /usr/local/bin/eslint.

Yes, I'm pre-pending although I'm not sure how did you come to this conclusion? If it's my choice of words in the docs, can you point me to the correct place to fix it?

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

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

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

- Loading data for nodejs/node/pull/53154
✔  Done loading data for nodejs/node/pull/53154
----------------------------------- PR info ------------------------------------
Title      src: traverse parent folders while running `--run` (#53154)
Author     Yagiz Nizipli  (@anonrig)
Branch     anonrig:traverse-until-root -> nodejs:main
Labels     c++, semver-minor, needs-ci
Commits    1
 - src: traverse parent folders while running `--run`
Committers 1
 - Yagiz Nizipli 
PR-URL: https://github.com/nodejs/node/pull/53154
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Daniel Lemire 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/53154
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Daniel Lemire 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sun, 26 May 2024 00:20:07 GMT
   ✔  Approvals: 2
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/53154#pullrequestreview-2086340450
   ✔  - Daniel Lemire (@lemire): https://github.com/nodejs/node/pull/53154#pullrequestreview-2088970099
   ✘  Last GitHub CI failed
   ℹ  Last Full PR CI on 2024-05-31T11:06:06Z: https://ci.nodejs.org/job/node-test-pull-request/59561/
- Querying data for job/node-test-pull-request/59561/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

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

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

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

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

Landed in 5731e2b