Conversation
…ault (#11587) `pnpm add -g foo bar` now installs `foo` and `bar` as separate isolated globals — removing one no longer wipes out the other. Packages can still be bundled into a single isolated install with a comma-separated list: `pnpm add -g foo,bar qar` keeps foo+bar together and qar separate.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
📝 WalkthroughWalkthroughThis PR changes global install behavior: CLI params are split into install groups (space-separated selectors become separate isolated global installs; comma-separated selectors bundle packages into a single install). Implemented in ChangesGlobal Install Grouping Behavior
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (pnpm add -g)
participant GroupBuilder as splitCommaSeparated / resolveLocalParam
participant GroupInstaller as installGroup
participant Resolver as installGlobalPackages
participant GlobalBin as Global bin/linking steps
CLI->>GroupBuilder: split param into groups (safe comma-splitting)
GroupBuilder->>GroupInstaller: create fresh install dir for group
GroupInstaller->>Resolver: install group (omitSummaryLog:true)
Resolver-->>GroupInstaller: resolved aliases & package info
GroupInstaller->>GlobalBin: approve builds, remove prior installs, create cache symlink, link bins
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR changes the default behavior of pnpm add -g to install each space-separated package into its own isolated global install directory (so removing one global package doesn’t remove others installed in the same invocation). It also introduces an opt-in grouping syntax using comma-separated selectors to intentionally bundle multiple globals into one isolated install group.
Changes:
- Update global add implementation to install one “group” per CLI arg by default, with comma-separated selectors forming a single group.
- Update/add e2e tests to cover per-arg isolation and comma-based bundling, and adjust the existing global-remove group test to use the new comma syntax.
- Add a changeset documenting the new default behavior and the comma grouping mechanism.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
global/commands/src/globalAdd.ts |
Implements per-arg global install grouping, plus comma-based bundling within a single arg. |
pnpm/test/install/global.ts |
Updates and adds tests to validate per-arg isolation and comma-bundled grouping semantics. |
.changeset/fix-11587-global-isolated-per-arg.md |
Documents the behavior change and the new comma grouping syntax. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/fix-11587-global-isolated-per-arg.md:
- Line 9: The example sentence is missing the noun after the issue link; update
the second bullet that contains "`pnpm add -g foo,bar qar`" so it reads
something like "`pnpm add -g foo,bar qar` bundles `foo` and `bar` into a single
isolated install while `qar` lives in its own isolated install (see `#11587`)`" —
ensure the phrase "isolated install" (or equivalent noun) is present and keep
the issue link as a reference.
In `@global/commands/src/globalAdd.ts`:
- Around line 65-68: The loop calling installGroup({ opts, globalDir,
globalBinDir, allowBuilds, params: group }, commands) must be made atomic: first
install and validate every group into isolated temporary dirs (or capture their
planned changes) without touching global state, and only if all installs succeed
perform the destructive steps (call removeExistingGlobalInstalls, create
symlinks, and link bins) to update globalDir/globalBinDir; update or add a new
staging function (e.g., installGroupStaged or a staging phase around
installGroup) to return success artifacts/paths for the commit phase, then run
the commit phase that performs the existing global mutations only once all
groups validated.
- Around line 163-165: The splitCommaSeparated function currently splits every
comma and breaks path/URL/package selectors that legitimately contain commas;
update it to detect path/protocol selectors and skip splitting in those cases:
if the param starts with './', '../', '/', 'file:' or contains '://', or
otherwise looks like a filesystem/path or URL (e.g., contains a slash and a
comma), return [param] unchanged; only perform the comma split/trim/filter for
true grouped package argument strings. Reference the splitCommaSeparated helper
and where resolveLocalParam consumes its output to ensure selector strings that
look like local paths or URLs are not split.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f1d6a9be-2603-4a01-bd8a-79e5dd9ccc05
📒 Files selected for processing (3)
.changeset/fix-11587-global-isolated-per-arg.mdglobal/commands/src/globalAdd.tspnpm/test/install/global.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: ubuntu-latest / Node.js 24 / Test
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,tsx,jsx}: Use trailing commas in code following Standard Style with modifications
Prefer functions over classes
Declare functions after they are used, relying on hoisting
Functions should have no more than two or three arguments; use a single options object for functions needing more parameters
Organize imports in order: standard libraries, external dependencies (sorted alphabetically), then relative imports
Files:
global/commands/src/globalAdd.tspnpm/test/install/global.ts
`splitCommaSeparated` now detects path-like params (`./`, `/`, `~`, `file:`, `link:`, Windows drive paths) and URLs (anything containing `://`), and skips splitting when the param as a whole resolves to an existing local path. Plain package specs like `foo,bar` are still split as before. Adds an e2e regression test using a local package whose directory contains commas. Also reword the changeset bullet so the example sentence doesn't end abruptly at the issue link.
`pnpm add -g foo bar` runs each space-separated arg as its own isolated install, but the default-reporter's summary pipeline takes the first `summary` log event and unsubscribes — so only the first group's "global: + X" block was printed and later groups disappeared from the summary even though they had been installed correctly. Adds an `omitSummaryLog` install option that suppresses the per-install summary log inside `mutateModules`. `handleGlobalAdd` enables it for each group and emits a single consolidated summary log at the very end, so the reporter prints one "global:" block listing every package that was added across all groups.
…iple groups When `pnpm add -g` is given more than one CLI param (and so installs several isolated groups), force the reporter to use its prefixed progress/stats output. Without that, the single-prefix stats pipeline limits emissions to one install via `take(2)`, so only the first group's "Packages: +N" line is printed and later groups' stats are silently dropped. Each group now shows its own progress and stats line labelled with the install dir, and the consolidated "global:" summary still prints once at the end. Single-package `pnpm add -g foo` output is unchanged.
The new omitSummaryLog install option is consumed by global.commands, so deps-installer needs a version bump alongside it.
Summary
Fixes #11587.
In v11,
pnpm add -g foo barbundledfooandbarinto a single isolated install group, sopnpm remove -g fooalso removedbar. This change reverts the default to per-package isolation while preserving a way to opt into grouped installs:pnpm add -g foo bar—fooandbarare installed as two independent globals; removing one does not affect the other.pnpm add -g foo,bar qar—fooandbarshare a single isolated install dir (and are removed together);qaris its own.Test plan
global remove deletes install group and bin shimstest to use the comma syntax (since that is the new way to express bundling).pnpm/test/install/global.tssuite passes (20 passed, 2 pre-existing skips).Written by an agent (Claude Code, claude-opus-4-7).
Summary by CodeRabbit
Bug Fixes
Tests
Logging / Reporting