Repository navigation
feat(workspace): manage pnpm workspaces with Bit, on the workspace-root component - #10729
davidfirst wants to merge 60 commits into
Conversation
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… component "bit pnpm sync" now tracks each pnpm project as a component and the root with rootDir ".", under trackAllFiles, instead of explicit file lists and a topology in tracker config. Drops the --pnpm-vcs-root/--pnpm-vcs-bootstrap flags, toolchain profiles, the vcs block in pnpm-workspace.yaml, the manifest migration and the package-scripts env (CI runs the scripts after "bit clone"). The import plan no longer rewrites package manifests. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ionless packages componentIdToPackageName now honors the dependency-resolver packageName config, so the linker and the root-components linking agree on the name. A found package with no version (a private pnpm package) is recorded as "*" instead of failing the dependency detection. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ce refs on import Adds teambit.envs/pnpm-workspace-env, a plain-JS env on the empty env that compiles and builds through the packages' own scripts, in a pnpm workspace rebuilt from the workspace-root component. "bit pnpm sync" assigns it to projects with scripts (--env to override). On import, a "workspace:" reference to a sibling the target workspace lacks becomes "catalog:", bound to its exact version. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
PR Summary by QodoManage pnpm workspaces through workspace-root components
AI Description
Diagram
High-Level Assessment
Files changed (36)
|
Code Review by Qodo
1. New workspace aspect omits typed config
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
…nd clone In a pnpm-adopted workspace, the component writer and bit clone no longer write dependencies into the root package.json or run bit's install and compile, which would also run the packages' build scripts. They hint to run "pnpm install" instead. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Code review by qodo was updated up to the latest commit 920cefc |
- mark each synced project in its tracker config, so a later sync removes only its own components (nameless ones included) and can move one from the env it assigned to another --env - drop a package name the package.json no longer gives - read the default catalog by the same rule as import writes it - clear a capsule's output dirs before copying a new build's output - list only removals and a summary in the sync report, with the error symbol for removals - skip schemaVersion in the e2e clean-status check Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Code review by qodo was updated up to the latest commit 1916412 |
…s load as ESM Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Code review by qodo was updated up to the latest commit d8b6c28 |
…napped with, one install hint on clone Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Code review by qodo was updated up to the latest commit d1bbb42 |
|
Code review by qodo was updated up to the latest commit ed0bd7a |
…t of bit's bootstrap
|
Code review by qodo was updated up to the latest commit c865649 |
… guard status issue providers
|
Code review by qodo was updated up to the latest commit 04b07b3 |
|
Code review by qodo was updated up to the latest commit ca19873 |
…son as its main file
|
Code review by qodo was updated up to the latest commit 6373fcf |
…no install hint for a clone that skips it
Code Review by QodoSorry, something went wrongWe weren't able to complete the code review on our side. Please try again manually by commenting/agentic_review on this PR.
|
…e package-name map per bitmap state
|
Code review by qodo was updated up to the latest commit 24aaca1 |
…p an import whose post-write handler fails
| it('should refer to the sibling by "catalog:", with the catalog binding it to the workspace', () => { | ||
| const appManifest = helper.fs.readJsonFile('packages/app/package.json'); | ||
| expect(appManifest.dependencies).to.deep.equal({ '@acme/math': 'catalog:' }); | ||
| expect(helper.fs.readFile('pnpm-workspace.yaml')).to.have.string('"@acme/math": workspace:*'); |
There was a problem hiding this comment.
6. Catalog checks repeat unit coverage 📘 Rule violation ➹ Performance
The new should refer to the sibling by "catalog:" end-to-end test repeats the dependency rewrite and catalog-binding assertions already in pnpm-workspace-sync.spec.ts. It runs after the same setup used by the build tests, adding a separate test case without exercising another command or integration path.
Agent Prompt
## Issue description
The new end-to-end catalog assertions duplicate focused spec coverage and add a separate test case to the CI suite.
## Fix Focus Areas
- e2e/harmony/add-harmony.e2e.ts[661-665]
## Recommended Fix
Remove the separate catalog assertion test. Keep the focused spec's manifest checks and the end-to-end build tests that use the workspace.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 0c9793a |
…es links or false link/dist issues
| Object.entries(catalogs).forEach(([catalogName, catalog]) => { | ||
| importedPackageNames.forEach((packageName) => { | ||
| if (!catalog[packageName]) return; | ||
| bind(catalogName, packageName, 'workspace:*'); | ||
| workspaceBoundPackageNames.add(packageName); |
There was a problem hiding this comment.
6. Imports change unrelated catalog entries 🐞 Bug ≡ Correctness
applyPnpmImportPlan() changes every existing catalog entry whose package name matches an imported component to workspace:*, without checking which catalog entries that component uses. Importing a local package can therefore change a named catalog entry used by another project from its selected version to the local workspace package.
Agent Prompt
## Issue description
Import reconciliation changes existing catalog entries solely because their package name matches an imported component, including entries the component never references.
## Fix Focus Areas
- scopes/workspace/pnpm-workspace/pnpm-workspace-sync.ts[945-970]
## Recommended Fix
Restrict rebinding to catalog entries used by imported manifests or otherwise explicitly identified for local rebinding. Preserve unrelated named-catalog entries, and add a test with the same package name in an unrelated catalog.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| async function markWorkspaceRoot(component: Component): Promise<WorkspaceRootData> { | ||
| const consumerComponent = component.state._consumer as ConsumerComponent; | ||
| return consumerComponent.componentMap?.rootDir === WORKSPACE_ROOT_DIR ? { isRoot: true } : undefined; | ||
| return consumerComponent.componentMap?.rootDir === WORKSPACE_ROOT_DIR ? { isRoot: true } : {}; |
There was a problem hiding this comment.
7. Former roots can retain their root marker 🐞 Bug ≡ Correctness
markWorkspaceRoot() returns {} for a non-root component, but upsertExtensionData() merges that
empty object into any existing extension data instead of clearing isRoot. When a previously
snapped root is loaded as a member, code reading its consumer extensions can still classify it as a
workspace root.
Agent Prompt
## Issue description
Returning an empty object does not clear a previously stored workspace-root marker because the component loader merges extension data.
## Fix Focus Areas
- scopes/workspace/workspace-root/workspace-root.main.runtime.ts[134-136]
- scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[1248-1260]
- scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[1269-1278]
## Recommended Fix
Explicitly replace or clear the existing workspace-root extension data when the component is not the current root. Test loading a formerly snapped root after tracking it as a member.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| .sort(([dirA], [dirB]) => dirA.localeCompare(dirB)) | ||
| .forEach(([rootDir, component]) => { | ||
| const files = [...component.filesystem.files].sort((a, b) => a.relative.localeCompare(b.relative)); | ||
| files.forEach((file) => hash.update(`${rootDir}/${file.relative}\0`).update(file.contents)); |
There was a problem hiding this comment.
8. Distinct workspace trees reuse stale builds 🐞 Bug ≡ Correctness
treeSignature() hashes each path and its raw contents without encoding where one file ends and the next begins. If one file's contents include the serialized path prefix of a following file, a different tree can produce the same signature; prepareTree() then skips rewriting the tree and reinstalling dependencies.
Agent Prompt
## Issue description
The tree signature's byte stream is ambiguous across file boundaries, allowing different sets of files to have the same cache key without a cryptographic collision.
## Fix Focus Areas
- scopes/workspace/pnpm-workspace/pnpm-workspace-tree.ts[228-237]
## Recommended Fix
Include an unambiguous content length or end-of-file framing for each path and contents pair before hashing. Add a regression test using one file whose contents contain another file's serialized path prefix.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| private async filterForeignWorkspaceRoots() { | ||
| const bitMapIds = this.consumer.bitmapIdsFromCurrentLaneIncludeRemoved; | ||
| const isTracked = (id: ComponentID) => Boolean(bitMapIds.searchWithoutVersion(id)); |
There was a problem hiding this comment.
9. Switching back to a lane drops the workspace's own root 🐞 Bug ≡ Correctness
filterForeignWorkspaceRoots decides whether a root is tracked using consumer.bitmapIdsFromCurrentLaneIncludeRemoved, which only holds .bitmap entries available on the lane you are switching from. If the workspace's own root component was first snapped on a lane, it is lane-only and unavailable on main, so bit switch <lane> from main treats it as another workspace's root, warns "use bit clone", and removes it from switchProps.ids/laneBitIds. saveLanesData then calls bitMap.syncWithIds with that filtered list, so the root stays unavailable and not marked onLanesOnly on its own lane.
Agent Prompt
## Issue description
`filterForeignWorkspaceRoots` decides whether a root is tracked using `consumer.bitmapIdsFromCurrentLaneIncludeRemoved`. That list leaves out `.bitmap` entries that are not available on the current lane. So the workspace's own lane-only root gets treated as a foreign root and is excluded when switching to its lane.
## Fix Focus Areas
- scopes/lanes/lanes/switch-lanes.ts[176-178]
## Recommended Fix
Base `isTracked` on every `.bitmap` entry, whatever lane it belongs to. For example: `const isTracked = (id) => Boolean(this.consumer.bitMap.getBitmapEntryIfExist(id, { ignoreVersion: true }))`, or use `this.consumer.bitMap.getAllBitIdsFromAllLanes().searchWithoutVersion(id)`. This way any root this workspace already tracks is never filtered out. Add a unit or e2e case: snap the root on a lane, switch to main, then switch back, and check that the root is still available.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 8c6255f |
Builds on #10675, ported onto the workspace-root component model (#10698). What changed from the original, and why:
bit init --standalone --external-package-manager --default-scope <scope>, thenbit pnpm syncbit pnpm init [--default-scope <scope>]bit import --pnpm-vcs-rootandbit switch --pnpm-vcs-bootstrapbit clone <root> [--lane]useExplicitFiles) for the root and projectsrootDir: "."plustrackAllFilespnpmVcs.workspace).bitmap, read throughWorkspaceRootMain.listMembers()teambit.workspace/pnpm-workspaceaspect: it ownsbit pnpm syncand the import handling (through an importer slot), and is itself the scripts env, so the env ships with bit and needs no installAdopting a workspace:
bit pnpm init. It creates a standalone workspace and runs the first sync, so no flags are needed. It does not changepackage.json, and it writes no AI agent or MCP files, because every file at the root goes into the root component. Re-runbit pnpm syncwhen projects are added, moved or deleted.pnpm owns installation (pnpm/rfcs#33).
bit pnpm initandbit pnpm syncadd the"teambit.workspace/pnpm-workspace": {}entry toworkspace.jsonc.Workspace.isPnpmWorkspace()reads only this entry. Apnpm-workspace.yamlfile alone does not change how bit installs.workspace.jsoncis a file of the root component, so a clone or an import of the root brings the entry along. In such a workspace:bit installstops and gives the pnpm command:pnpm install, orpnpm add <pkg> --filter <project>.run "pnpm install".package.json.bit statusshows what a sync would change. It lists untracked projects, projects that left, moved projects (with thebit movecommand to run), andworkspace:references to move to the catalog. These show under "workspace issues". They do not block a snap. They come from a new status slot,registerWorkspaceIssues.Dropped on purpose
engines.node/pnpm.vcs.profilechecks on import): the current revision of RFC: Bit version control for pnpm workspaces pnpm/rfcs#33 removed them. The RFC keepsenginesas package metadata for the tools that read it.package.jsonat the root and in the project directories is supported.vcs:block inpnpm-workspace.yaml: the root component's.bitmapnow holds that identity.Kept:
workspace:tocatalog:at sync. The sync changes aworkspace:reference between projects tocatalog:, and writes the specifier to the default catalog (comments ofpnpm-workspace.yamlstay). It refuses two different specifiers for one package, and a catalog entry that binds it otherwise. The snappedpackage.jsonthen sayscatalog:, so an import into another workspace changes only that workspace's catalog, and the component is not modified. The import-side rewrite stays for components snapped before this change.A
workspace:catalog value needs pnpm 11.28.0 / 12.7.0 for the install and thepnpm -rorder (pnpm/pnpm#15591), so the CircleCI e2e and bundle jobs install 12.7.0. The pnpm tasks run the pnpm the user's shell finds: bit's installs put the directory of its own node first on PATH, and a pnpm there hid the user's.Sync and import keep
pnpm-workspace.yamlconsistent:.bitmap. If the same package is now in an untracked directory, the sync stops and asks forbit move <old-dir> <new-dir>, so the component keeps its id and history.A component that carries its own package.json (every synced project, where it is the main file):
trackAllFilesit keeps the file as source, so it imports into a regular bit workspace too.package.jsontakemain,exports,bin,scriptsand the rest from it. Bit still writes the name, the version,componentIdand the dependencies, at their exact versions.