-
Notifications
You must be signed in to change notification settings - Fork 951
fix: cascade lane updateDependents during local bit snap #10322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
davidfirst
wants to merge
23
commits into
master
Choose a base branch
from
fix-update-dependents-cascade-on-snap
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
c18b009
fix: cascade lane updateDependents during local bit snap
davidfirst 96b875e
fix: base cascaded updateDependents on main head, not on the prior snap
davidfirst e301f7f
Merge branch 'master' into fix-update-dependents-cascade-on-snap
davidfirst de511c9
docs: clarify why updateDependents are excluded from the auto-tag tri…
davidfirst 874148b
fix: re-snap lane.components that depend on new updateDependents in _…
davidfirst b478402
refactor: push updateDependents directly from export instead of rerou…
davidfirst 13d3a27
Merge branch 'master' into fix-update-dependents-cascade-on-snap
davidfirst 122fec1
fix(snapping): address copilot review — broaden cascade deps, soften …
davidfirst 215e815
refactor(snapping): use compact() and ComponentIdList.searchWithoutVe…
davidfirst 748c4cc
Merge branch 'master' into fix-update-dependents-cascade-on-snap
davidfirst 91ade16
fix(sources): preserve local updateDependents cascade on import via o…
davidfirst db7d945
Merge branch 'master' into fix-update-dependents-cascade-on-snap
davidfirst 40c396c
feat(snap,export): surface cascaded updateDependents as 'snapped upda…
davidfirst 198df65
Merge branch 'master' into fix-update-dependents-cascade-on-snap
davidfirst c9c2f7d
fix(reset): revert cascaded updateDependents via LaneHistory baseline
davidfirst 0f863ff
fix(sources.mergeLane): clear overrideUpdateDependents on the remote'…
davidfirst ac1845d
fix(reset): pick lane-history entry BEFORE earliest reset batch (by d…
davidfirst 0ac9686
Merge branch 'master' into fix-update-dependents-cascade-on-snap
davidfirst 977482d
refactor(snapping): fold cascaded updateDependents into auto-snapped …
davidfirst 33f95c6
perf: cap scope.get concurrency via pMapPool; precompute updateDepend…
davidfirst 77c83ab
fix(reset): always persist lane after applyUpdateDependentsFromHistor…
davidfirst ab3cba4
feat(merge): refresh lane.updateDependents when merging main into a lane
davidfirst 8362f76
fix(reset): load ModelComponent without version when dropping orphane…
davidfirst File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
142 changes: 142 additions & 0 deletions
142
scopes/component/snapping/include-lane-components-for-updep-snap.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| import type { ComponentID } from '@teambit/component-id'; | ||
| import { ComponentIdList } from '@teambit/component-id'; | ||
| import { compact } from 'lodash'; | ||
| import { pMapPool } from '@teambit/toolbox.promise.map-pool'; | ||
| import { concurrentComponentsLimit } from '@teambit/harmony.modules.concurrency'; | ||
| import type { Component } from '@teambit/component'; | ||
| import type { ScopeMain } from '@teambit/scope'; | ||
| import type { Logger } from '@teambit/logger'; | ||
| import type { Lane } from '@teambit/objects'; | ||
|
|
||
| export type LaneCompsForUpDepSnap = { | ||
| components: Component[]; | ||
| ids: ComponentIdList; | ||
| }; | ||
|
|
||
| /** | ||
| * When `bit _snap --update-dependents` runs in a bare scope, it creates new Version objects for | ||
| * the target components (they land in `lane.updateDependents`). Any entry in `lane.components` that | ||
| * depends on one of those targets now has a stale dep: its recorded version points to the target's | ||
| * pre-updateDependents version (typically the main tag), not the fresh `updateDependents` hash. | ||
| * | ||
| * This helper finds those affected `lane.components` and returns them as extra snap seeders so | ||
| * they can be re-snapped in the same `_snap --update-dependents` pass, with dep refs rewritten to | ||
| * the cascaded hashes by `updateDependenciesVersions`. They stay in `lane.components` (not moved | ||
| * to `updateDependents`) because they were already there — the caller distinguishes via the | ||
| * `updateDependentIds` VersionMakerParam. | ||
| * | ||
| * The dependency set expands via fixed-point iteration so transitive dependents on the lane are | ||
| * picked up too (edit X in updateDependents → Y depends on X → Z depends on Y; all three end up | ||
| * in the same snap pass with pre-assigned hashes, which also handles cycles). | ||
| */ | ||
| export async function findLaneComponentsDependingOnUpdDepTargets({ | ||
| lane, | ||
| targetIds, | ||
| scope, | ||
| logger, | ||
| }: { | ||
| lane: Lane; | ||
| targetIds: ComponentIdList; | ||
| scope: ScopeMain; | ||
| logger: Logger; | ||
| }): Promise<LaneCompsForUpDepSnap> { | ||
| const empty: LaneCompsForUpDepSnap = { components: [], ids: new ComponentIdList() }; | ||
| const laneComponents = lane.components; | ||
| if (!laneComponents.length || !targetIds.length) return empty; | ||
|
|
||
| const legacyScope = scope.legacyScope; | ||
|
|
||
| // Ensure every lane.components Version object is available locally — we need to inspect their | ||
| // recorded deps to decide who to pull in. | ||
| const laneCompIds = ComponentIdList.fromArray(laneComponents.map((c) => c.id.changeVersion(c.head.toString()))); | ||
| try { | ||
| await legacyScope.scopeImporter.importWithoutDeps(laneCompIds, { | ||
| cache: true, | ||
| lane, | ||
| reason: 'for finding lane.components that depend on the updateDependents being snapped', | ||
| }); | ||
| } catch (err: any) { | ||
| logger.debug(`findLaneComponentsDependingOnUpdDepTargets: failed to pre-fetch lane.components: ${err.message}`); | ||
| } | ||
|
|
||
| type LoadedEntry = { id: ComponentID; depIds: ComponentID[] }; | ||
| const loaded: LoadedEntry[] = []; | ||
| // Best-effort loading — the prefetch above swallows import errors, so a single corrupt or | ||
| // missing object shouldn't abort the whole `_snap --update-dependents` pass. | ||
| for (const laneComp of laneComponents) { | ||
| try { | ||
| const modelComponent = await legacyScope.getModelComponentIfExist(laneComp.id); | ||
| if (!modelComponent) continue; | ||
| const version = await modelComponent.loadVersion(laneComp.head.toString(), legacyScope.objects, false); | ||
| if (!version) { | ||
| logger.debug( | ||
| `findLaneComponentsDependingOnUpdDepTargets: Version object for ${laneComp.id.toString()}@${laneComp.head.toString()} is missing, skipping` | ||
| ); | ||
| continue; | ||
| } | ||
| // Match the full dependency set used by `updateDependenciesVersions` (runtime + dev + peer + | ||
| // extension) so a lane.component that depends on a target only via peer or extension deps is | ||
| // still pulled into the snap pass. | ||
| const depIds = version.getAllDependenciesIds(); | ||
| loaded.push({ id: laneComp.id.changeVersion(laneComp.head.toString()), depIds }); | ||
| } catch (err: any) { | ||
| logger.debug( | ||
| `findLaneComponentsDependingOnUpdDepTargets: failed to load ${laneComp.id.toString()}@${laneComp.head.toString()}: ${err.message}, skipping` | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| if (!loaded.length) return empty; | ||
|
|
||
| const includeSet = new Set<string>(targetIds.map((id) => id.toStringWithoutVersion())); | ||
| let changed = true; | ||
| while (changed) { | ||
| changed = false; | ||
| for (const entry of loaded) { | ||
| const key = entry.id.toStringWithoutVersion(); | ||
| if (includeSet.has(key)) continue; | ||
| const matches = entry.depIds.some((depId) => includeSet.has(depId.toStringWithoutVersion())); | ||
| if (matches) { | ||
| includeSet.add(key); | ||
| changed = true; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const toInclude = loaded.filter((entry) => { | ||
| const key = entry.id.toStringWithoutVersion(); | ||
| if (!includeSet.has(key)) return false; | ||
| // the targets themselves are already in the seed set handled by the caller; don't double-add. | ||
| if (targetIds.searchWithoutVersion(entry.id)) return false; | ||
| return true; | ||
| }); | ||
| if (!toInclude.length) return empty; | ||
|
|
||
| // Cap concurrency: on large lanes the fixed-point expansion can pull in many dependents, and an | ||
| // unbounded Promise.all of scope.get() calls would spike memory/IO. | ||
| const loadedComps = await pMapPool( | ||
| toInclude, | ||
| async (entry) => { | ||
| try { | ||
| const comp = await scope.get(entry.id); | ||
| if (!comp) { | ||
| logger.debug( | ||
| `findLaneComponentsDependingOnUpdDepTargets: unable to load ${entry.id.toString()} from scope, skipping` | ||
| ); | ||
| return undefined; | ||
| } | ||
| return comp; | ||
| } catch (err: any) { | ||
| logger.debug( | ||
| `findLaneComponentsDependingOnUpdDepTargets: failed to load ${entry.id.toString()} from scope: ${err.message}, skipping` | ||
| ); | ||
| return undefined; | ||
| } | ||
| }, | ||
| { concurrency: concurrentComponentsLimit() } | ||
| ); | ||
| const components = compact(loadedComps); | ||
| if (!components.length) return empty; | ||
| const ids = ComponentIdList.fromArray(components.map((c) => c.id)); | ||
| return { components, ids }; | ||
| } |
132 changes: 132 additions & 0 deletions
132
scopes/component/snapping/include-update-dependents-in-snap.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| import type { ComponentID } from '@teambit/component-id'; | ||
| import { ComponentIdList } from '@teambit/component-id'; | ||
| import type { Component } from '@teambit/component'; | ||
| import type { ScopeMain } from '@teambit/scope'; | ||
| import type { Logger } from '@teambit/logger'; | ||
| import type { Lane } from '@teambit/objects'; | ||
|
|
||
| export type UpdateDependentsForSnap = { | ||
| components: Component[]; | ||
| ids: ComponentIdList; | ||
| }; | ||
|
|
||
| /** | ||
| * When snapping on a lane that has `updateDependents`, fold the relevant entries into the same | ||
| * snap pass so they land with correct dependency hashes on the first try. Returns the components | ||
| * to include as extra snap seeders along with their ids; the caller appends them to the main | ||
| * snap's seeds before handing things off to the VersionMaker. | ||
| * | ||
| * Without this, `updateDependents` go stale as soon as any lane component they depend on is | ||
| * re-snapped locally, because the old entries keep pointing at Version objects whose recorded | ||
| * dependencies reference outdated lane-component hashes. Re-snapping them in the same pass (rather | ||
| * than in a separate post-step) avoids producing two Version objects per cascade and keeps the | ||
| * number of downstream Ripple CI builds at one per component, just like a normal snap. | ||
| * | ||
| * *** Why we base the cascade on main head (and ignore the current updateDependents snap) *** | ||
| * The prior updateDependents entry points to an older snap (the one the "snap updates" button | ||
| * produced last time). Parenting the new cascade off that old snap would drift the lane off main: | ||
| * say A was 0.0.1 when first seeded into updateDependents, but main has since moved to 0.0.2 — | ||
| * using the old snap as the parent means the new cascade's history never includes A@0.0.2, and | ||
| * divergence calculations get messy. Instead, we always start from A's current main head. The new | ||
| * updateDependents snap is a direct descendant of main, one commit ahead, with deps rewritten to | ||
| * the lane versions. Any previously cascaded snap on the lane is simply orphaned — that's fine; | ||
| * the lane only ever points at the latest. | ||
| * | ||
| * *** Why the cascade set is a fixed-point expansion *** | ||
| * Starting from the ids the user is snapping, we add any updateDependent whose recorded | ||
| * dependencies (at main head) reference an id already in the set, then repeat. This handles | ||
| * transitive cascades (A -> B -> C, edit C → A and B cascade) and cycles (A -> B -> C -> A) | ||
| * because hashes are pre-assigned by `setHashes()` before deps are rewritten. | ||
| */ | ||
| export async function includeUpdateDependentsInSnap({ | ||
| lane, | ||
| snapIds, | ||
| scope, | ||
| logger, | ||
| }: { | ||
| lane?: Lane; | ||
| snapIds: ComponentIdList; | ||
| scope: ScopeMain; | ||
| logger: Logger; | ||
| }): Promise<UpdateDependentsForSnap> { | ||
| const empty: UpdateDependentsForSnap = { components: [], ids: new ComponentIdList() }; | ||
| if (!lane) return empty; | ||
| const updateDependents = lane.updateDependents; | ||
| if (!updateDependents || !updateDependents.length) return empty; | ||
|
|
||
| const legacyScope = scope.legacyScope; | ||
|
|
||
| // Fetch each updateDependent from main (no lane context) so the loaded ConsumerComponent | ||
| // carries main's head as its version. This flows through `setNewVersion` into | ||
| // `previouslyUsedVersion` and ultimately becomes the parent of the new cascaded snap — keeping | ||
| // it a direct descendant of main rather than of an earlier (now-orphaned) updateDependents snap. | ||
| const mainIds = ComponentIdList.fromArray(updateDependents.map((id) => id.changeVersion(undefined))); | ||
| try { | ||
| await scope.import(mainIds, { | ||
| preferDependencyGraph: false, | ||
| reason: 'for cascading updateDependents in the local snap (using main head as the base)', | ||
| }); | ||
| } catch (err: any) { | ||
| logger.debug(`includeUpdateDependentsInSnap: failed to pre-fetch main head for updateDependents: ${err.message}`); | ||
| } | ||
|
|
||
| type LoadedEntry = { id: ComponentID; depIds: ComponentID[]; component: Component }; | ||
| const loaded: LoadedEntry[] = []; | ||
| // Every step below is best-effort: the prefetch above swallows import errors, so a missing or | ||
| // corrupt local object shouldn't convert into a hard snap crash here. Log and skip instead. | ||
| for (const updDepId of updateDependents) { | ||
| try { | ||
| const idWithoutVersion = updDepId.changeVersion(undefined); | ||
| const modelComponent = await legacyScope.getModelComponentIfExist(idWithoutVersion); | ||
| if (!modelComponent || !modelComponent.head) { | ||
| logger.debug(`includeUpdateDependentsInSnap: ${updDepId.toString()} has no main head locally, skipping`); | ||
| continue; | ||
| } | ||
| const mainHeadStr = modelComponent.getTagOfRefIfExists(modelComponent.head) || modelComponent.head.toString(); | ||
| const idAtMainHead = idWithoutVersion.changeVersion(mainHeadStr); | ||
| const component = await scope.get(idAtMainHead); | ||
| if (!component) { | ||
| logger.debug(`includeUpdateDependentsInSnap: unable to load ${idAtMainHead.toString()} from scope, skipping`); | ||
| continue; | ||
| } | ||
| const consumerComp = component.state._consumer; | ||
| // Mirror `updateDependenciesVersions`, which rewrites deps across runtime, dev, peer and | ||
| // extension dependencies. If we skip any of those here, a component that depends on a snapped | ||
| // id only through (say) a peerDependency would escape the cascade set and stay stale. | ||
| const depIds = consumerComp.getAllDependencies().map((d) => d.id); | ||
| loaded.push({ id: component.id, depIds, component }); | ||
| } catch (err: any) { | ||
| logger.debug(`includeUpdateDependentsInSnap: failed to load ${updDepId.toString()}: ${err.message}, skipping`); | ||
| } | ||
| } | ||
|
|
||
| if (!loaded.length) return empty; | ||
|
|
||
| const cascadeSet = new Set<string>(snapIds.map((id) => id.toStringWithoutVersion())); | ||
| let changed = true; | ||
| while (changed) { | ||
| changed = false; | ||
| for (const entry of loaded) { | ||
| const key = entry.id.toStringWithoutVersion(); | ||
| if (cascadeSet.has(key)) continue; | ||
| const matches = entry.depIds.some((depId) => cascadeSet.has(depId.toStringWithoutVersion())); | ||
| if (matches) { | ||
| cascadeSet.add(key); | ||
| changed = true; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const toInclude = loaded.filter((entry) => { | ||
| const key = entry.id.toStringWithoutVersion(); | ||
| if (!cascadeSet.has(key)) return false; | ||
| // don't include anything the user is already snapping directly. | ||
| if (snapIds.searchWithoutVersion(entry.id)) return false; | ||
| return true; | ||
| }); | ||
| if (!toInclude.length) return empty; | ||
|
|
||
| const components = toInclude.map((entry) => entry.component); | ||
| const ids = ComponentIdList.fromArray(components.map((c) => c.id)); | ||
| return { components, ids }; | ||
| } | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.