feat(lanes | component): redesign the changes & compare experience#10378
Open
luvkapur wants to merge 131 commits into
Open
feat(lanes | component): redesign the changes & compare experience#10378luvkapur wants to merge 131 commits into
luvkapur wants to merge 131 commits into
Conversation
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>
…eview to public teambit.code These three new components defaulted to the private teambit.review scope, so exporting them to the public changes-redesign lane failed with "can't export private components to public lane". Re-home them alongside the other diff/compare components (diff-viewer, inline-diff-viewer, inline-code-compare, code-compare) in the public teambit.code scope. Verified: no same-named components exist on teambit.code, so no collision. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
|
Code review by qodo was updated up to the latest commit 78f523b |
|
Code review by qodo was updated up to the latest commit 5ea49d3 |
|
Code review by qodo was updated up to the latest commit fa58452 |
|
Code review by qodo was updated up to the latest commit 6529a67 |
|
Code review by qodo was updated up to the latest commit 6b60ef6 |
…ed keyword types
A function without an explicit return annotation whose inferred type is a
keyword (e.g. `any` when React types fail to resolve) went through resolveType's
definition-hunt, which resolved the declaration's own name and stored the
function itself as its return type (`function Checkbox(...): any` extracted with
returnType TypeRefSchema('Checkbox')) — and duplicated the visited declaration
into the internal modules.
- extractor: skip the definition hunt for keyword type strings, and treat a
definition span contained in the node's own span as a self-reference
- diff engine: when a return-type node renders to the function's own name but
the signature's return annotation disagrees, trust the signature — already
built (poisoned) artifacts cannot be re-extracted
- bump the api-diff cache namespace to v3 so stale computed diffs recompute
- regenerate the button-schemas golden (3 self-refs corrected, duplicate
internal entries gone) and add spec coverage for both defense paths
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…groups
- run each staggered lazy mount inside React.startTransition so the heavy
section render is time-sliced and urgent input (view-mode clicks, search)
preempts it — switching tabs mid-load read as frozen
- tokenize files over 20k chars in requestIdleCallback instead of inside the
mount render: whole-file shiki tokenizing was a non-interruptible chunk;
large diffs now paint plain text first and colorize from an idle slot
- fix empty scope groups rendering as stray "0" headers: the bare
.groupHidden { display: none } was defeated by the later
.laneCompareGroup { display: grid } rule (same specificity, file order
wins) — use a compound selector so the hide always applies
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…and paging - tri-state change classification: preserve the server's "couldn't classify" (changes: null) through the lane-diff entity and UI instead of collapsing it to NONE — an unclassified component stays visible in every view and blocks the "no changes" blank state, so a transient derivation failure can no longer present as a verified all-clear - importMainHeads: catch per-component head-resolution failures so the best-effort prefetch can't reject and abort the whole lane diff - getAPIDiff: skip the persistent cache when either side is the live checkout (mirrors compare()), so a cached snap-to-snap diff can't mask on-disk API changes of a modified checkout - bound the compare/api-diff persistent cache with a 14-day TTL — payloads embed full per-file contents, so entries otherwise accumulate forever - useBulkPagedQuery: reset paging state on the full session identity (host/skip/pageSize, not just pairs); recover in updateQuery when the accumulated result has no host but the new page does; memoize the pairs stringify on reference identity - highlighter: don't memoize a rejected init promise — a transient failure permanently disabled highlighting for the session - format the default-export mock tsconfig with prettier Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Code review by qodo was updated up to the latest commit 8cb780c |
davidfirst
reviewed
Jul 17, 2026
…e classification Human review (davidfirst) + Qodo findings on the lane-diff work: - extract the disk-persisted memo layer out of the ever-growing LanesMain into a dedicated LaneDiffCache class in lanes/diff (David: "lots of new methods, all the same thing — maybe a new class in lanes/diff") - fix cache invalidation: the top-level result key now fingerprints the lane COMPOSITION (sorted component heads) instead of Lane.hash(). Lane hash is a random uuid minted once at creation and never moves as the lane advances, so the prior key served stale results forever (David flagged exactly this). Covered by lane-diff-cache.spec.ts. - dependency-only changes no longer also emit ChangeType.ASPECTS, so dep-only components stop appearing under the Config view with an empty panel (Qodo). Change classification extracted to a pure classifyVersionChanges() with a regression spec proving dep-only => [DEPENDENCY], never ASPECTS. - resolve-main-head: use the ScopeMain API (getBitObjectModelComponent) instead of the legacy scope, and drop the redundant scope-existence guard since a ComponentID always carries a scope (both David comments). - mark the inline-config compare tab lazy so its two no-cache aspect queries don't fire for CSS-hidden panels while browsing other views (Qodo). The config sidebar tree is unaffected (fed centrally by RegistryFeeder). - bump change-types/diff-status memo versions to invalidate entries written under the old dep-as-config classification. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Code review by qodo was updated up to the latest commit 26de86c |
…ottle scheduleSave() returned early when a timer already existed, so a sustained burst of store*() calls (once per component during a large-lane diff) wrote roughly every flush-window instead of once after the burst — contradicting the method's documented "500ms after the last store" debounce contract and adding avoidable full-map serializations on a hot path. Reset the timer on each call (clear + re-arm) so the flush lands after the LAST store. The flush interval is now an injectable ctor param (defaults to 500, production unchanged) so the debounce is deterministically testable — added two regression tests asserting a 10-store burst collapses to a single write that captures the whole burst, and that distinct memos each flush once. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
davidfirst
approved these changes
Jul 17, 2026
|
Code review by qodo was updated up to the latest commit 7d630b1 |
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
2 participants
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.
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
InlineComponentCompareis 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.data-view-modeattribute plus CSS, so a mode click never remounts or re-fetches (fixes the "switching is slow" and console-error regressions).DiffModeProvider/useDiffModegives a single global split/unified toggle across every file instead of a per-file one.Comparing a lane against main
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".skipUpToDatewas 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, thediffStatusresult-memo key was derived fromLane.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.getHeadOnMain, so "merge from main" no longer drops components without a local main head.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
CompareDataProviderplus a bulkcompareComponents(pairs)GraphQL query fetch code/aspect/test diffs for all component pairs in one paginated call, keyed bycompareId; inline panels read from this context instead of firing per-file queries.FileRegistrycontext feeds the sidebar file tree from the same bulk result.changesderivation dropped from about 47s to about 8s: diff rawVersionobjects 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 viauseSyncExternalStoreplus a version-guarded notify.New code diff viewer (no Monaco)
teambit.code/ui/diff-viewerbuilt on shiki'screateHighlighterCoreplus@shikijs/engine-javascript(WASM-free JS regex engine, grammars code-split per language), far leaner than the Monaco diff it replaces.inline-code-compare(Monaco still backs the live code editor, untouched). Addsjsonc/json5highlighting for config files.Semantic API diff view
API diff correctness
X (default)added to public API" on docs-only diffs:export default Xwas extracted inconsistently across builds (UnImplementedSchemaversus a"X (default)"TypeRefSchemaalias). The extractor now emits a stabledefaultre-reference, and the diff is made robust to historical artifacts: it filtersUnImplementedSchemaexports, dedupes redundant re-export aliases, and surfaces genuinely-unresolved exports instead of diffing them.@see, and so on), not just the description.Base / compare resolution
?version) points the compare side at that clean version instead.computeComparediffs against on-disk files when the compare side is the checked-out workspace (compareIsLiveWorkspace); those mutable diffs are never cached.Shared overviews
workspace-overviewandlane-overviewnow consume one sharedcomponents-overviewcomponent instead of duplicating the grid/card logic.