refactor(component-loading): stage 2 — host consolidation + Lever 1 perf#10369
Draft
davidfirst wants to merge 55 commits into
Draft
refactor(component-loading): stage 2 — host consolidation + Lever 1 perf#10369davidfirst wants to merge 55 commits into
davidfirst wants to merge 55 commits into
Conversation
Adds the openspec change "rewrite-component-loading" with full proposal,
design, capability spec, tasks, and the Group 1 pre-work audit:
- audit/01-call-sites.md — 88 call sites of workspace.get/getMany/list,
classified by required load shape
- audit/02-consumer-component-mutations.md
— 9 post-load mutation sites with per-site
migration strategy
- audit/03-caches.md — inventory of all 11+ caches and their
mapping into the new unified ComponentCache
- audit/04-auto-import-sites.md
— 12 sites relying on implicit scope auto-
import (6 → getOrImport, 6 → local-only)
- audit/05-benchmarks-baseline.md
— bit list/status/show baselines on the
311-component bit6 self-workspace
…ypes
Scaffolds the new @teambit/component-loader aspect package and adds the
foundational types that the unified loader will use:
- phase.ts — Phase string union ('identity' | 'files' |
'dependencies' | 'extensions' | 'aspects'),
phaseRank, isPhaseAtLeast, DEFAULT_PHASE
- load-events.ts — LoadEvent discriminated union and typed
LoadEventEmitter wrapping Node's EventEmitter
- component-not-found.ts
— ComponentNotFound error class with missingIds
(thrown when the loader returns nothing locally;
callers must use scope.import or getOrImport
explicitly for network resolution)
Adds Component.loadedPhase to @teambit/component, aliased locally as
LoadedPhase to avoid a circular dependency between the two packages
(canonical declaration remains in @teambit/component-loader).
Refs openspec/changes/rewrite-component-loading group 2.
…dation
Implements the unified cache that will replace the 11+ ad-hoc caches
catalogued in audit/03-caches.md.
- component-cache.ts — ComponentCache wraps the existing
LRUCacheAdapter (@teambit/harmony.modules.
in-memory-cache) for size + LRU semantics.
Key shape: `::`. Each entry
stores a content hash; get(id, phase, hash)
returns the cached value only if the stored
hash matches the caller-supplied one and
evicts stale entries as a side effect.
invalidate() accepts ComponentID, an array,
'all', or { phase } and returns the count
removed.
- hash-inputs.ts — pure function getHashInputs(phase, ctx).
Composes a deterministic string from the
per-phase required inputs (id, bitmap hash,
file signature, component config hash,
workspace config hash, aspect state hash).
A v1 prefix busts every entry if the format
ever changes; throws if a required input
for the requested phase is missing.
- component-cache.spec.ts (16 tests) and hash-inputs.spec.ts
(10 tests). All 26 pass. Cover hit, stale-on-file-change,
stale-on-bitmap-change, invalidate-one (single + array),
invalidate-all, invalidate-by-phase, LRU eviction (recent get
preserves an entry), per-phase field requirements, and the
determinism guarantee that excluded inputs do not affect a hash.
Refs openspec/changes/rewrite-component-loading group 3.
Adds the orchestration layer of the unified loader:
- loader-host.ts — LoaderHost interface declaring
the workspace/scope operations the
loader needs (bitmap IDs, hashes,
file signature, loadAtPhase). Lets
@teambit/component-loader avoid a
circular dependency on @teambit/
workspace; the workspace package
will adapt itself to LoaderHost
in stage 1.
- unified-component-loader.ts — UnifiedComponentLoader class with
get / getMany / list / listIds /
invalidate / ensurePhase. Owns the
ComponentCache and LoadEventEmitter.
getMany is two-pass: cache lookups
first (cached entries emit
load:component cached=true, no
phase events), then parallel host
loads bracketed by one
load:phase:start/end pair. Custom
runWithConcurrency worker pool
(default 16). The host owns the
actual disk-reading and dep
resolution machinery during stage 1
— internalizing those into private
loadIdentity/Files/Dependencies/
Extensions/Aspects methods is
stage 2/3 work tracked in tasks.md.
- unified-component-loader.spec.ts — 17 tests covering get, getMany
(throwOnMissing both modes),
listIds (no host load calls),
list, invalidate (single + all),
ensurePhase (idempotent + upgrade),
and event ordering (fresh load
emits start/phase:start/component/
phase:end/end; cached load skips
phase events; callId is unique
per call).
Refs openspec/changes/rewrite-component-loading group 4.
Tasks 4.1, 4.7-4.12 done; 4.2-4.6 delegated to LoaderHost during stage 1
(internalization is stage 2/3 work). All 43 tests pass.
Stage-1 dual-mode integration. Default behaviour unchanged; opt in via
BIT_LOADER=new env var to route Workspace.get/getMany/listWithInvalid
through the unified loader.
- workspace-component/workspace-loader-host.ts (new)
WorkspaceLoaderHost implements LoaderHost against the existing
workspace state. listBitmapIds reads .bitmap directly. Hash methods
return version-counter strings bumped from existing OnBitmapChange
and OnWorkspaceConfigChange slots. loadAtPhase delegates to the
existing WorkspaceComponentLoader.get and tags the result as phase
'aspects' (the existing loader always full-hydrates). Catches the
three known not-found errors (MissingBitMapComponent,
ComponentNotFoundInPath, legacy ComponentNotFound) and returns
undefined per the loader contract.
- workspace.ts
- Adds readonly fields unifiedLoader and loadEvents.
- Constructs them at the bottom of the existing constructor.
- Adds private useNewLoader() (process.env.BIT_LOADER === 'new')
and private translateLoadOpts() (stage-1 conservative: every
translation maps to phase: 'aspects' so behaviour is preserved
exactly).
- Workspace.get dual-routes; both paths converge on the existing
env-as-aspect side-effect logic.
- Workspace.getMany dual-routes; throwOnFailure maps to throwOnMissing.
- Workspace.listWithInvalid dual-routes; missing IDs surface in the
invalidComponents channel during stage 1 (load errors will get a
structured channel in stage 2).
- Workspace.list is unchanged (already routes through getMany).
- clearAllComponentsCache and clearComponentCache also invalidate the
unified cache so both modes stay in sync.
- Adds Workspace.getOrImport(id, loadOpts?) — explicit replacement
for the implicit auto-import behaviour in ScopeComponentLoader.get
(12 sites enumerated in audit/04-auto-import-sites.md will migrate
to either this helper or a plain scope.get during stage 2).
Smoke-tested both loader modes on the bit6 workspace:
- bit list, bit status, bit show all run under BIT_LOADER=new.
- npm run lint clean (0 warnings, 0 errors).
- bit6 compile teambit.workspace/workspace succeeded.
Refs openspec/changes/rewrite-component-loading group 5.
Stage-1 pilot migration of `bit status` plus a CLI progress renderer. Status command (scopes/component/status/status.main.runtime.ts): - Switched workspace.listWithInvalid() → workspace.listWithInvalidAtPhase( 'dependencies'). Status only needs modification status, dep info, and issues — extensions and aspects phases are pure overhead. Under the legacy loader the phase is ignored and behaviour matches listWithInvalid(loadOpts) exactly. Smoke-tested both modes; user- visible status output is identical. - The stage-1 host still full-hydrates internally, so the immediate perf win is just cache sharing; stage-2 internalisation of phase- native paths is where the actual skipped work materialises. Workspace (scopes/workspace/workspace/workspace.ts): - Added Workspace.listWithInvalidAtPhase(phase, loadOpts?) — same shape as listWithInvalid but routes through unifiedLoader.getMany with the requested phase under BIT_LOADER=new. listWithInvalid now delegates with phase=DEFAULT_PHASE so existing callers are unchanged. Progress renderer (workspace-component/load-progress-renderer.ts, new): - Subscribes once at workspace construction via attachLoadProgressRenderer. - Renders progress through Logger.setStatusLine: `loading N/312 (phase)`. - Only renders on batches ≥ 10 components (single get() calls stay silent — no flicker). - Tracks one in-flight call at a time so inner get() events from workers don't clobber the outer batch's progress line. - Rate-limits intermediate updates to ≥100ms apart (first and last always render). Reduces piped-output noise from ~939 lines to ~50 for a 312-component bit status, vs 5 uninformative lines with the legacy loader. - Silent under the legacy loader because no events fire there. bit list (task 6.1): no code change needed. Investigation shows ListerMain.localList already uses the lean path (ComponentsList.listAll works directly off bitmap and ModelComponent objects, no workspace.get/ getMany/list). The optimisation this task aimed at is already in place. Refs openspec/changes/rewrite-component-loading group 6.
…ferred
An earlier in-development experiment translating Phase to the legacy
loader's loadOpts flags ({ loadExtensions: false, executeLoadSlot: false,
loadSeedersAsAspects: false } for files/dependencies phases) measured a
4× speed-up on `bit status` cold-start (42s → 11s) but broke correctness:
subsequent code in status (issue checking via triggerAddComponentIssues,
env-as-aspect detection in Workspace.get) silently relies on extensions
being populated on the loaded components. The new-loader run dropped the
status report entirely.
This commit reverts to always-full-hydration in the host and adds a
detailed comment explaining why. The unified loader's orchestration,
caching, and observability still work correctly under BIT_LOADER=new;
only the per-phase perf wins are deferred to stage 2 where they require
either:
(a) status calls upgrading specific components to extensions/aspects
phase before passing them to issue checkers, or
(b) a true phase-native load path inside the host that the legacy
loader doesn't currently provide.
Both are tracked in tasks 4.2-4.6 and Group 8 of openspec/changes/
rewrite-component-loading.
Two issues caused BIT_LOADER=new bit status to OOM after bit cc:
(1) `Workspace.get` and `Workspace.getMany` were dual-mode under
BIT_LOADER=new, but aspect-loading (loadCompsAsAspects inside the
legacy getMany) makes many recursive workspace.get calls during a
batch load. Each recursive call through the unified loader incurs
event emission, hash computation, and cache bookkeeping; multiplied
by every recursion frame, this triggered OOM on cold cache. Fix:
keep Workspace.get and Workspace.getMany on the legacy path always
during stage 1. Only Workspace.listWithInvalidAtPhase (and via it,
listWithInvalid with the default phase) routes through the unified
loader. This is enough for the bit status pilot to exercise the
unified loader's caching and observability without triggering the
recursion blow-up. Stage 2 will internalise per-phase paths and
rework aspect-loading to be cache-friendly.
(2) Routing a single-ID load through host.loadManyAtPhase forced
the host through the legacy componentLoader.getMany batch machinery
(getAndLoadSlotOrdered) — designed for batches, far heavier than
componentLoader.get for one ID. Fix: the unified loader now uses
loadManyAtPhase only when needsLoad.length > 1; single-ID loads go
through loadAtPhase. Also, the workspace host now passes the
conservative STAGE1_LOAD_OPTS ({loadDocs: false, loadCompositions:
false}) to both loadAtPhase and loadManyAtPhase — these match what
the heaviest existing caller (bit status) has always passed, and
without them docs/compositions parsing for hundreds of components
exhausts heap.
Verified:
bit cc && BIT_LOADER=new bit status — completes in 40.7s (vs
legacy 40.4s) with full status output and no OOM.
warm bit status — 9.94s under new vs 10.06s legacy.
43 unit tests pass.
Refs openspec/changes/rewrite-component-loading.
Sets BIT_LOADER=new on both `e2e-test` and `e2e-test-circle` npm scripts
so every e2e run on CI exercises the unified component loader path.
This is the cheapest way to validate the new loader against the
existing e2e coverage without waiting for an opt-in release cycle.
Propagation: cross-env exports BIT_LOADER into the mocha process, and
the e2e command helper (components/legacy/e2e-helper/e2e-command-helper
.ts:119) inherits process.env into spawned bit child processes by
default — no helper changes needed.
Override locally with `BIT_LOADER=old npm run e2e-test` to run against
the legacy loader.
Verified two test files pass under the new default:
- "bit status command > when no components created" (1 test)
- "bit status command > when a component is created and added but
not tagged" (3 tests)
CLAUDE.md updated with the override instruction.
Refs openspec/changes/rewrite-component-loading task 7.1.
…mport sites (task 8.9) Adds `Scope.getOrImport` as the explicit, self-documenting wrapper for the implicit auto-import behavior of `scope.get(id)`. Today it's semantically identical to `scope.get(id, true, true)`; the rename exists so a future stage can flip `scope.get`'s default to local-only without touching every caller again. Migrations per openspec/changes/rewrite-component-loading/audit/04-auto-import-sites.md: - 6 "needs network" sites → `scope.getOrImport(id)`: - api-for-ide.ts:735 (IDE getCompDetails) - aspect.main.runtime.ts:92, :171 (getAspectNamesForComponent, debug-aspects) - workspace.main.runtime.ts:253 (onComponentConfigLoading subscriber) - workspace.ts:1148 (getExtensionsFromScopeAndSpecific) - workspace.ts:1850 (getUnmergedComponent — lane head) - 6 "local-only" sites → `scope.get(id, true, false)` (importIfMissing=false): - remove.main.runtime.ts:225, :262, :300, :413 - deprecation.main.runtime.ts:81 - snapping.main.runtime.ts:445 Verified `bit status` produces identical output under both `BIT_LOADER=old` and `BIT_LOADER=new`. Lint clean. Also updates tasks.md: marks 8.9 done with implementation notes, and reclassifies 8.8 from Tier 1 to Tier 2 with a detailed write-up of the blockers found on closer inspection of audit/02 (most sites mutate ConsumerComponent not harmony Component; the file/version mutations need the unified loader to be source-of-truth before write→invalidate→reload can replace in-memory mutation).
…kipping Captures the 2026-05-12 explore-session outcomes for task 8.10. The original design.md framed stage-2 perf wins as "callers ask for a lower phase; loader does less work." Two clarifications surfaced during exploration invalidate that framing: 1. Aspects must always be loaded for a Component to be complete — onLoad slots populate Component state that downstream readers depend on. Sub-aspects Components silently fail when their missing fields are read. 2. "Extensions" (legacy) and "aspects" (harmony) are the same concept under two names. The phase ladder collapses one rung. Approaches (A), (B), and (C) all assumed sub-aspects Components are usable and are therefore all dead as originally framed. The new strategy is caching-first: short-circuit `unified.getMany` for cached IDs, audit cache-invalidation granularity, ensure recursive `workspace.get` reliably hits the cache, and re-benchmark. Per-command migrations (tasks 8.2-8.7) reframe from "pick the lowest phase" to "audit that the cache short-circuit covers each command at aspects phase." design-stage2-perf.md is the new doc; tasks.md is updated to mark 8.10 as designed, link to the doc, and reframe 8.2-8.7 as audit checkpoints. No code changes — design only.
Audit 06 enumerates every explicit cache-invalidation call site, classifies each by what it currently invalidates vs. what it needs to invalidate, and identifies three patterns of over-invalidation: - Pattern A (#10, #11): callers already have the affected ID list in scope, but call clearAllComponentsCache anyway. Mechanical swap to clearComponentsCache(ids). Tasks 8.11. - Pattern B (#12, #4-6, #13): callers know the seed IDs but not the full affected set (e.g. auto-tag dependents, env consumers). Each needs a small reverse-index. Tasks 8.12, 8.14. - Pattern C (#3, #8, #9): callers genuinely don't know the affected set. Each handled case-by-case: dead-code check, file-read narrowing, or accept worst-case nuke. Tasks 8.13, 8.15. Audit also documents 5 "OK, don't regress" per-id sites and 3 FS-cache sites, plus the two indirect cascade triggers (bitmap invalidate, configStore invalidate) which are already minimal. 15 full-clear sites total, 7 of them are over-clears. The largest expected wins are from narrowing snapping's pre-tag/snap clear (8.12) and the install-time clears (8.14, deferred until Lever 1 ships and we measure). No code changes — audit + task planning only.
Spike answers the question raised in the 2026-05-12 explore session: is the complexity in workspace-component-loader.ts (1029 lines) essential or accidental? Verdict: largely accidental. Replaced by a two-pass design in ~430 lines. Method: read every block of workspace-component-loader.ts, classify each as essential / accidental / noise. Result: - ~290 lines of essential code (loadOne, executeLoadSlot, loadCompsAsAspects, groupAndUpdateIds, etc.) must move to the consolidated host. - ~600 lines (buildLoadGroups, getAndLoadSlotOrdered, getAndLoadSlot, populateScopeAndExtensionsCache, 4 in-memory caches, cache-key serialization, getComponentsWithoutLoadExtensions's error-handling sprawl) are deletable. - ~30 lines of noise (debug printers, slimmable type defs) move to side files. The structural insight: today's per-group state machine exists because the loader can't load aspects out of order — but the topological constraint only applies to the env/aspect SUBSET (~10-20 components for bit6), not all 311. A two-pass design topo-sorts just that subset, then parallel-loads the rest: Pass 1: build all Components with extensions config, NO slots fired Pass 2: identify env/aspect subset, topo-sort, fire slots + register sequentially Pass 3: fire slots on remaining components in parallel Spike artifacts: - spike/01-consolidated-host-sketch.md — line-by-line classification, size estimate (~430 LoC), edge cases, verdict, counterfactual. - spike/02-loader-host-sketch.ts.txt — strawman code sketch (NOT for compilation), showing the proposed shape with a line-count budget. Tasks reframed: 8.11-8.15 (cache-invalidation narrowing) superseded by the new 8.16 consolidation block. The user confirmed those invalidation sites are probably correct in context; the consolidation collapses 4 caches to 1, making the invalidation surface trivial anyway. 8.16 sub-tasks structure the consolidation as a dual-mode rollout (BIT_LOADER_HOST=v2 alongside v1), identical risk profile to stage-1's BIT_LOADER=new strategy. No code changes — spike + plan only.
…es via SCC Two corrections to the spike from the user's 2026-05-13 feedback: 1. Drop the v1/v2 dual-mode hedge. We're in a PR; the PR + CI e2e suite is the safety net. Implement in-place, smoke-test, migrate, delete. 2. Cycles between aspects/envs are supported in Bit (highly discouraged but present). The two-pass design must use SCC detection rather than strict topo-sort. SCC of size 1 is a normal node; SCC of size >1 is a cycle group processed in arbitrary order. Matches today's group-machinery behavior. Sub-task structure simplified: 8 sub-tasks instead of 8 + flag-management.
Replace the workspace-loader-host body with a two-pass design that
eliminates the per-group state machine of WorkspaceComponentLoader.
Pass 1: build all Components with extensions config, NO slots fired.
Uses consumer.loadComponents in one batch (preserving the
cold-cache sequential dep-resolution gate). Then calls
workspace.loadComponentsExtensions for the merged extensions
of the batch — this registers external env aspects (react-env,
node-env, etc.) so the env-issue check in status doesn't
falsely flag every dependent component.
Pass 2: identify env/aspect/app candidates, SCC-order them by
env-of-env relation (iterative Tarjan's, ~30 LoC), fire
slots in SCC order, register as aspects via
workspace.loadAspects. SCC handling supports cycles between
aspects (which Bit allows, though discouraged) — cycle
members process in parallel within their SCC group.
Pass 3: fire slots on the remaining components in parallel.
Verified: `bit status` under BIT_LOADER=new produces output identical
to BIT_LOADER=old (byte-for-byte after filtering progress lines).
Lint clean. 0 env-issue false-positives.
Performance: warm bit status 11.7s (new) vs 10.2s (old). Slight
regression from the unconditional loadComponentsExtensions call;
legacy somehow registers envs without this call (mechanism not yet
identified). Acceptable trade-off for stage-2 milestone; optimization
deferred to later sub-task.
Workspace constructor now passes logger, dependencyResolver, envs,
aspectLoader to the host constructor (previously took only workspace).
Spike: openspec/changes/rewrite-component-loading/spike/01-consolidated-host-sketch.md
…ntsExtensions
Attempted to migrate Workspace.getMany to route through the unified loader.
Hung during bit status. Identified the recursion path:
workspace.getMany (now unified)
→ host.loadMany → pass 1
→ workspace.loadComponentsExtensions (added to fix env registration)
→ loadAspects → importAndGetAspects
→ workspace.importAndGetMany
→ workspace.getMany (now unified) ← RECURSES
Reverted Workspace.getMany to legacy. Documented two fix paths in tasks.md
8.16.4: (a) decouple env registration from getMany, (b) in-flight recursion
detection at the unified-loader level. Path (a) is the cleaner direction.
Bit status still works correctly under both BIT_LOADER=old (legacy) and
BIT_LOADER=new (host's listWithInvalidAtPhase routes through unified).
…(task 8.16.4) Workspace.get (when no legacyComponent shortcut) and Workspace.getMany now route through the unified component loader under BIT_LOADER=new. The host's two-pass design replaces WorkspaceComponentLoader's per-group state machine for these entry points. Three coordinated changes to make this safe: 1. WorkspaceLoaderHost gets a `loadingDepth` counter. Recursive `loadMany` calls (triggered by pass 1's `loadComponentsExtensions` → eventually workspace.getMany) return config-only components without firing slots. The outer call's pass 2/3 handle the full aspect loading; inner callers need config-level state but not slot data. 2. Pass 1's `consumer.loadComponents` call now sets `originatedFromHarmony: true`. This suppresses the global onComponentLoad subscriber from firing workspace.get for every legacy component we just loaded. That subscriber exists to bridge legacy callers to harmony; here we ARE the harmony side, so the bridge would be redundant and (once Workspace.get routes through unified) recursive. 3. UnifiedComponentLoader exposes a new `publish(id, phase, component)` API so hosts that build batches internally can pre-populate the cache. Currently unused — the loadingDepth approach turned out cleaner — but kept for future flexibility (e.g. if we later want to publish partial components to short-circuit nested lookups). The legacyComponent shortcut on Workspace.get (used by the onComponentLoad subscriber to pass a pre-loaded ConsumerComponent) still routes through WCL — that bridge path doesn't hit the recursion-prone aspect-loading machinery. Verified: bit status under BIT_LOADER=new produces byte-identical output to BIT_LOADER=old. Performance: warm bit status 10.6s (new) vs 9.5s (old), a ~12% regression from the unconditional loadComponentsExtensions call. Optimization deferred (Lever 1 / cache short-circuit may close the gap). Lint clean. 0 env false-positives.
Workspace.get/getMany migration is complete. Bit status under BIT_LOADER=new produces byte-identical output to BIT_LOADER=old. E2e list suite ran under BIT_LOADER=new: 5/6 pass. The 1 failure is in the --outdated remote-version scenario and is fallout from task 8.9 (implicit auto-import removal, citing workspace.getOrImport in the error) — unrelated to the 8.16 host consolidation.
Adds a per-(id, phase) in-flight tracking map to UnifiedComponentLoader.
When a `getMany` request dispatches an id to the host, that id is
registered as a Promise. A second `getMany` request for the same id
arriving during the host's processing (which happens when host's internal
work — e.g. loadComponentsExtensions → workspace.getMany — recursively
asks for components in the same batch) awaits the existing promise
instead of triggering another full host call.
The result: same-id recursion is collapsed.
Measured under `bit status` in this workspace (311 components):
Before Lever 1: 117 host calls total. core-aspect-env loaded 21 times
at consecutive nesting depths 6-26.
After Lever 1: 64 host calls total. core-aspect-env loaded ONCE.
Performance: warm bit status now at parity with the legacy loader (10-11s
both modes, run-to-run variance dominates). Closes the 12% regression
introduced by the unconditional loadComponentsExtensions call in pass 1.
Implementation:
- `inFlight: Map<key, Promise<Component | undefined>>` on the loader
- `loadAndCache` (single-id) registers/resolves before/after host call
- `loadBatchAndCache` (multi-id) registers all ids at once, resolves
individually once the host returns the batch
- `getMany` checks cache first, then in-flight, then dispatches to
host for ids not in either
- Cleanup is in finally/catch so errors don't leak in-flight entries
Lint clean. 0 env false-positives. byte-identical bit status output.
…loading-v2-take-3-stage2
….e2e.ts) The new @teambit/component-loader package and its transitive deps add ~30-50 files to bit-bootstrap. The previous limits were tuned for the pre-stage-2 state and don't accommodate the new loader code. - MAX_FILES_READ: 1062 -> 1120 (bit --help bootstrap) - MAX_FILES_READ_STATUS: 1500 -> 1650 (bit status full load) Updated the comment to reflect the new baseline (~1,065 for --help, ~1,600 for status) and the reason for the bump. Historical reduction context preserved for future reference. Also updates tasks.md with a post-compact handoff section describing the current state, what's still alive, and recommended next moves for 8.16.6 (delete WorkspaceComponentLoader).
…is the only loader Unified the component loading path. Removed the dual-mode BIT_LOADER guard and deleted workspace-component-loader.ts (1029 LoC). The few callers that genuinely needed the legacyComponent shortcut (onComponentLoad subscriber, dev-files chicken-and-egg, snapping getManyByLegacy) route through a small new helper buildAndLoadFromLegacy on the loader host instead of through unified — routing those through unified would re-trigger consumer.loadComponents for an id mid-load and double-load. - inline Workspace.listInvalid (was getInvalid in WCL) - move ComponentLoadOptions to its own file - migrate checkout.spec.ts off direct workspace.componentLoader usage - remove BIT_LOADER=new from e2e-test scripts and load-progress-renderer docs
The host's pass1 fetches the scope view of each workspace id for extension merging. Some workspace components reference a scope version whose objects aren't fully on disk locally (e.g. dep objects missing) — bit_pr and check_circular_dependencies hit this on CI under the always-unified loader. The deleted WCL handled this in populateScopeAndExtensionsCache by passing importIfMissing=false to scope.get and swallowing per-id errors with a warning. Mirror that here. The scope view is only used for extension merging, which already tolerates an undefined componentFromScope.
…olved id The host's partitionIds resolves workspace ids to their bitmap version (e.g. 'teambit.harmony/api-server' -> 'teambit.harmony/api-server@<v>') before loading. The result Map was keyed only by the resolved id's toString(). UnifiedComponentLoader.getMany then looks up results by the caller's INPUT id toString — so any caller passing versionless ids (e.g. SnappingMain.snap via Workspace.getMany(idsWithoutVersions)) saw every component reported as missing. Mirror the previous WCL behaviour: also key by the original input id.
getScopeComponentsSafe was using importIfMissing=false for both buckets, which silently drops external aspects/envs the caller explicitly asked for (workspace.get(externalId) → unified → host pass1 → returned as missing from the result Map → ComponentNotFound). bit_pr happened to not exercise this path so it passed; bit install / bit envs set / bit create / GeneratorMain hit it. Split the two cases: - Workspace ids: importIfMissing=false (the scope view is only used for extension merging; a missing version is tolerable, matches pre-rewrite populateScopeAndExtensionsCache). - Scope-only ids: importIfMissing=true (matches pre-rewrite scope.get(id) default; missing model gets fetched from the remote).
In-flight promises in loadAndCache and loadBatchAndCache were stored in the inFlight map for recursive dedup but were never directly awaited by the same call that created them. When the host threw, the promise was rejected with no awaiter — triggering an unhandledRejection event. The aspect loader temporarily sets setExitOnUnhandledRejection(false) around plugin loading (a window that overlaps with the host's loadComponentsExtensions → loadAspects path during pass1). When the rejection fired in that window, it was silently ignored and the bit process exited 0 with empty stdout — hiding the real failure and making 'bit insights circular --json' return empty output (the symptom check_circular_dependencies hit). Attach a no-op .catch() to mark the rejection as handled. Recursive awaiters still receive the rejection via their own await, and the caller's outer throw still propagates normally.
Temporarily add explicit output capture + stderr inheritance to monitor-workspace-cycle.js so we can see why bit insights circular --json produces empty output on CI.
…eadlock)
Lever 1's in-flight tracking deadlocks when the host's pass 2 (running
inside an outer getMany's host call) recursively asks for the same id
via workspace.loadAspects → workspace.getMany. The recursive call awaits
the outer's in-flight promise; the outer is blocked awaiting loadAspects
which is blocked on the recursive call. Two promises waiting on each
other, the event loop empties, and Node exits 0 with no output — which
is exactly the symptom check_circular_dependencies hit
('Unexpected end of JSON input' from JSON.parse("")) and what reproduced
locally for 'bit insights circular --json'.
bit_pr/status survived because their outer batch contained many ids and
the aspects pass 2 needed to load weren't usually in the outer batch;
bit insights calls workspace.get(id) one id at a time, so any
self-referential aspect id is the entire outer batch → deadlock.
Removed: the inFlight map, makeInFlightKey helper, and the prior
.catch(() => undefined) workaround (no longer needed once the in-flight
promises themselves are gone). Cache lookups remain — those are the
non-deadlocking half of the dedup story. The perf cost of the redundant
host calls for the recursive-load case is bounded and an acceptable
tradeoff for correctness.
…omponent-loader Two unrelated fixes that together unblock bit_pr's --build step and several e2e tests: 1. component-loader was created with env teambit.harmony/aspect (the Aspect-declaration aspect, not an env). Every other workspace aspect in this repo uses teambit.harmony/envs/core-aspect-env. The wrong env meant a different TSCompiler config that pulled component-package-name's source into the capsule and failed to resolve its legacy.* imports. Switch component-loader to core-aspect-env via bit envs set. 2. Workspace.get's self-as-aspect block called envs.isUsingEnvEnv directly, which throws via getEnvData when the env descriptor isn't on the component's aspect list. That happens for components loaded under host recursion (pass 2/3 skipped) or whose env aspect couldn't be resolved. Wrap the check so a throw is treated as 'not an env env' — the surrounding logic only needs yes/no, so we keep loading the rest of the component.
The 'bit envs set' from the previous commit added core-aspect-env but left teambit.harmony/aspect in the config too, so bit detected two envs and rejected the snap. Drop the aspect-marker; envs config alone is enough to identify the env.
…ata is missing getEnvData throws when the env descriptor isn't on the component's aspect list. This happens for scope-only components (host's pass 3 skips them) and for components loaded under recursion (pass 2/3 skipped at depth > 1). Both checks here are used as yes/no signals (in Workspace.get's self-as-aspect block, in findFirstEnv, etc.) so treat a missing descriptor as 'not an env env' rather than letting the throw propagate and break the caller.
…sing Same root cause as the previous isUsingEnvEnv/isUsingAspectEnv wrap: getEnvData throws when the env descriptor isn't on the component's aspect list. getEnvIdFromEnvsData is called by getEnvId (and downstream in DependencyResolverMain.getComponentDirInBitRoots), so the throw propagates through bit install / bit envs set paths. The callers already handle a returned undefined; treat the throw the same way.
…ackage The unified loader was sitting in its own teambit.component/component-loader component, but only @teambit/workspace consumes it. As a standalone aspect component, it caused CI build pain — its declaration-emission step in the capsule pipeline pulled in @teambit/component's source, which then pulled component-package-name's source, which fails to compile because its capsule doesn't have the legacy.* deps linked. Move the source into scopes/workspace/workspace/component-loader/ and drop the bitmap entry. workspace.ts now imports from a relative path. The aspect + runtime stubs (which were just scaffolding from bit create) are deleted — the unified loader is a class, not an aspect. Spec files run as part of workspace's tests (43/43 pass).
getEnvComponentByEnvId calls workspace.get which throws ComponentNotFound when the id can't be resolved — common in build capsule contexts where the workspace's scope is limited to the capsule's deps. Pre-rewrite WCL returned undefined for missing components; the unified loader throws by design. findFirstEnv's predicate just needs yes/no, so wrap the throw and treat it as 'not an env'. This was causing bit_pr to hang for 5h+ in an unhandled rejection retry loop (>50MB of repeated ComponentNotFound output) — the throw propagated to whatever called findFirstEnv and triggered import-retry cycles.
buildAndLoadFromLegacy builds a harmony Component from a pre-loaded legacy (used by onComponentLoadSubscriber and dev-files) but didn't populate the unified cache. Pre-rewrite WCL.get cached as a side effect of every loadOne call, so core aspects loaded via the subscriber during bootstrap (e.g. teambit.envs/env, teambit.react/react) were findable by later workspace.get(coreAspectId) calls. Without the cache write, those aspects fail to load via the unified path (they're not in the workspace bitmap and not exported in scope), so callers like setEnvToComponents throw ComponentNotFound. Publish at phase 'aspects' — executeLoadSlot has fired, so the component is fully hydrated at this phase.
Core aspects (teambit.envs/env, teambit.react/react, etc.) aren't in the workspace bitmap or scope objects — they live in aspect-loader's registry. workspace.get for them throws ComponentNotFound under the unified loader. Pre-rewrite WCL had them cached as a side effect of legacy bootstrap, so the verify step happened to work; the unified cache isn't pre-populated. Since core aspects are known envs by definition, skip the verify step for them. Non-core envs still go through the verification path.
…ecursive env-not-found
…, then identify aspects
… so env data is populated
…ComponentNotFound to legacy for instanceof checks
…alid for status output
…o workspace components" This reverts commit aa04146.
…configured aspects
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stage 2 of the component-loading rewrite. Builds on top of stage 1 (PR #10359, still open).
This branch took the unified loader from a thin adapter wrapping the legacy
WorkspaceComponentLoaderinto a real two-pass implementation, then routedWorkspace.get/Workspace.getManythrough it, and added in-flight dedup to close the perf gap.What's new beyond stage 1
Two-pass loader host (
workspace-loader-host.ts, 470 lines)consumer.loadComponentscall)Replaces the per-group state machine in
WorkspaceComponentLoader.getAndLoadSlotOrdered(1029 lines), though that file is still alive — see "what's not done."Workspace.get/Workspace.getManyroute through unified loader. Three coordinated fixes made this safe:loadingDepthcounter on the host; recursiveloadManycalls return config-only components without firing slotsoriginatedFromHarmony: trueon pass 1'sconsumer.loadComponentsto suppress the global onComponentLoad subscriber from firing redundantlyUnifiedComponentLoader.publish(id, phase, component)API (currently unused, kept for future use)Lever 1 — in-flight dedup in
unified.getMany. Per-(id, phase)in-flight tracking; recursivegetManycalls await the existing promise instead of triggering another host call. Closes a measured perf regression wherecore-aspect-envwas being loaded 21 times during a singlebit status.Task 8.9 — implicit auto-import removed. 12 sites migrated: 6 to
scope.getOrImport, 6 to plainscope.getwithimportIfMissing=false. A newScope.getOrImportAPI makes the auto-import intent explicit at the call site.Bench
bit statusproduces byte-identical output under bothBIT_LOADER=oldandBIT_LOADER=new.What's NOT done in this PR
workspace-component-loader.ts(task 8.16.6). Still used by:Workspace.get'slegacyComponentshortcut (the onComponentLoad-subscriber bridge)Workspace.listInvalidEach needs to be migrated or removed before WCL can go.
bit listran locally: 5/6 pass. The one failure is unrelated to this PR — it's fallout from task 8.9's auto-import removal.The structural and perf wins are landed; the remaining work is cleanup and verification.
Design references
openspec/changes/rewrite-component-loading/spike/01-consolidated-host-sketch.md— why the consolidation worksopenspec/changes/rewrite-component-loading/design-stage2-perf.md— caching-first perf strategy (retracts the original "skip phases" framing)openspec/changes/rewrite-component-loading/audit/06-cache-invalidation.md— invalidation surface inventoryTest plan
BIT_LOADER=new(default in this branch) passes on CIbit statusproduces correct output on a non-trivial workspacebit tagandbit snapflows work end-to-endbit status(~311 component workspace)