Skip to content

feat(lanes | component): redesign the changes & compare experience#10378

Open
luvkapur wants to merge 114 commits into
masterfrom
changes-redesign
Open

feat(lanes | component): redesign the changes & compare experience#10378
luvkapur wants to merge 114 commits into
masterfrom
changes-redesign

Conversation

@luvkapur

@luvkapur luvkapur commented May 15, 2026

Copy link
Copy Markdown
Member

Reworks how changes are viewed across the app: the single-component compare, the lane compare, and the workspace/lane overviews, onto one shared, bulk-fetched implementation, and replaces the Monaco-based code diff with a lean shiki-based viewer. It also brings the semantic API diff into the compare surface as a first-class view, and fixes lane-vs-main comparison. The goal was best-in-class feature and performance parity for reviewing changes, with Bit's own tokens, theme, and identity.

Unified compare surface

  • InlineComponentCompare is now the single implementation shared by the single-component compare page and lane-compare: one set of view-mode tabs (Code / Preview / Docs / Dependencies / Tests / Config / API), one diff pane, one toolbar.
  • Which views appear is driven by real per-view content counts (changed files, compositions, changed aspects, dep changes, and so on), so a reviewer never lands on an empty view, mirroring the cloud changes view.
  • View switching is instant: panels are all mounted and shown/hidden purely via a data-view-mode attribute plus CSS, so a mode click never remounts or re-fetches (fixes the "switching is slow" and console-error regressions).
  • DiffModeProvider/useDiffMode gives a single global split/unified toggle across every file instead of a per-file one.

Comparing a lane against main

  • Lane-vs-main compare no longer depends on main's version being in the local scope: base resolution (getHeadOnMain / importMainHeads) falls back to the remote default-lane ref and bulk-imports the missing main heads, so lane-only components whose main version was never fetched are compared correctly instead of being misclassified as new or "no changes".
  • Fixed two separate "shows no changes vs main" bugs. First, skipUpToDate was dropping every component of a lane that is forked and ahead of main: a forward-only lane has no target-only snaps, so each read as up-to-date and got filtered out; source-ahead components are now kept. Second, the diffStatus result-memo key was derived from Lane.hash (a method, not the raw hash), so it never invalidated when the lane or main advanced and served a stale empty result; it is now keyed on the real hash plus main heads.
  • The main-to-lane merge direction uses the same remote-aware getHeadOnMain, so "merge from main" no longer drops components without a local main head.
  • New per-pair source indicator (baseSource: 'workspace' | 'scope'): a small cloud badge on the sidebar row marks bases resolved from the remote scope versus the local workspace.

Bulk data fetching

  • New CompareDataProvider plus a bulk compareComponents(pairs) GraphQL query fetch code/aspect/test diffs for all component pairs in one paginated call, keyed by compareId; inline panels read from this context instead of firing per-file queries.
  • FileRegistry context feeds the sidebar file tree from the same bulk result.
  • On large lanes (a 204-component lane vs main), cold changes derivation dropped from about 47s to about 8s: diff raw Version objects instead of fully loading both components, defer the (UI-unused) API change-type off the critical path, and lift the artificial concurrency cap. A client render loop in the file registry was also removed via useSyncExternalStore plus a version-guarded notify.

New code diff viewer (no Monaco)

  • New teambit.code/ui/diff-viewer built on shiki's createHighlighterCore plus @shikijs/engine-javascript (WASM-free JS regex engine, grammars code-split per language), far leaner than the Monaco diff it replaces.
  • Fully themed with Bit's own design tokens: each token scope is painted a sentinel that resolves to a Bit design-system CSS var, so highlighting follows light/dark automatically with zero hardcoded syntax colors.
  • Split/unified, intra-line token-level diff, expand-context, soft-wrap. Wired into the Code tab via inline-code-compare (Monaco still backs the live code editor, untouched). Adds jsonc/json5 highlighting for config files.

Semantic API diff view

  • The API diff is now an on-demand view mode in the compare surface, gated so it only appears when there is something to show: real changes, unresolved exports, or a per-version schema gap.
  • Change blocks cluster member changes by severity (breaking, minor, patch); additions and removals render their actual signature as a syntax-highlighted snippet (collapsed by kind), modifications keep the token-level before/after, and documentation changes show the removed/added JSDoc alongside the declaration it belongs to.
  • Distinct, unambiguous states: "No API changes" (identical), "Schema unavailable" (one version could not be read, for example built before API extraction), "couldn't be analyzed" (extraction gaps), and a calm blank state when neither side exposes an API. A couldn't-compute is never dressed up as a verified no-op.

API diff correctness

  • Fixed a phantom "X (default) added to public API" on docs-only diffs: export default X was extracted inconsistently across builds (UnImplementedSchema versus a "X (default)" TypeRefSchema alias). The extractor now emits a stable default re-reference, and the diff is made robust to historical artifacts: it filters UnImplementedSchema exports, dedupes redundant re-export aliases, and surfaces genuinely-unresolved exports instead of diffing them.
  • Doc diffs now include JSDoc tags (@see, and so on), not just the description.

Base / compare resolution

  • Default base is the previous published version and compare is the live workspace (on-disk, including uncommitted), so the default compare shows what changed since the last version. Selecting a version in the main dropdown (?version) points the compare side at that clean version instead.
  • Server computeCompare diffs against on-disk files when the compare side is the checked-out workspace (compareIsLiveWorkspace); those mutable diffs are never cached.

Shared overviews

  • workspace-overview and lane-overview now consume one shared components-overview component instead of duplicating the grid/card logic.

luvkapur and others added 30 commits May 13, 2026 15:56
Defines scope, architecture, hook bindings, and acceptance criteria
(no-drawer, no-monaco, tab rewiring, manual smoke) for porting the
new lane-compare design into ____bit while keeping workspace data
sources.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
16-task plan with explicit scope/name/rootDir per new bit component
so resulting npm package ids match imports from the new design.
Phases A-F cover primitives, non-monaco diff foundation, inline tab
components, compare shell, lane-compare port (drawer stripped), and
final tab-list wiring + acceptance gates.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ports CompareToolbar from new-changes workspace; adds @teambit/design.inputs.selectors.multi-select@2.0.17 to workspace policy (was missing).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ports CompareSidebar (with SidebarGroup and SidebarComponentItem sub-components)
from the new-changes workspace into the component-compare component and exports
all public types from the index.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add teambit.dependencies/ui/deps-diff-table — a pure table component for rendering dependency diffs between two component versions, with status indicators and compare URL links.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds teambit.code/ui/inline-code-compare, which renders per-file inline diffs
using DiffFileRenderer from inline-diff-viewer and wires into ComponentCompare context.
Fixed import path (component.ui.component-compare.component-compare) and removed unused variable.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ports teambit.review/ui/inline-tests-compare from lane-compare branch; remaps bare component-compare import to component-compare.component-compare package id.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds teambit.review/ui/inline-config-compare — renders aspect config/data
diffs between two component versions using DiffFileRenderer.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eHeader

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Required by InlineComponentCompare (Task 12). bit install side-effects.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds teambit.semantics/ui/api-diff-view — a full-pane React component that queries and renders API surface changes between two component versions using GQL.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e data

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…hapes

- Drop `isMerged` references from lane-compare-page (LaneModel only has `deleted`)
- Adapt `useLaneComponents` call to single-arg signature returning an object
- Use `componentDescriptors` (not `components`) for the compositions map since `.get()` is a ComponentDescriptor method

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…hema

InlineContextProvider and EagerFileRegistrar used useGetComponents /
createComponentModel from the cloud GraphQL schema (getComponents /
CompDescriptor), which the workspace/scope dev-server schema does not
expose - every InlineComponentCompare fired a 400, breaking the code
diff and stalling the lane-diff query via a re-render storm.

Replace with the schema-compatible useComponent hook (spec section 7.1
fallback). Drop the now-unused cloud packages from workspace.jsonc.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extract workspace-overview's data-agnostic pieces into a shared
@teambit/explorer.ui.components-overview component; refactor both
workspace-overview and lane-overview into thin adapters.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… plan

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rview

Creates @teambit/explorer.ui.components-overview with data-agnostic card,
filter-panel, aggregation hooks, and a new ComponentsOverview orchestrator
that accepts getHref / storageNamespace injection points.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…onent

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wraps the header slot and filter command bar in a single sticky div,
measures its height via ResizeObserver into --components-overview-sticky-height,
and uses that CSS var as the top offset for section headers.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit d9fa440

Comment thread components/ui/inline-diff-viewer/inline-diff-viewer.tsx
Comment thread components/ui/inline-diff-viewer/inline-diff-viewer.tsx Outdated
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit a8ab799

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jul 7, 2026

Copy link
Copy Markdown

Code Review by Qodo

Grey Divider

New Review Started

This review has been superseded by a new analysis

Grey Divider

Qodo Logo

Comment thread scopes/lanes/lanes/lanes.main.runtime.ts
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 1234e2a

Comment thread components/ui/diff-viewer/highlighter.ts
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 56b4a56

Comment thread scopes/semantics/schema/api-diff-compute.spec.ts
Comment thread scopes/lanes/diff/lane-diff-generator.ts
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 7b46de6

# Conflicts:
#	scopes/workspace/workspace/ui/workspace/use-workspace.ts
Comment thread components/entities/semantic-schema-diff/api-diff.ts
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit d87a9d5

…e types

In the deferChanges path (default for UI lane compares) common snaps aren't
imported eagerly, so deriveChangeTypes() could fail to load the merge-base
Version and drop a genuinely-changed component from the diff. Import the
missing snap(s) on demand and retry once before giving up.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 85133fb

Comment on lines +84 to +113
const defaultLaneState = useCallback(
(compId?: string) => {
const sortedTabs = extractLazyLoadedData(tabs)?.sort(sortTabs);

const defaultState = (compId && defaultComponentCompareState?.get(compId)) || {};

const value: ComponentCompareState = {
tabs: {
controlled: true,
id: sortedTabs?.[0]?.id,
element: sortedTabs?.[0]?.element,
},
code: {
controlled: true,
},
aspects: {
controlled: true,
},
preview: {
controlled: true,
},
versionPicker: {
element: null,
},
...defaultState,
};
return value;
},
[defaultComponentCompareState]
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Stale default tab state 🐞 Bug ≡ Correctness

LaneCompareProvider’s defaultLaneState() reads tabs but its useCallback deps omit tabs, so
lazy-loaded/updated tab definitions won’t be reflected and lane compare panels can keep an
undefined/stale default tab selection.
Agent Prompt
### Issue description
`defaultLaneState` uses `tabs` (`extractLazyLoadedData(tabs)`) but `tabs` is not included in the `useCallback` dependency array. This can cause stale closures where the provider never picks up newly loaded/updated tabs and never re-initializes panel state accordingly.

### Issue Context
Tabs can be `MaybeLazyLoaded<TabItem[]>`, so they may resolve after initial render. With the current deps, `defaultLaneState` can keep using the initial (often empty) `tabs` value.

### Fix Focus Areas
- Update hook dependencies and state re-init triggers
  - components/ui/compare/lane-compare/lane-compare.provider.tsx[84-143]

### Suggested fix
- Include `tabs` (and any other referenced values like `sortTabs` if not stable) in `defaultLaneState`’s deps.
- Consider memoizing `sortedTabs` via `useMemo` and depend on that instead.
- Ensure the `useEffect` that rebuilds `laneCompareState` also re-runs when the effective tab list changes (e.g., include `tabs`/`sortedTabs`/`defaultLaneState` in deps).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread components/ui/inline-code-compare/inline-code-compare.tsx
Comment thread scopes/harmony/graphql/graphql.main.runtime.ts
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 7b5b858

Comment thread components/ui/diff-viewer/diff-model.ts
Comment thread scopes/harmony/graphql/graphql.main.runtime.ts
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 95ba129

The lane export failed because ui/inline-code-compare was a NEW component with
default-scope teambit.code, colliding with the existing teambit.code/ui/inline-code-compare
on main (unrelated histories → unmergeable). Re-track it as the existing component
(teambit.code/ui/inline-code-compare@0.0.1) so our implementation lands as a snap on top
of main's history and the lane exports cleanly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines +233 to +238
) {
this.loadSnapsDistanceMemoFromDisk();
this.loadApiDiffMemoFromDisk();
this.loadChangeTypesMemoFromDisk();
this.loadDiffStatusMemoFromDisk();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Async memo load race 🐞 Bug ☼ Reliability

LanesMain kicks off async disk-memo loads in its constructor without awaiting/guarding them, so
early diffStatus work can populate memo Maps and then be overwritten by stale on-disk entries when
the loaders resolve. This makes lane-diff results nondeterministic right after server start and can
serve stale snap-distance/change-type/diffStatus data until recomputed.
Agent Prompt
### Issue description
`LanesMain` starts async memo loads in the constructor and immediately serves requests. Because the loaders mutate the same `Map` instances after `await fs.readFile(...)`, they can overwrite entries written by live computations that happened before loading finished.

### Issue Context
This impacts `snapsDistanceMemo`, `changeTypesMemo`, and `diffStatusResultMemo` (and any other memo loaded similarly). Right after process start, first requests can compute results and store them, then the loader resolves and overwrites the same keys with stale values from disk.

### Fix Focus Areas
- scopes/lanes/lanes/lanes.main.runtime.ts[219-238]
- scopes/lanes/lanes/lanes.main.runtime.ts[290-301]

### Implementation direction
- Introduce a `private memosReady: Promise<void>` (or similar) initialized in the constructor as `this.memosReady = this.initMemos()`.
- `initMemos()` should `await Promise.all([loadSnapsDistanceMemoFromDisk(), loadApiDiffMemoFromDisk(), loadChangeTypesMemoFromDisk(), loadDiffStatusMemoFromDisk()])`.
- At the beginning of `diffStatus()` (and any other method that reads/writes these memos), `await this.memosReady`.
- Additionally (defense-in-depth), when applying loaded entries, prefer `if (!map.has(k)) map.set(k, v)` to avoid overwriting values computed since startup if a future refactor reintroduces concurrency.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines 269 to +279
async getAPIDiff(baseIdStr: string, compareIdStr: string): Promise<Record<string, any> | null> {
// never persist a result that can still change: `null` (snaps couldn't load), FAILED (schema
// retrieval threw) and NOT_BUILT (snap not yet built) must recompute next call; NO_EXTRACTOR/
// DISABLED are stable env properties and safe to cache (see `isApiDiffCacheable`).
// the `:v2` namespace keeps pre-availability-aware results from being served.
return this.getOrCompute(
this.apiDiffInflight,
`component-compare:api-diff:v2:${baseIdStr}|${compareIdStr}`,
() => this.computeAPIDiff(baseIdStr, compareIdStr),
(v) => v !== null && ComponentCompareMain.isApiDiffCacheable(v)
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Workspace api diff cached 🐞 Bug ≡ Correctness

ComponentCompareMain.getAPIDiff() now uses a persistent cache keyed only by `(baseIdStr,
compareIdStr)` but does not bypass persistent caching for live-workspace comparisons like
compare() does. This can persist a mutable API diff derived from on-disk workspace source (unbuilt
components) and later serve a stale API diff after edits under the same IDs.
Agent Prompt
### Issue description
`compare()` explicitly skips reading/writing the persistent cache when the comparison is against live workspace files (`comparesLiveWorkspace(...)`). `getAPIDiff()` does not, yet `SchemaMain.computeAPIDiff()` documents that unbuilt workspace components extract schema from live source and therefore must not be memoized.

### Issue Context
Because `getAPIDiff()` persists results (no TTL) keyed only by `(baseIdStr, compareIdStr)`, a workspace-derived API diff can be written once and then returned even after source changes, as long as the IDs remain the same.

### Fix Focus Areas
- scopes/component/component-compare/component-compare.main.runtime.ts[115-150]
- scopes/component/component-compare/component-compare.main.runtime.ts[269-290]

### Implementation direction
- Add a `skipPersistentCache` argument to the `getOrCompute(...)` call in `getAPIDiff()`, mirroring `compare()`:
  - `const skip = this.comparesLiveWorkspace(baseIdStr, compareIdStr);`
  - pass `skip` as the final `getOrCompute` parameter.
- Consider bumping the cache key namespace (e.g. `api-diff:v3`) if existing persisted entries might have been written for live-workspace cases.
- Optionally add an additional cacheable guard if the API-diff result object can encode “workspace/live” (but the simplest/most robust is to reuse `comparesLiveWorkspace`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 9e79e8c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants