Skip to content

feat(shim): improve npm global install experience#708

Merged
fengmk2 merged 31 commits intomainfrom
node-path-not-found
Mar 7, 2026
Merged

feat(shim): improve npm global install experience#708
fengmk2 merged 31 commits intomainfrom
node-path-not-found

Conversation

@fengmk2
Copy link
Copy Markdown
Member

@fengmk2 fengmk2 commented Mar 6, 2026

Summary

  • Post-install hint for npm install -g: When Vite+ manages Node via shims, npm install -g <pkg> installs binaries into the managed Node's bin dir which is not on PATH. After install, the shim now checks whether installed binaries are reachable and offers to create links in ~/.vite-plus/bin/, plus a tip suggesting vp install -g instead.
  • Suppress npm output from vp install -g: Pipe npm stdout/stderr so install noise is hidden on success, only shown on failure for debugging. Add --no-fund flag.
  • Clean up CLI output: Replace raw println! with output::raw/output::warn, remove unnecessary two-space prefix from install/uninstall messages.

Test plan

  • cargo test -p vite_global_cli — unit tests pass
  • cargo clippy -p vite_global_cli — no new warnings
  • Snap tests updated for all affected test cases
  • pnpm bootstrap-cli && vp install -g cowsay — verify clean output

closes VP-217

image.png

@netlify
Copy link
Copy Markdown

netlify Bot commented Mar 6, 2026

Deploy Preview for viteplus-staging canceled.

Name Link
🔨 Latest commit 29f16b8
🔍 Latest deploy log https://app.netlify.com/projects/viteplus-staging/deploys/69ac073e4e8bd7000864bbf3

@fengmk2 fengmk2 self-assigned this Mar 6, 2026
Copy link
Copy Markdown
Member Author

fengmk2 commented Mar 6, 2026

@fengmk2 fengmk2 force-pushed the node-path-not-found branch 2 times, most recently from c1234de to 9449ff0 Compare March 6, 2026 09:34
@fengmk2 fengmk2 changed the title feat(shim): add post-install hint for npm install -g in shim feat(shim): improve npm global install experience Mar 6, 2026
@fengmk2 fengmk2 marked this pull request as ready for review March 6, 2026 11:26
@linear
Copy link
Copy Markdown

linear Bot commented Mar 6, 2026

VP-217 vp env: active Node bin not added to PATH

Problem

  • After installing/using Vite+ (vp env / managed Node), the active Node version's global npm bin directory is not added to $PATH.

User impact

  • Global CLIs installed via npm i -g (e.g. codex) keep resolving to whatever is on the user's original PATH.
  • Leads to repeated "Do you want to update?" prompts and installs into different per-Node global locations when switching Node versions.

Expected

  • When Vite+ selects a Node version, ensure that Node's global npm bin (and/or the Node-managed bin dir) is on PATH for the session so global CLIs resolve consistently.

Context

  • MK: "I checked myself, and it's true that it's not set. I'll look at the code to see why." (Discord thread)

Comment thread crates/vite_global_cli/src/shim/exec.rs Outdated
Comment thread crates/vite_global_cli/src/shim/dispatch.rs Outdated
Comment thread crates/vite_global_cli/src/commands/env/bin_config.rs Outdated
Comment thread packages/cli/snap-tests-global/npm-global-uninstall-vp-managed/snap.txt Outdated
@fengmk2 fengmk2 force-pushed the node-path-not-found branch from 1871f6d to 3ab3849 Compare March 6, 2026 15:32
fengmk2 and others added 12 commits March 7, 2026 00:01
When Vite+ manages Node via shims, `npm install -g <pkg>` installs
binaries into the managed Node's bin dir which is not on PATH. The
binaries are silently unreachable.

After `npm install -g` completes successfully, the shim now checks
whether installed binaries are reachable from PATH. If not:
- Interactive mode: prompts user to create a link in ~/.vite-plus/bin/
- Non-interactive mode: creates links automatically
- Always prints a tip suggesting `vp install -g` instead

Uses spawn+wait instead of exec for `npm install -g` specifically so
post-install checks can run. All other npm commands continue using exec
for zero overhead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace per-binary reachability check with PATH-scope check. The previous
implementation only checked if binaries existed in ~/.vite-plus/bin/, which
missed cases where users set a custom npm prefix (e.g., NPM_CONFIG_PREFIX)
whose bin dir is already on PATH.

Now uses `npm config get prefix` to determine the actual npm global prefix,
derives the bin dir from it, and checks if it's on the user's original PATH.
If yes, binaries are reachable and no shim/hint is needed. If not, creates
shims and shows the tip as before.

Also adds snap tests for custom npm prefix scenarios (both on-PATH and
off-PATH).
Use `output::info` and `output::error` from `vite_shared::output` instead
of ad-hoc `eprintln!("vp: ...")` calls in `create_bin_link` and
`check_npm_global_install_result`. This aligns with the project's logging
standards (`info:`, `warn:`, `error:` prefixes).

Also adds `output.status.success()` check to `npm config get prefix`
fallback to avoid using error output as a prefix path.
npm's @npmcli/redact matches hyphenated UUID patterns (e.g.,
`xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx`) and treats any config value
containing them as a secret, causing `npm config get prefix` to fail
with "The prefix option is protected". Removing hyphens from the UUID
avoids this pattern match.

This also removes the NPM_CONFIG_PREFIX env var workaround in
`get_npm_global_prefix`, since `npm config get prefix` now works
correctly without UUID-like segments in the path.
- Pipe npm stdout/stderr instead of inheriting, so install noise is
  hidden on success and only shown on failure for debugging
- Add --no-fund flag to suppress npm funding messages
- Replace raw println! with output::raw/output::warn for consistent
  CLI output formatting
- Remove unnecessary two-space prefix from all install/uninstall messages
- Update RFC examples to match new output format
When `npm install -g` encounters a binary that's already managed by
`vp install -g` (detected via BinConfig), warn the user instead of
silently skipping. When `npm uninstall -g` runs, skip removal of
vp-managed shims to prevent breaking managed installations.

Also adds npm global uninstall support (parse, collect bins before
uninstall, remove links after) and cleans up CLI output formatting.
- Unify `parse_npm_global_install` and `parse_npm_global_uninstall` into
  a shared `parse_npm_global_command` with parameterized subcommands
- Merge identical `NpmGlobalInstall`/`NpmGlobalUninstall` into `NpmGlobalCommand`
- Extract `read_npm_package_json` helper for the duplicated package.json
  reading with npm-prefix-then-node-dir fallback
- Extract `collect_bin_names_from_npm` to share bin collection logic
  between install and uninstall flows

Net: -48 lines with no behavior change.
- Add BinSource enum (Vp/Npm) to BinConfig with backward-compatible
  serde default, so npm-created links are distinguishable from
  vp-managed shims and user-owned binaries
- Record BinConfig with source:"npm" when create_bin_link() succeeds
- Only remove links with source:"npm" during npm uninstall -g cleanup,
  protecting user-owned binaries and vp-managed shims
- Add sync methods (load_sync/save_sync/delete_sync) to BinConfig
  using direct error-kind matching instead of exists() pre-checks
- Capture --prefix/--prefix=value from npm CLI args and use it for
  package.json lookups and bin dir resolution
- Detect Windows drive-letter paths (C:\...) as local package specs
- Add snap tests for preexisting binary protection and --prefix support
…pm uninstall

Fix two defects in npm global install/uninstall shim handling:

1. resolve_npm_prefix() now resolves relative --prefix values (e.g.,
   ./custom) against cwd instead of silently falling back to the default
   npm prefix. Uses cwd.join(prefix) which handles both relative and
   absolute paths correctly.

2. remove_npm_global_uninstall_links() now checks both source == Npm AND
   package ownership before removing a link. This prevents uninstalling
   package A from removing a bin link that was overwritten by package B.
   collect_bin_names_from_npm() returns (bin_name, package_name) pairs,
   and check_npm_global_install_result() updates BinConfig ownership
   when a force-install overwrites an existing npm link.
- Use output::error instead of eprintln! in spawn_tool() (exec.rs)
- Save node_version in BinConfig::new_npm() so npm-installed packages
  track which Node.js version was used
- Remove "Uninstalling..." log from vp remove -g (fast enough to skip)
- Update snap tests for the above changes
- Use to_absolute_path_buf() instead of to_path_buf() for AbsolutePath
- Dereference vite_str::Str before passing to std::fs::write
fengmk2 added 2 commits March 7, 2026 00:01
Add `after` step to uninstall the npm global package so subsequent runs
don't see "changed" instead of "added" from residual state.
@fengmk2 fengmk2 force-pushed the node-path-not-found branch from 7e902fa to dfd8ffd Compare March 6, 2026 16:06
When `npm install -g pkg-a pkg-b` and both packages declare the same
binary name, missing_bins gets duplicate entries. The second
create_bin_link call fails (EEXIST) so BinConfig is never saved for the
last package, leaving stale ownership. Dedup by bin_name keeping the
last entry to match npm's "last writer wins" behavior.
@fengmk2 fengmk2 force-pushed the node-path-not-found branch 2 times, most recently from 2e83449 to 449380c Compare March 6, 2026 16:46
- Extract `exit_code_from_status()` in exec.rs to return 128+signal on
  Unix when a child is killed by a signal, instead of always returning 1
- Make the `exists()` early-return in `remove_npm_global_uninstall_links`
  Unix-only, since on Windows shim files are regular files that always
  exist regardless of target validity
- Clean up stale .cmd files on Windows during repair
@fengmk2 fengmk2 force-pushed the node-path-not-found branch from 449380c to 1824df7 Compare March 6, 2026 17:04
…d snap test

Narrow CI to only run the failing snap test case and add debug
eprintln! calls at every decision point in check_npm_global_install_result
to identify why the "Skipped" message is not being output.
Comment thread crates/vite_global_cli/src/shim/dispatch.rs Outdated
fengmk2 added 5 commits March 7, 2026 09:18
The snap test only captures stdout, so eprintln debug output was
invisible. Switch all debug logs to println so they appear in the
snap test output.
Log the full dispatch context before check_npm_global_install_result:
parse result, args, exit code, home dir, npm_prefix, and node_dir.
- Remove all debug println! from dispatch.rs
- Bump NAPI cache key v3→v4 to force binary rebuild on CI
- Restore CI to run full test suite with git diff --exit-code
- Mark vp ls -g output as ignoreOutput (pnpm presence varies)
- Normalize npm tree characters (ASCII `` `-- `` → Unicode └──)
… install

- Add tracing::debug! at every decision point in dispatch and
  check_npm_global_install_result
- Set VITE_LOG=trace in CI snap test env to capture tracing output
- Run only npm-global-install-hint snap test to isolate the issue
Comment thread packages/tools/src/snap-test.ts
fengmk2 added 2 commits March 7, 2026 16:13
When pnpm runs via the vp shim, vp sets VITE_PLUS_TOOL_RECURSION=1
before exec to prevent infinite shim loops. This env var leaked into
snap test subprocesses (matched by the VITE_* passthrough pattern),
causing every npm/node command to bypass managed shim dispatch and
fall through to system binaries — hiding the "Linked" messages.

Also reverts all debugging artifacts (tracing, --verbose, VITE_LOG,
cache key bump) and removes the overly broad npm tree character
normalization that corrupted cowsay ASCII art.
The exec unit tests spawned `sh` without an absolute path, which fails
on CI macOS runners where PATH may not include /bin during cargo test.
Use `/bin/sh` on Unix and `cmd` on Windows instead.
@fengmk2 fengmk2 force-pushed the node-path-not-found branch from c2c8c84 to 0e6559d Compare March 7, 2026 08:21
Comment thread crates/vite_global_cli/src/shim/dispatch.rs Outdated
fengmk2 added 2 commits March 7, 2026 19:00
…nstall

`is_local_path()` only checked for `./`, `../`, `/`, and Windows drive paths.
Bare `.` and `..` are valid npm local path specs but were treated as package
names, so `npm install -g .` failed to create a fallback link.
- Add `output::raw_inline()` for no-newline stdout output
- Replace `print!` prompt in dispatch.rs with `output::raw_inline()`
- Replace `eprintln!` in exec_unix with `output::error()`
- Use HashSet instead of Vec for O(n) dedup in `dedup_missing_bins()`
Copy link
Copy Markdown
Member Author

fengmk2 commented Mar 7, 2026

Merge activity

  • Mar 7, 11:33 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Mar 7, 11:33 AM UTC: @fengmk2 merged this pull request with Graphite.

@fengmk2 fengmk2 merged commit 62bfff3 into main Mar 7, 2026
22 checks passed
@fengmk2 fengmk2 deleted the node-path-not-found branch March 7, 2026 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants