feat(workspace): workspace-root component (rootDir ".") and the trackAllFiles flag - #10698
davidfirst wants to merge 58 commits into
Conversation
PR Summary by QodoAllow components to own the workspace root
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
Code Review by Qodo
1. Root validation adds avoidable test cost
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
|
Follow-up: the root component now tracks Tracking it verbatim does not converge — snapping rewrites every entry's So only the durable part of the map is versioned: Also fixed: adding a component inside the workspace root used to fail with "files already used by component", because the root had already claimed them. The root now yields to the more specific component and drops those files on its next scan. |
|
Code review by qodo was updated up to the latest commit f0ca113 |
|
Went through all 24 component issues one by one against the workspace-root component. The result was not "none of them are relevant" — testing changed the answer. I first ignored everything dependency-derived (18 issues). That made things worse: with So the list is narrowed to the three that misfire for a structural reason — the root component has no env toolchain, no compiler, and nothing imports it as a package:
Everything else is kept. The dependency-related issues never fire for a component whose files hold no imports, so ignoring them buys nothing and costs the guard when they do fire. Net effect: |
|
Code review by qodo was updated up to the latest commit d166385 |
…nd write paths the workspace-root component (rootDir ".") is a bag of the workspace's own config files. three things treated it as a regular source component: - env: it defaulted to the regular default env, giving it a compiler and a dependency policy it can never use. it now defaults to the empty env. an env set explicitly on it still wins. - install: its dir is the workspace root, so handing it to the package manager collided with the root project - pnpm resolved it to an empty "file:" spec and failed to build the lockfile, breaking "bit install" entirely. - write: importing it into another workspace wrote a .bitmap into a sub-directory, silently turning that dir into a broken nested workspace, and checking out an earlier version of it crashed on a non-BitError. the empty env removes the compiler-derived issue structurally, so the issue-ignore list added for this component is no longer needed and is reverted.
|
Follow-up on two questions raised in review: what happens when a workspace-root component is imported, and what env it should get. Env. It was defaulting to the regular default env, which hands a bag of config files a compiler and a dependency policy it can never satisfy. It now defaults to This turned out to be the better fix for the component-issues question. With no compiler, Import. Two real bugs, both reproduced:
Fixed by never writing Third bug found on the way: 17 e2e + 10 unit passing, lint clean. |
|
Code review by qodo was updated up to the latest commit 016b3c4 |
- remove/eject: rootDir "." was passed to RemovePath with recursive deletion, so removing the root component wiped the entire workspace - nested components, .bit, .bitmap and unrelated files. its files are the workspace's own, so untracking it now leaves them in place. - re-adding "bit add ." threw, since files were compared against a "./" prefix they never have. - a second component claiming the workspace root was accepted, then failed .bitmap's duplicate-rootDir validation on the next load. now rejected with a message naming the current owner. - "bit add ." skipped dotfiles and enumerated node_modules; it now uses the same ignore list as the rescan, so both agree on what the root component owns. - .bitTmp and the legacy .bit.map.json are excluded from the root file-set. - the .bitignore/.gitignore lookup resolved against the process cwd rather than the workspace. - the writer rejected a rootDir of "." whenever no .bitmap entry existed yet, which also blocked restoring a stashed root component. it now rejects only when a different component owns the root. - .bitmap normalization no longer clears "scope": unlike "version" it is stable after the first export, and clearing it collapsed components from other scopes onto the workspace default on restore.
|
Code review by qodo was updated up to the latest commit b2c0a9d |
…nto "." "bit import <root-component> --path ." crashed with an undefined path: "--path ." resolves to an empty relative path, which was stored as an empty rootDir. it is now normalized to ".", and the workspace root - which always holds .bit, .bitmap and workspace.jsonc - is no longer rejected as "not empty" for the component that owns it. this is the flow that restores a git-free workspace from its scope.
|
Code review by qodo was updated up to the latest commit 99c1fd8 |
|
Code review by qodo was updated up to the latest commit b49b410 |
…t-generated files bit drops package.json, a root-level tsconfig.json and lint configs, and the npm/yarn lockfiles from every component because it generates them. a workspace adopted from an existing monorepo owns those files, and without them a workspace restored from the scope can be neither installed nor built. with "trackAllFiles": true in teambit.workspace/workspace, only the git-ignored files and the hard exclusions (node_modules, .env, ...) are left out.
davidfirst
changed the title
|
Code review by qodo was updated up to the latest commit d2b6186 |
|
Code review by qodo was updated up to the latest commit 57708e5 |
…e remote, tolerate missing members
|
Code review by qodo was updated up to the latest commit 216f3ee |
… one import, use it in clone
…export stops modifying the root
|
Code review by qodo was updated up to the latest commit 47c9c0b |
… names, join the output with the toolkit
|
Code review by qodo was updated up to the latest commit fbbf9b4 |
…-component-nesting # Conflicts: # scopes/harmony/cli-reference/cli-reference.docs.mdx
|
Code review by qodo was updated up to the latest commit f51ca92 |
…e root, it would take the workspace with it
…pace tracks, carry trackAllFiles through init
|
Code review by qodo was updated up to the latest commit 16122c6 |
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
|
Code review by qodo was updated up to the latest commit be52daf |
… so a tracked root stops reporting them
| it('should refuse, because the root would fail to load without its main file', () => { | ||
| const cmd = () => helper.command.addComponent('packages/comp1', { i: 'comp1' }); | ||
| expect(cmd).to.throw('main file of the workspace-root component'); |
There was a problem hiding this comment.
1. Root validation adds avoidable test cost 📘 Rule violation ➹ Performance
The new adding a nested component that holds the main file E2E block runs two full addComponent() commands solely to exercise the deterministic throwForTakingWorkspaceRootMainFile() path check. Both ordinary and ignored-main-file variants stop at the same validation before any map write or link, so they add Harmony setup and command overhead for branches that can be covered beside the tracker.
Agent Prompt
## Issue description
Two new Harmony E2E cases exercise only the deterministic workspace-root main-file path validation, adding unnecessary command and workspace setup overhead.
## Fix Focus Areas
- e2e/harmony/add-harmony.e2e.ts[263-271]
- scopes/component/tracker/add-components.ts[918-929]
## Recommended Fix
Extract or export the workspace-root main-file validation helper as needed, cover its ordinary and ignored-file-equivalent path branches in a co-located tracker `.spec.ts`, and remove the two redundant E2E cases.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const consumer = this.consumer; | ||
| if (!consumer) return; | ||
| const rootMap = findWorkspaceRootMap(consumer.bitMap); | ||
| if (!rootMap) return; |
There was a problem hiding this comment.
2. Members keep stale root provenance 🐞 Bug ≡ Correctness
recordWorkspaceRoot() returns when findWorkspaceRootMap() finds no current root, leaving any existing workspace-root aspect root value untouched. When a component imported from a rooted workspace is tagged or snapped in a rootless one, its new version still identifies the old root, so provenance consumers report workspace files that were not used for that version.
Agent Prompt
## Issue description
Components retain an old workspace-root association when tagged or snapped in a workspace that has no workspace-root component because `recordWorkspaceRoot()` returns without clearing existing aspect data.
## Fix Focus Areas
- scopes/component/snapping/version-maker.ts[775-789]
- scopes/workspace/workspace-root/workspace-root-data.ts[42-61]
- scopes/workspace/workspace-root/workspace-root-data.spec.ts[48-68]
## Recommended Fix
Add a workspace-root data helper that removes the existing `root` association, or removes the aspect entry when it has no other data. Invoke it for every versioned workspace member when no current root map exists, while preserving the root marker where applicable, and add coverage for re-versioning a component from a rooted workspace in a rootless 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 3578fa4 |
| const patternsPerDir = await Promise.all( | ||
| Array.from(ignoreFileByDir, async ([fileDir, name]) => { | ||
| const absoluteDir = path.join(consumerPath, fileDir); | ||
| const patterns = name === BIT_IGNORE ? await getBitIgnoreFile(absoluteDir) : await getGitIgnoreFile(absoluteDir); |
There was a problem hiding this comment.
9. Nested ignore precedence can reverse 🐞 Bug ≡ Correctness
getNestedIgnorePatterns() preserves the unsorted relativePaths insertion order when building ignoreFileByDir, then concatenates each file's patterns in that same order. When traversal yields a descendant ignore file before its ancestor, the ancestor's later rule overrides descendant exceptions, so workspace-root scans claim a different file set than Git.
Agent Prompt
## Issue description
Nested ignore files are applied in filesystem traversal order rather than Git's ancestor-before-descendant precedence order, allowing parent rules to override child exceptions.
## Fix Focus Areas
- components/legacy/bit-map/component-map.ts[165-184]
## Recommended Fix
Sort the collected ignore-file entries by directory depth from shallowest to deepest before reading and flattening their patterns, with a deterministic secondary path ordering.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (componentRootDir === WORKSPACE_ROOT_DIR || existingComponentMap?.rootDir === WORKSPACE_ROOT_DIR) { | ||
| this.throwForNonWorkspaceRootComponent(component); | ||
| this.throwForSymlinksInTheWay(component); |
There was a problem hiding this comment.
10. Root tracking fails on symlinked paths 🐞 Bug ≡ Correctness
getWriteParamsOfOneComponent() calls throwForSymlinksInTheWay() before honoring skipWritingToFs, although persistComponentsData() performs no filesystem write for that option. A bit import <root> --path . --track-only therefore rejects any incoming path backed by a symlink even though track-only is explicitly routed through the no-write path.
Agent Prompt
## Issue description
Workspace-root imports run symlink write-safety validation even when track-only mode suppresses all filesystem persistence.
## Fix Focus Areas
- scopes/component/component-writer/component-writer.main.runtime.ts[291-295]
- scopes/component/component-writer/component-writer.main.runtime.ts[163-168]
## Recommended Fix
Keep workspace-root identity validation for bitmap tracking, but call `throwForSymlinksInTheWay()` only when `opts.skipWritingToFs` is false.
ⓘ 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 dfda122 |
2 participants
Context: the Bit side of pnpm/rfcs#33, Bit version control for pnpm workspaces: each pnpm project is a component and the unclaimed files belong to a root component. This PR lands that root-component model in bit; the adoption command and the pnpm-specific pieces follow, based on #10675.
Lets a single component own the workspace root (
rootDir: "."), and adds a workspace flag that tracks the files bit normally treats as generated. Together they make a git-free workspace restorable from its scope: the root component carries the repository-level files and.bitmap, and the flag keepspackage.jsonand friends.Workspace-root component
On the name: "root component" already means the dependency-resolver's
rootComponents(envs and apps installed as roots undernode_modules/.bit_roots), and "workspace component" is every component loaded from a workspace (WorkspaceComponent). "Workspace-root component" is whatrootDir: "."says, clashes with neither, and pairs with "nested components" for the ones inside it. Code uses theWORKSPACE_ROOT_DIRconstant and theworkspaceRootprefix.rootDir: "."is valid and is the only root-dir allowed to contain other components. Its file-set is everything under the root minus the nested components' root-dirs, re-scanned like any other component, so files added later are picked up..bit/,.git/andnode_modulesare never claimed..bitmap, with versions normalized on load so it converges after a snap. The writer never writes.bitmapback, so an imported root cannot create a phantom nested workspace.bit add .tracks it withteambit.harmony/empty-envas explicit config (so env resolution and the dependency policy agree), and it is excluded from install and link. Its files are not parsed for dependencies either: nothing installs, links or builds the root, and repo scripts may require anything, so detection would only produce blocking issues with no consumer for the result. Its main file defaults toworkspace.jsonc, the root has no entry point of its own;--mainstill overrides.bit removeandbit ejectdo not delete the workspace. Re-adding it is a no-op; a second root component is rejected at add time.teambit.workspace/workspace-rootowns the concept. The root marks itself in its aspect data ({ "isRoot": true }), and that marker, not the files it carries, is what tells a root apart, e.g. on import onto.. On snap, every member of the workspace records the root it was snapped in, at the root's version after that snap:{ "root": "scope/root@version" }. A new or modified root joins everybit tagandbit snapof its members, so the recorded version always has the files the member was made with; a root tagged along gets a patch bump of its own, whatever--verthe members got, and the command output says so. Both are data, not config, so they never make a component modified and the root moving on does not touch its members. The record tells a CI or a clone which root files (lockfile, tsconfig, scripts) a version was made with, andbit showprints it as "workspace root".bit clone <root-id> [dir]makes a workspace out of it, the waygit clonemakes a working tree out of a repository. It runs outside a workspace, in an empty or absent directory (default: the component name), needs nobit init, lands the root files at the root (workspace.jsoncincluded; nothingbit initgenerates is added), imports every component the root's versioned.bitmaplists into the directory it records, then installs and compiles (-xto skip). The versioned.bitmaphas no versions, so the components come at their heads on main;--lane <scope>/<name>clones the workspace as it is on a lane and comes out on it. A version on the root id pins the root files only. A component the root lists that its remote does not have is reported and skipped.--remote <url>registers a self-hosted scope in the new workspace first.bit import <root> --path .stays as the low-level primitive..without--overrideis accepted only in a fresh workspace (nothing else tracked), which is the restore flow; an established workspace gets the usual conflict error listing the root files that would be overwritten.trackAllFiles"trackAllFiles": trueunderteambit.workspace/workspacestops bit from droppingpackage.json, a root-leveltsconfig.jsonand lint configs, and the npm/yarn lockfiles. Only git-ignored files and the hard exclusions stay out. Meant for workspaces adopted from an existing monorepo, where those files are the source of truth. Import writes the model's files regardless, so a component with a trackedpackage.jsonshows as modified in a workspace without the flag.Tests
bit-map.spec.ts(nesting rules,getNestedRootDirs,.bitmapnormalization and the versioned-map reader),component-map.spec.ts(ignore logic with and without the flag) anddetermine-main-file.spec.ts(the root's main-file default),workspace-root-data.spec.ts(the root marker and the snapped-in root record).add-harmony.e2e.tscovers root tracking,.bitmapconvergence, a modified root joining a member's snap and tag, remove, re-add, checkout, import into another workspace and onto., env defaults, adopt → export →bit clonewith the flag, andbit clone --lane.