fix(exec): npx to run specified version if multiple version exist globally #7587
Conversation
milaninfy
force-pushed
the
mm/fix-npx-version-issue
branch
2 times, most recently
from
7f945d2
to
2a9292c
Compare
milaninfy
force-pushed
the
mm/fix-npx-version-issue
branch
from
2a9292c
to
95c6997
Compare
milaninfy
changed the title
milaninfy
force-pushed
the
mm/fix-npx-version-issue
branch
from
95c6997
to
afe4815
Compare
workspaces/libnpmexec/lib/index.js
Outdated
| @@ -41,6 +41,10 @@ const missingFromTree = async ({ spec, tree, flatOptions, isNpxTree }) => { | |||
| // registry spec that is not a specific tag. | |||
| const nodesBySpec = tree.inventory.query('packageName', spec.name) | |||
| for (const node of nodesBySpec) { | |||
| // continue if node is not at specified depth, no need to check further | |||
| if (depth !== undefined && node.depth !== depth) { | |||
There was a problem hiding this comment.
This is a great solution. However we are currently not really using depth so much as telling this function to only look at depth 0.
It would probably make more sense in the future when reading this if this was called something like shallow and was a Boolean. and then if (shallow && node.depth) would be the test for continuing.
There was a problem hiding this comment.
Makes sense.. Corrected in latest push.
milaninfy
force-pushed
the
mm/fix-npx-version-issue
branch
2 times, most recently
from
f891839
to
f966233
Compare
workspaces/libnpmexec/lib/index.js
Outdated
| const { manifest: globalManifest } = | ||
| await missingFromTree({ spec, tree: globalTree, flatOptions }) | ||
| if (!globalManifest && await fileExists(`${globalBin}/${args[0]}`)) { | ||
| const { node: globalNode } = |
There was a problem hiding this comment.
Why did we switch from manifest to node here?
There was a problem hiding this comment.
Initially I kept both, but the missingFromTree function in all code path it returns either { node } or { manifest } not sure if there ever a case if manifest is null and node is null as well, and also later we were checking for !manifest so changed to node, but I think now that you pointed it out it wont make a difference even if I keep it as it was earlier.
milaninfy
force-pushed
the
mm/fix-npx-version-issue
branch
from
f966233
to
72125a8
Compare
When multiple version of the same package is exist globally either at top level or at any level as a sub dependency, even though the version specified does not exist at top level it was running top level bin since it matches the bin name.
This fixes checks for depth of the found node along with already existing specs checks.
Fixes: #7486