Skip to content
This repository has been archived by the owner on Jan 19, 2022. It is now read-only.

feat: add walk up dir lookup to satisfy local bins #2

Closed
wants to merge 2 commits into from

Conversation

ruyadorno
Copy link
Contributor

@ruyadorno ruyadorno commented Apr 27, 2021

Currently @npmcli/run-script already supports this walk up lookup logic
that allows any nested folder to use a bin that is located inside a
node_modules/.bin folder higher in the directory tree.

This commit adds the same logic from:
https://github.com/npm/run-script/blob/47a4d539fb07220e7215cc0e482683b76407ef9b/lib/set-path.js#L24-L28
to libnpmexec so that users may use a binary from a dependency of a
nested workspace in the context of that workspace's folder.

References

Fixes: npm/cli#2826

@ruyadorno ruyadorno requested review from isaacs and a team April 27, 2021 19:44
Copy link
Contributor

@isaacs isaacs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine, but we should pull out the "walk up the path adding nodE_modules/.bin to things" into a standalone package that both run-script and libnpmexec use, and we should at least have the ability to set a root dir that we don't allow it to walk up past.

Rough cut at a thing like that: https://gist.github.com/isaacs/3db535658c1cb0976126159d63286423

README.md Outdated Show resolved Hide resolved
lib/file-exists.js Outdated Show resolved Hide resolved
Currently @npmcli/run-script already supports this walk up lookup logic
that allows any nested folder to use a bin that is located inside a
node_modules/.bin folder higher in the directory tree.

This commit adds the same logic from:
https://github.com/npm/run-script/blob/47a4d539fb07220e7215cc0e482683b76407ef9b/lib/set-path.js#L24-L28
to libnpmexec so that users may use a binary from a dependency of a
nested workspace in the context of that workspaces' folder.

Fixes: npm/cli#2826
@ruyadorno
Copy link
Contributor Author

did the first pass aligning the implementation here to use walk-up-path, let's follow up with the module refactor later 😊 but this should be good to land now

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] npx doesn't work when in child workspace
2 participants