Skip to content

docs: add RFC for add/remove package commands with workspace support#220

Closed
fengmk2 wants to merge 2 commits intomainfrom
add-package-rfc
Closed

docs: add RFC for add/remove package commands with workspace support#220
fengmk2 wants to merge 2 commits intomainfrom
add-package-rfc

Conversation

@fengmk2
Copy link
Copy Markdown
Member

@fengmk2 fengmk2 commented Oct 11, 2025

Add comprehensive RFC for vite add and vite remove commands that provide
a unified interface for package management across pnpm/yarn/npm with full
workspace support following pnpm's API design.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

Copy link
Copy Markdown
Member Author

fengmk2 commented Oct 11, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

fengmk2 and others added 2 commits October 20, 2025 10:54
Add comprehensive RFC for `vite add` and `vite remove` commands that provide
a unified interface for package management across pnpm/yarn/npm with full
workspace support following pnpm's API design.

Key features:
- Auto-detection of package manager (pnpm/yarn/npm)
- Multiple package support in single command
- Workspace filtering with pnpm-inspired syntax (--filter)
- Workspace root operations (-w flag)
- Workspace protocol support (--workspace for pnpm)
- Common flag translation (-D, -E, -P, -O, -g)
- Command aliases (rm, un, uninstall)
- No caching overhead (direct execution)

Filter patterns (pnpm-focused):
- Exact match: --filter app
- Wildcards: --filter "app*"
- Scope: --filter "@myorg/*"
- Exclusion: --filter "!test*"
- Dependencies: --filter "app..."
- Graceful degradation for yarn/npm

Examples:
  vite add react --filter app
  vite add @myorg/utils --workspace --filter web
  vite add typescript -D -w
  vite remove lodash --filter "app*"

Implementation follows existing install command pattern with new
add.rs and remove.rs modules. Package manager adapter provides
command translation with proper argument ordering for each PM.

RFC includes:
- Complete command mapping for all package managers
- Workspace filter patterns and compatibility matrix
- Implementation architecture with code samples
- Test strategy with 12 test cases
- Real-world monorepo workflow examples
- Phased implementation plan (MVP → Workspace → Advanced)
- 1,373 lines of comprehensive design documentation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@fengmk2 fengmk2 marked this pull request as ready for review October 20, 2025 13:24
Copilot AI review requested due to automatic review settings October 20, 2025 13:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds an RFC proposing new vite add and vite remove commands that provide a unified package management interface across pnpm/yarn/npm with workspace-aware behavior modeled after pnpm’s API.

  • Introduces command syntax, flag mapping, and workspace filter semantics
  • Outlines implementation architecture with Rust code sketches (CLI, adapters)
  • Documents testing strategy, UX, and future enhancements

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +308 to +313
if !filters.is_empty() {
// yarn workspace <name> add
for filter in filters {
args.push("workspace".to_string());
args.push(filter.clone());
}
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

For yarn, multiple workspace targets cannot be batched by repeating workspace <name> in a single command. yarn workspace <name> add ... must be run once per workspace, or the implementation should restrict yarn to a single --filter (e.g., use only filters.first() or error when filters.len() > 1). The current approach would yield an invalid command like yarn workspace app workspace web add react.

Suggested change
if !filters.is_empty() {
// yarn workspace <name> add
for filter in filters {
args.push("workspace".to_string());
args.push(filter.clone());
}
if let Some(filter) = filters.first() {
// yarn only supports a single workspace per command
args.push("workspace".to_string());
args.push(filter.clone());

Copilot uses AI. Check for mistakes.
Comment on lines +365 to +374
PackageManagerType::Yarn => {
if !filters.is_empty() {
for filter in filters {
args.push("workspace".to_string());
args.push(filter.clone());
}
}
args.push("remove".to_string());
args.extend_from_slice(packages);
args.extend_from_slice(extra_args);
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Same issue as add: yarn cannot repeat workspace <name> to target multiple workspaces in one command. Either restrict yarn to a single filter (use only the first filter) or execute yarn workspace <name> remove ... once per targeted workspace.

Copilot uses AI. Check for mistakes.
Comment on lines +209 to +229
#[command(disable_help_flag = true)]
Add {
/// Packages to add
packages: Vec<String>,

/// Filter packages in monorepo (can be used multiple times)
#[arg(long, value_name = "PATTERN")]
filter: Vec<String>,

/// Add to workspace root (ignore-workspace-root-check)
#[arg(short = 'w', long)]
workspace_root: bool,

/// Only add if package exists in workspace
#[arg(long)]
workspace: bool,

/// Arguments to pass to package manager
#[arg(allow_hyphen_values = true, trailing_var_arg = true)]
args: Vec<String>,
},
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

With this clap definition, examples like vite add -D typescript will fail because -D (and similar flags) aren’t defined options and occur before the positional packages. trailing_var_arg only helps once positional parsing begins. To match the documented UX, define the common flags (e.g., -D/-E/-P/-O/-g) as explicit clap options and then forward them to the package manager (or require flags to follow -- or come after packages, and update examples accordingly).

Copilot uses AI. Check for mistakes.
},

/// Remove packages from dependencies
#[command(disable_help_flag = true, alias = "rm", alias = "un", alias = "uninstall")]
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

[nitpick] Prefer using aliases = [\"rm\", \"un\", \"uninstall\"] in clap rather than repeating alias = ... for readability and to avoid attribute parsing pitfalls.

Suggested change
#[command(disable_help_flag = true, alias = "rm", alias = "un", alias = "uninstall")]
#[command(disable_help_flag = true, aliases = ["rm", "un", "uninstall"])]

Copilot uses AI. Check for mistakes.
// ... existing commands

/// Add packages to dependencies
#[command(disable_help_flag = true)]
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

The CLI help sections advertise -h, --help for both commands (e.g., lines 939–940 and 975), but disable_help_flag = true removes the default help flags. Either drop disable_help_flag or add an explicit help flag to align the implementation sketch with the documented help output.

Copilot uses AI. Check for mistakes.
Comment on lines +147 to +153
| `-g, --global` | `-g` | `global add` | `--global` / `-g` | Install globally |
| `--save-catalog` | `--save-catalog` | `--save-catalog` | N/A | Save the new dependency to the default catalog |
| `--save-catalog-name <catalog_name>` | SAME | N/A | N/A | Save the new dependency to the specified catalog |
| `--allow-build <names>` | SAME | N/A | N/A | A list of package names allowed to run postinstall |

**Note**: For pnpm, `--filter` must come before the command (e.g., `pnpm --filter app add react`). For yarn/npm, it's integrated into the command structure.

Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Two accuracy issues: (1) yarn global add/remove is only available in Yarn Classic; Yarn Berry (2.x/3.x/4.x) removes global. Please clarify the version constraints or note that global operations are unsupported on modern Yarn. (2) --save-catalog is a pnpm-specific flag; Yarn doesn’t implement it. The mapping should mark Yarn as N/A (or document a Yarn-specific alternative if applicable).

Suggested change
| `-g, --global` | `-g` | `global add` | `--global` / `-g` | Install globally |
| `--save-catalog` | `--save-catalog` | `--save-catalog` | N/A | Save the new dependency to the default catalog |
| `--save-catalog-name <catalog_name>` | SAME | N/A | N/A | Save the new dependency to the specified catalog |
| `--allow-build <names>` | SAME | N/A | N/A | A list of package names allowed to run postinstall |
**Note**: For pnpm, `--filter` must come before the command (e.g., `pnpm --filter app add react`). For yarn/npm, it's integrated into the command structure.
| `-g, --global` | `-g` | `global add`<sup>†</sup> | `--global` / `-g` | Install globally |
| `--save-catalog` | `--save-catalog` | N/A | N/A | Save the new dependency to the default catalog (pnpm only) |
| `--save-catalog-name <catalog_name>` | SAME | N/A | N/A | Save the new dependency to the specified catalog (pnpm only) |
| `--allow-build <names>` | SAME | N/A | N/A | A list of package names allowed to run postinstall |
**Note**: For pnpm, `--filter` must come before the command (e.g., `pnpm --filter app add react`). For yarn/npm, it's integrated into the command structure.
<sup>†</sup> `yarn global add/remove` is only available in Yarn Classic (v1). Yarn Berry (v2+) does not support global add/remove commands; global operations are unsupported in modern Yarn.

Copilot uses AI. Check for mistakes.
},

/// Remove packages from dependencies
#[command(disable_help_flag = true, alias = "rm", alias = "un", alias = "uninstall")]
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

[nitpick] The help text lists Aliases: rm, un, uninstall (line 966) which matches the intent, but if disable_help_flag = true is retained to provide custom help, ensure these aliases appear in the actual generated help output (or remove disable_help_flag so clap includes them automatically).

Suggested change
#[command(disable_help_flag = true, alias = "rm", alias = "un", alias = "uninstall")]
#[command(alias = "rm", alias = "un", alias = "uninstall")]

Copilot uses AI. Check for mistakes.

### 2. Interactive Mode

> Referer to ni's interactive mode https://github.com/antfu-collective/ni
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'Referer' to 'Refer'.

Suggested change
> Referer to ni's interactive mode https://github.com/antfu-collective/ni
> Refer to ni's interactive mode https://github.com/antfu-collective/ni

Copilot uses AI. Check for mistakes.
-w, --workspace-root Add to workspace root (ignore-workspace-root-check)
--workspace Only add if package exists in workspace
-D, --save-dev Add as dev dependency
-P, --save-peer Add as peer dependency
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

[nitpick] This description can mislead: --save-peer typically writes to peerDependencies and also to devDependencies (pnpm, npm, and Yarn Classic). Consider clarifying with 'Add as peer dependency (also saved to devDependencies by most package managers)'.

Suggested change
-P, --save-peer Add as peer dependency
-P, --save-peer Add as peer dependency (also saved to devDependencies by most package managers)

Copilot uses AI. Check for mistakes.
@fengmk2 fengmk2 closed this Oct 21, 2025
branchseer pushed a commit that referenced this pull request Mar 11, 2026
Includes:
- feat: tree-view task selector with grouped packages (#219)
- fix: align package headers with top-level items in task selector (#220)
- chore: Adjust `vp run` output styling (#213)
- Improve shell quote handling with proper nested quote support (#217)

Update snap tests for `[vp run]` → `vp run:` output styling change.

https://claude.ai/code/session_0139LsSbe67hcD8NKKaK6Y8U
@branchseer branchseer mentioned this pull request Mar 11, 2026
2 tasks
branchseer pushed a commit that referenced this pull request Mar 11, 2026
Includes:
- feat: tree-view task selector with grouped packages (#219)
- fix: align package headers with top-level items in task selector (#220)
- chore: Adjust `vp run` output styling (#213)
- Improve shell quote handling with proper nested quote support (#217)

Update snap tests for `[vp run]` → `vp run:` output styling change.

https://claude.ai/code/session_0139LsSbe67hcD8NKKaK6Y8U
branchseer pushed a commit that referenced this pull request Mar 11, 2026
Includes:
- feat: tree-view task selector with grouped packages (#219)
- fix: align package headers with top-level items in task selector (#220)
- chore: Adjust `vp run` output styling (#213)
- Improve shell quote handling with proper nested quote support (#217)

Update snap tests for `[vp run]` → `vp run:` output styling change.
Add cross-platform normalization for `cat:` error message spacing.

https://claude.ai/code/session_0139LsSbe67hcD8NKKaK6Y8U
branchseer pushed a commit that referenced this pull request Mar 11, 2026
Includes:
- feat: tree-view task selector with grouped packages (#219)
- fix: align package headers with top-level items in task selector (#220)
- chore: Adjust `vp run` output styling (#213)
- Improve shell quote handling with proper nested quote support (#217)

Update snap tests for `[vp run]` → `vp run:` output styling change.
Add cross-platform normalization for `cat:` error message spacing.

https://claude.ai/code/session_0139LsSbe67hcD8NKKaK6Y8U
branchseer pushed a commit that referenced this pull request Mar 11, 2026
Includes:
- feat: tree-view task selector with grouped packages (#219)
- fix: align package headers with top-level items in task selector (#220)
- chore: Adjust `vp run` output styling (#213)
- Improve shell quote handling with proper nested quote support (#217)

Update snap tests for `[vp run]` → `vp run:` output styling change.
Add cross-platform normalization for `cat:` error message spacing.

https://claude.ai/code/session_0139LsSbe67hcD8NKKaK6Y8U
branchseer added a commit that referenced this pull request Mar 11, 2026
## Summary
- Bumps vite-task dependency from 4bbcba17 to 6fdc4f10

Includes:
- feat: tree-view task selector with grouped packages (#219)
- fix: align package headers with top-level items in task selector (#220)
- chore: Adjust vp run output styling (#213)
- Improve shell quote handling with proper nested quote support (#217)

Updates snap tests for output styling change.

## Test plan
- [x] cargo check --all-targets --all-features passes locally
- [ ] CI passes on all platforms
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.

2 participants