feat(pre_use): warn when another node, e.g. pnpm's, shadows vfox in PATH (#38)

Author: linghengqian

README.md

@@ -55,6 +55,10 @@ See [docs/source-build.md](docs/source-build.md) for details.
 
 See [docs/lts.md](docs/lts.md) for details.
 
+### pnpm
+
+See [docs/pnpm.md](docs/pnpm.md) for details.
+
 ### Testing
 
 See [docs/e2e.md](docs/e2e.md) for details.

docs/pnpm.md

@@ -0,0 +1,65 @@
+# pnpm
+
+pnpm as a package manager, for example from `npm install -g pnpm`, works
+next to this plugin. The conflict starts when pnpm also installs Node.js:
+`pnpm runtime set node <version> -g` (or the deprecated
+`pnpm env use --global <version>`) puts a second `node` into
+`$PNPM_HOME/bin`. Only the `node` that comes first in `PATH` runs, and with
+the shell setup pnpm writes that is pnpm's, whatever `vfox use` selected.
+
+Observed with pnpm 12.14.0 and vfox 1.0.12 in bash on Ubuntu.
+
+## Symptoms
+
+- `vfox use nodejs@24.21.0` prints `Now using nodejs@24.21.0` and
+  `vfox current nodejs` agrees, but `node --version` prints the
+  pnpm-managed version. Global, session and project scope are all affected.
+- `type -a node` lists `$PNPM_HOME/bin/node` before the vfox path.
+- In a directory with `.nvmrc` or `.node-version`, `node` runs a copy pnpm
+  downloads on first use, even when vfox has that version installed.
+- `npm install -g <pkg>` succeeds but the command is not found. pnpm's
+  runtime ships no npm, so vfox's `npm` runs on pnpm's `node` and installs
+  into pnpm's copy of Node.js under `$PNPM_HOME`.
+
+## Why
+
+1. `pnpm runtime set node -g` refuses to run until `$PNPM_HOME/bin` is on
+   `PATH` (`ERR_PNPM_GLOBAL_BIN_DIR_NOT_IN_PATH`). The fix it suggests,
+   `pnpm setup`, adds a block at the end of the shell rc file, below
+   `eval "$(vfox activate bash)"`, that prepends `$PNPM_HOME/bin` to `PATH`.
+2. At every prompt `vfox env` keeps the `PATH` entries that come before its
+   first own path ahead of all SDK paths, so that an activated Python
+   virtualenv keeps priority. `$PNPM_HOME/bin` is such an entry.
+3. That `node` is pnpm's shim: it runs pnpm's global Node.js, or the version
+   the current project pins through `devEngines.runtime`, `engines.runtime`,
+   `.node-version` or `.nvmrc`
+   ([project-aware global bins](https://pnpm.io/global-packages#project-aware-global-bins)).
+   pnpm's `globalShims` setting and `PNPM_SHIM_BYPASS=1` only turn off the
+   project lookup; `node` still runs pnpm's global Node.js, not vfox's.
+
+`vfox use nodejs@<version>` prints a note when a `node` sits ahead of vfox
+this way, whichever tool put it there:
+
+```text
+Note: /home/user/.local/share/pnpm/bin/node comes before vfox in PATH, so `node` will not run nodejs@24.21.0. See https://github.com/version-fox/vfox-nodejs/blob/main/docs/pnpm.md
+```
+
+## Keeping vfox in charge of `node`
+
+Remove the Node.js that pnpm installed globally. pnpm keeps working, since
+`pnpm setup` copied it into `$PNPM_HOME`:
+
+```sh
+pnpm remove -g node
+```
+
+Activating vfox below pnpm's block in the rc file is not enough: pnpm's
+block prepends `$PNPM_HOME/bin` again unless it already comes first, so
+nested shells put pnpm's `node` back in front.
+
+Projects that pin a runtime with `devEngines.runtime` keep it for scripts:
+after `pnpm install`, `pnpm run` and `pnpm exec` use the pinned Node.js from
+`node_modules/.bin`, while a bare `node` follows vfox.
+
+To let pnpm manage Node.js instead, do not select `nodejs` with vfox in the
+same shell.

hooks/pre_use.lua

@@ -26,6 +26,21 @@ function PLUGIN:PreUse(ctx)
         version = shorthands[version]
     end
 
+    -- vfox reports the switch even when another `node` keeps winning PATH
+    -- lookup, so say so here instead of leaving `node --version` to
+    -- contradict it.
+    local shadowing = util.shadowing_node()
+    if version ~= nil and shadowing ~= nil then
+        print(
+            "Note: "
+                .. shadowing
+                .. " comes before vfox in PATH, so `node` will not run nodejs@"
+                .. version
+                .. ". See "
+                .. util.PnpmDocUrl
+        )
+    end
+
     --- return the version information
     return {
         version = version,

lib/util.lua

@@ -19,6 +19,7 @@ UTIL.FileName = "node-v%s-%s-%s%s"
 UTIL.VersionSourceUrl = "/index.json"
 UTIL.UnofficialBaseUrl = "https://unofficial-builds.nodejs.org/download/release"
 UTIL.ReadmeUrl = "https://github.com/version-fox/vfox-nodejs/blob/main/README.md"
+UTIL.PnpmDocUrl = "https://github.com/version-fox/vfox-nodejs/blob/main/docs/pnpm.md"
 
 function UTIL.getBaseUrl()
     local mirror = os.getenv("VFOX_NODEJS_MIRROR")
@@ -120,6 +121,51 @@ function UTIL.libstdcxx_hint()
     return "the libstdc++ system package (libstdcpp on OpenWrt)"
 end
 
+--- Whether a PATH entry belongs to vfox: like vfox's own check, one of its
+--- segments is `.vfox` (or the legacy `.version-fox`).
+local function is_vfox_path(dir)
+    for segment in string.gmatch(dir, "[^/\\]+") do
+        if segment == ".vfox" or segment == ".version-fox" then
+            return true
+        end
+    end
+    return false
+end
+
+--- First `node` that `vfox env` keeps ahead of the SDK paths. vfox leaves
+--- PATH entries before its first own path in front, as user-injected (so an
+--- activated virtualenv keeps priority); a `node` there, such as the one
+--- `pnpm runtime set node -g` writes to `$PNPM_HOME/bin`, shadows the
+--- version `vfox use` selects. With no vfox path on PATH yet, vfox puts its
+--- paths first and nothing shadows them.
+--- See https://github.com/version-fox/vfox-nodejs/blob/main/docs/pnpm.md
+--- @return string|nil path of the shadowing executable
+function UTIL.shadowing_node()
+    local path = os.getenv("PATH")
+    if path == nil or path == "" then
+        return nil
+    end
+    local windows = RUNTIME ~= nil and RUNTIME.osType == "windows"
+    local separator = windows and ";" or ":"
+    local names = windows and { "\\node.exe", "\\node.cmd" } or { "/node" }
+    local ahead = {}
+    for dir in string.gmatch(path, "[^" .. separator .. "]+") do
+        if is_vfox_path(dir) then
+            for _, candidate_dir in ipairs(ahead) do
+                for _, name in ipairs(names) do
+                    local candidate = (candidate_dir:gsub("[/\\]+$", "")) .. name
+                    if UTIL.file_exists(candidate) then
+                        return candidate
+                    end
+                end
+            end
+            return nil
+        end
+        table.insert(ahead, dir)
+    end
+    return nil
+end
+
 function UTIL.compare_versions(v1o, v2o)
     local v1 = v1o.version
     local v2 = v2o.version

tests/hooks_test.lua

@@ -287,5 +287,75 @@ test("legacy latest_available resolves against the index", function()
     os.remove(path)
     assert(result.version == "22.23.3", tostring(result.version))
 end)
+-- PreUse notes a `node` that vfox keeps ahead of its own paths, such as the
+-- one `pnpm runtime set node -g` writes to $PNPM_HOME/bin.
+dofile("hooks/pre_use.lua")
+local function with_path(path, existing, fn)
+    local saved_getenv, saved_exists, saved_print = os.getenv, util.file_exists, print
+    local printed = {}
+    os.getenv = function(key)
+        if key == "PATH" then
+            return path
+        end
+        return saved_getenv(key)
+    end
+    util.file_exists = function(p)
+        return existing[p] == true
+    end
+    print = function(line)
+        table.insert(printed, line)
+    end
+    local ok, err = pcall(fn)
+    os.getenv, util.file_exists, print = saved_getenv, saved_exists, saved_print
+    assert(ok, err)
+    return printed
+end
+local installed = { ["24.21.0"] = { version = "24.21.0" }, ["22.23.3"] = { version = "22.23.3" } }
+test("pre_use notes a node ahead of vfox", function()
+    RUNTIME = { osType = "linux", archType = "amd64" }
+    local printed = with_path("/home/u/.local/share/pnpm/bin:/home/u/.vfox/sdks/nodejs/bin:/usr/bin", {
+        ["/home/u/.local/share/pnpm/bin/node"] = true,
+        ["/home/u/.vfox/sdks/nodejs/bin/node"] = true,
+    }, function()
+        assert(PLUGIN:PreUse({ version = "24", installedSdks = installed }).version == "24.21.0")
+    end)
+    assert(#printed == 1, "expected one note")
+    assert(printed[1]:find("/home/u/.local/share/pnpm/bin/node", 1, true), printed[1])
+    assert(printed[1]:find("nodejs@24.21.0", 1, true), printed[1])
+    assert(printed[1]:find("docs/pnpm.md", 1, true), printed[1])
+end)
+test("pre_use stays quiet when vfox comes first", function()
+    RUNTIME = { osType = "linux", archType = "amd64" }
+    local printed = with_path("/home/u/.vfox/sdks/nodejs/bin:/home/u/.local/share/pnpm/bin", {
+        ["/home/u/.local/share/pnpm/bin/node"] = true,
+    }, function()
+        assert(PLUGIN:PreUse({ version = "22.23.3", installedSdks = installed }).version == "22.23.3")
+    end)
+    assert(#printed == 0, tostring(printed[1]))
+end)
+test("shadowing_node follows vfox's PATH split", function()
+    RUNTIME = { osType = "linux", archType = "amd64" }
+    local existing = { ["/opt/node/bin/node"] = true }
+    with_path("/opt/node/bin/:/home/u/.version-fox/temp/1/nodejs/bin", existing, function()
+        assert(util.shadowing_node() == "/opt/node/bin/node", tostring(util.shadowing_node()))
+    end)
+    -- Behind another SDK's vfox path the entry ranks below every vfox path.
+    with_path("/home/u/.vfox/sdks/java/bin:/opt/node/bin:/home/u/.vfox/sdks/nodejs/bin", existing, function()
+        assert(util.shadowing_node() == nil)
+    end)
+    -- No vfox path yet: vfox will put its paths first.
+    with_path("/opt/node/bin:/usr/bin", existing, function()
+        assert(util.shadowing_node() == nil)
+    end)
+end)
+test("shadowing_node on windows", function()
+    RUNTIME = { osType = "windows", archType = "amd64" }
+    local pnpm = "C:\\Users\\u\\AppData\\Local\\pnpm\\bin"
+    with_path(pnpm .. ";C:\\Users\\u\\.vfox\\sdks\\nodejs;C:\\Windows", {
+        [pnpm .. "\\node.exe"] = true,
+    }, function()
+        assert(util.shadowing_node() == pnpm .. "\\node.exe", tostring(util.shadowing_node()))
+    end)
+end)
 print(state.passed .. " passed, " .. state.failed .. " failed")
 assert(state.failed == 0)