-
Notifications
You must be signed in to change notification settings - Fork 946
fix(deps): prevent env tooling deps from polluting package.json in external PM mode #10146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…ternal PM mode When using `externalPackageManager: true`, `bit create` was adding ~40 env tooling dependencies (eslint, vitest, webpack, etc.) to the user's package.json. These deps are needed by Bit-managed workspaces but not when users manage their own package manager. Changes: - Skip adding env self peers in `_getDefaultPeerDependencies()` for external PM - Skip collecting env self peers in `getEnvsSelfPeersPolicy()` for external PM - Skip adding generator envs to workspace policy in `writeDependenciesToPackageJson()` - Respect existing deps/devDeps/peerDeps when writing to package.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes an issue where using external package manager mode (externalPackageManager: true) was causing Bit to add ~40 environment tooling dependencies (eslint, vitest, webpack, @types/*, etc.) to the user's package.json. These dependencies are needed for Bit-managed workspaces but should not be added when users manage their own package manager.
Key Changes:
- Skip adding environment tooling dependencies in external package manager mode
- Skip adding generator environment packages to workspace policy when writing to package.json
- Protect existing user-defined dependencies from being overridden
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| scopes/workspace/workspace/workspace.ts | Adds logic to skip dependencies that already exist in the user's package.json (dependencies, devDependencies, or peerDependencies) to prevent overriding user's versions |
| scopes/workspace/install/install.main.runtime.ts | Conditionally skips adding configured generator envs to workspace policy when external package manager is enabled |
| scopes/dependencies/dependency-resolver/manifest/workspace-manifest-factory.ts | Adds early returns in two methods (getEnvsSelfPeersPolicy and _getDefaultPeerDependencies) to skip collecting and adding env tooling dependencies when external package manager mode is enabled |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // When using external package manager, don't add the env package itself. | ||
| // Users don't need the env installed - they manage their own tooling. | ||
| if (!this.dependencyResolver.config.externalPackageManager) { | ||
| await this.addConfiguredGeneratorEnvsToWorkspacePolicy(mergedRootPolicy); | ||
| } |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new external package manager logic lacks test coverage. The changes introduce critical conditional behavior that skips adding generator envs to workspace policy when external package manager mode is enabled. This should be covered by tests to ensure it works correctly and doesn't regress in the future.
| // When using external package manager, skip collecting env self peers. | ||
| // Users manage their own tooling dependencies. | ||
| if (this.dependencyResolver.config.externalPackageManager) { | ||
| return undefined; | ||
| } |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new conditional logic for external package manager mode that skips collecting env self peers lacks test coverage. This is a critical change that affects which dependencies get included in the workspace manifest, and should have tests to verify correct behavior in both external and internal package manager modes.
| // When using external package manager, don't add env's tooling dependencies to components. | ||
| // The user manages their own tooling (eslint, vitest, webpack, etc.) and we should only | ||
| // include the component's actual source code dependencies (detected from imports). | ||
| if (this.dependencyResolver.config.externalPackageManager) { | ||
| return {}; | ||
| } |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new conditional logic for external package manager mode that skips adding env's tooling dependencies to components lacks test coverage. This is a significant behavioral change that should be tested to ensure components don't receive unwanted tooling dependencies in external PM mode while still getting them in internal mode.
Summary
When using
externalPackageManager: true, runningbit createwas adding ~40 env tooling dependencies (eslint, vitest, webpack, @types/*, etc.) to the user's package.json. These dependencies are needed for Bit-managed workspaces but should not be added when users manage their own package manager.Changes:
Before:
bit create react buttonadded 40+ dependencies to package.jsonAfter: Only adds deps actually used by the component's source code