feat(workspace): workspace-root component (rootDir ".") and the trackAllFiles flag - #10698
davidfirst wants to merge 110 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. Nested-ignore sorting fails formatting
|
![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 6ff664e |
…with no root Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Code review by qodo was updated up to the latest commit eb1e095 |
…inherited the data is supplied fresh by the loader on each load and replaced wholesale by writeWorkspaceRoot, so neither clear had anything to undo. verified by stubbing both to no-ops: the add-harmony e2e file passed all 64 cases unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Code review by qodo was updated up to the latest commit 8b59694 |
"bit add ." is one keystroke from "git add ." and meant something else until now, so the intent is spelled out. a workspace that already has a root does not ask again - the existing "already tracked by" message is the useful one there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Code review by qodo was updated up to the latest commit 4f2e1c3 |
…view the symlink guard for a root write now accounts for the config file the writer enables by itself when one is already at the rootDir; an explicit main file is checked against the scan exclusions, not only the ignore rules, so it cannot be tracked and then dropped by the next rescan; a merge snap brings a new or modified root along as tag and snap do; and the watcher keeps ignoring the never-tracked files when trackAllFiles is on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Code review by qodo was updated up to the latest commit 113ad10 |
…ut the flag naming "." as the rootDir is already the intent, as a resolved track-data entry declaring it is. the flag is for "bit add", where the path can be typed out of git habit - asking a caller of track() for it answered with a command it is not running. also covers the clone report for members their remote does not have, and pins that a component tracked at the root skips the dir-conflict check like any other tracked directory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Code review by qodo was updated up to the latest commit 49aefcb |
the tag-along is for the members a merge snaps. a batch of hidden lane entries has no workspace state to snap the root against and records no root, so there is nothing for a root version to make right. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Code review by qodo was updated up to the latest commit 9e2becd |
…t name the versioned .bitmap comes from a remote, so a member is no longer written into a directory bit or git keeps for itself (.bit holds the objects the clone reads from), nor into one another member already owns - the writer undoes its own relocation, so the later component would land on the earlier one's files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
|
it patched five filenames of a general case: a workspace-root component owns the whole tree, so every file at the root it does not track is reported as inside a component that ignores it - git-ignored output included, which the watcher never consulted. naming a handful of them fixes nothing and hides component.json, which people edit by hand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| async function markWorkspaceRoot(component: Component): Promise<WorkspaceRootData | undefined> { | ||
| const consumerComponent = component.state._consumer as ConsumerComponent; | ||
| return consumerComponent.componentMap?.rootDir === WORKSPACE_ROOT_DIR ? { isRoot: true } : undefined; |
There was a problem hiding this comment.
1. New versions retain obsolete root roles 🐞 Bug ≡ Correctness
markWorkspaceRoot() returns undefined for every non-root and the component loader ignores falsy data while shallow-merging truthy data, so the aspect entry loaded from the prior version is never replaced. When a former root is moved under a directory, or a member becomes the root or is snapped in a rootless workspace, the next version can retain isRoot or root, causing bit show and clone eligibility to describe the obsolete workspace relationship.
Agent Prompt
## Issue description
Workspace-root aspect data from the prior component version is currently retained because the component-load callback returns no data for non-roots and the loader merges returned objects. Role changes and snapshots in rootless workspaces must remove obsolete `isRoot` and `root` fields before the new version is created.
## Fix Focus Areas
- scopes/workspace/workspace-root/workspace-root.main.runtime.ts[104-106]
- scopes/component/snapping/version-maker.ts[785-800]
- scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[1198-1215]
## Recommended Fix
Explicitly replace or clear workspace-root aspect data when the current bitmap role differs from the loaded version: remove `root` when a component becomes the root, remove `isRoot` when it ceases to be the root, and clear the prior `root` pointer from components being versioned when the workspace has no current root. Avoid relying on the loader's shallow merge or an `undefined` callback result to remove fields.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if ((inFileSet && excludedFromScan) || excludedByIgnoreRules) { | ||
| throw new ExcludedMainFile(relativeToComponent(mainNormalized)); |
There was a problem hiding this comment.
2. Added components lose their main file 🐞 Bug ≡ Correctness
addOneComponent() only rejects a scan-excluded main when it was already present in filteredMatchedFiles, although _addMainFileToFiles() manually adds an existing excluded main after the scan. Selecting a nested .bitmap, .git file, or another scan-excluded path as --main therefore succeeds at add time and the next rescan removes that main file from the component.
Agent Prompt
## Issue description
The new main-file validation lets an explicit main that was excluded from the directory scan be appended to the component's initial file list. A later rescan consistently removes that file, leaving the component without its declared main file.
## Fix Focus Areas
- scopes/component/tracker/add-components.ts[650-672]
- components/legacy/bit-map/component-map.ts[94-99]
## Recommended Fix
Reject an explicit main whenever it is excluded by `filterByScanIgnorePatterns()`, regardless of whether it was in the filtered scan result. Keep the existing ignore-rule validation, but do not gate the scan-exclusion result on `inFileSet`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| export function isOwnedByNestedComponent( | ||
| relativePath: PathLinuxRelative, | ||
| nestedRootDirs: PathLinuxRelative[] | ||
| ): boolean { | ||
| return nestedRootDirs.some((nestedRootDir) => relativePath.startsWith(`${nestedRootDir}/`)); | ||
| } |
There was a problem hiding this comment.
3. Root updates overwrite nested source files 🐞 Bug ≡ Correctness
isOwnedByNestedComponent() compares incoming root file paths to nested root directories with a case-sensitive prefix check. On a case-insensitive filesystem, casing that differs between an older root version and the current nested root directory makes a nested file appear root-owned, so writing that root version persists over the nested component's source.
Agent Prompt
## Issue description
Workspace-root writes exclude files owned by nested components using a case-sensitive prefix comparison. On case-insensitive filesystems, paths that differ only by case can refer to the same nested file yet bypass that exclusion and be written from the root version.
## Fix Focus Areas
- scopes/component/component-writer/component-writer.ts[37-42]
- scopes/component/component-writer/component-writer.ts[121-129]
## Recommended Fix
Make nested ownership comparison filesystem-aware: normalize both paths to a common case on case-insensitive platforms before checking complete directory-segment ownership. Use that comparison consistently before adding root files to `DataToPersist`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const writeToPathPerId: Record<string, string> = {}; | ||
| entries.forEach((entry) => { | ||
| if (entry.rootDir === WORKSPACE_ROOT_DIR) return; | ||
| writeToPathPerId[entry.id] = resolveComponentDir(this.workspacePath, entry); | ||
| }); | ||
| throwForOverlappingDirs(writeToPathPerId); | ||
| const ids = Object.keys(writeToPathPerId); |
There was a problem hiding this comment.
12. A clone silently omits a listed member 🐞 Bug ☼ Reliability
writeMembers() indexes destinations in a normal object using the unvalidated member ID, so assigning an entry named __proto__ invokes the inherited prototype setter instead of creating an enumerable destination. Object.keys() then excludes that listed member from the import request, leaving it absent from both the cloned components and the missing-members report.
Agent Prompt
## Issue description
Clone member destinations are keyed by IDs parsed from the versioned bitmap in a normal JavaScript object. The special `__proto__` key is not retained as an enumerable own property, so that member is omitted before importing and is never reported as missing.
## Fix Focus Areas
- scopes/workspace/workspace-root/clone.ts[180-199]
- components/legacy/bit-map/bit-map.ts[1212-1228]
## Recommended Fix
Store destinations in `Object.create(null)` or a `Map`, and derive the import ID list from that safe collection. Validate parsed member IDs before use so malformed bitmap entries fail explicitly rather than being silently skipped.
ⓘ 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 4ba95d3 |
| // git reads the ignore files of a path from the shallowest down, so a deeper rule decides over the | ||
| // one above it. the scan hands them over in whatever order it walked, so they are ordered here | ||
| // rather than by that: the patterns are applied in the order they are returned. | ||
| const byDepth = Array.from(ignoreFileByDir).sort(([dirA], [dirB]) => dirA.split('/').length - dirB.split('/').length); |
There was a problem hiding this comment.
1. Nested-ignore sorting fails formatting 📘 Rule violation ⚙ Maintainability
The byDepth declaration exceeds the configured 120-character print width and has not been wrapped as Prettier requires. Running the repository formatting check against this changed file reformats that declaration, causing the check to fail.
Agent Prompt
## Issue description
The `byDepth` declaration exceeds the repository's configured Prettier print width, so the formatting check fails.
## Fix Focus Areas
- components/legacy/bit-map/component-map.ts[198-198]
## Recommended Fix
Run Prettier on the file or manually wrap the `Array.from(ignoreFileByDir).sort(...)` expression across multiple lines in the form Prettier produces.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| // the live map is never written from a versioned copy, see isWorkspaceMapFile | ||
| if (isWorkspaceMapFile(relativePath)) return; | ||
| if (isOwnedByNestedComponent(relativePath, nestedRootDirs)) return; |
There was a problem hiding this comment.
12. Imported older components lose their map 🐞 Bug ≡ Correctness
ComponentWriter.populateComponentsFilesToWrite excludes every file named .bitmap before persisting it, without checking whether the component is a workspace-root component. An ordinary component version that contains a top-level map file is therefore imported without that file, while checkout deletion treats only components with rootDir === "." as special.
Agent Prompt
## Issue description
The component writer now suppresses `.bitmap` for every component. Suppression is required for workspace-root components so importing one cannot create a nested workspace, but ordinary component versions that contain this file must retain their versioned contents when imported or checked out.
## Fix Focus Areas
- scopes/component/component-writer/component-writer.ts[121-129]
- scopes/component/checkout/checkout-version.ts[127-134]
## Recommended Fix
Make the write-time `.bitmap` exclusion conditional on the component being a workspace-root component (using its root marker or equivalent root-specific state), rather than on the filename alone. Keep the corresponding checkout/remove behavior aligned so ordinary components preserve and update their versioned `.bitmap` files.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const writeToPathPerId: Record<string, string> = {}; | ||
| entries.forEach((entry) => { | ||
| if (entry.rootDir === WORKSPACE_ROOT_DIR) return; | ||
| writeToPathPerId[entry.id] = resolveComponentDir(this.workspacePath, entry); | ||
| }); |
There was a problem hiding this comment.
13. Malformed root maps omit listed members 🐞 Bug ≡ Correctness
writeMembers() stores destinations in writeToPathPerId under entry.id, so a later parsed entry silently overwrites an earlier entry with the same logical ID. readVersionedBitmapEntries() derives IDs from independently supplied map keys and name/scope fields without rejecting duplicates, leaving the overwritten listed member absent from both the import request and the missing-members result.
Agent Prompt
## Issue description
Workspace cloning silently collapses two versioned `.bitmap` entries that resolve to the same logical component ID. The second destination overwrites the first in the per-ID import map, so the clone reports neither an error nor the discarded entry as missing.
## Fix Focus Areas
- scopes/workspace/workspace-root/clone.ts[180-198]
- components/legacy/bit-map/bit-map.ts[1212-1228]
## Recommended Fix
Validate that parsed versioned bitmap entries have unique logical IDs before constructing the per-ID destination map, and throw a clear BitError identifying the duplicate entries. Alternatively, perform the same duplicate check in `readVersionedBitmapEntries()` so all consumers receive validated entries.
ⓘ 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 ea42c93 |
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 . --roottracks it. The flag spells out the intent, sincebit add .is one keystroke fromgit add .and means something else entirely; without it the add is refused with a message naming the flag, and passing it where no path is the workspace root is refused too rather than ignored. Only creating a root needs it — re-adding one that exists does not, so the existing "already tracked by" error still surfaces. It is tracked 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.