-
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 1 commit
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
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
122 changes: 122 additions & 0 deletions
122
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,122 @@ | ||
| 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. | ||
| * | ||
| * The `cascade set` is computed by a fixed-point expansion: start with the ids the user is | ||
| * snapping, then repeatedly add any updateDependent whose recorded dependencies reference an id | ||
| * already in the set. 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; | ||
| const updateDependentsIdList = ComponentIdList.fromArray(updateDependents); | ||
|
|
||
| try { | ||
| await legacyScope.scopeImporter.importWithoutDeps(updateDependentsIdList, { | ||
| cache: true, | ||
| includeUpdateDependents: true, | ||
| // VersionHistory is needed so that `getDivergeData` can resolve the remote head during | ||
| // export. Without it, `bit snap` succeeds locally but `bit export` fails with | ||
| // "TargetHeadNotFound" because the old updateDependents hash isn't reachable through | ||
| // VersionHistory in workspaces that never imported the updateDependent's objects. | ||
| includeVersionHistory: true, | ||
| lane, | ||
| reason: 'for including updateDependents in the local snap', | ||
| }); | ||
| } catch (err: any) { | ||
| logger.debug(`includeUpdateDependentsInSnap: failed to pre-fetch updateDependents Version objects: ${err.message}`); | ||
| } | ||
|
|
||
| const loaded: Array<{ id: ComponentID; depIds: ComponentID[]; component?: Component }> = []; | ||
| for (const updDepId of updateDependents) { | ||
| const oldHash = updDepId.version; | ||
| if (!oldHash) continue; | ||
| const modelComponent = await legacyScope.getModelComponent(updDepId); | ||
| const version = await modelComponent.loadVersion(oldHash, legacyScope.objects, false); | ||
| if (!version) { | ||
| logger.debug( | ||
| `includeUpdateDependentsInSnap: Version object for ${updDepId.toString()} is missing locally, skipping` | ||
| ); | ||
| continue; | ||
| } | ||
| const depIds = [...version.dependencies.get().map((d) => d.id), ...version.devDependencies.get().map((d) => d.id)]; | ||
|
davidfirst marked this conversation as resolved.
Outdated
|
||
| loaded.push({ id: updDepId, depIds }); | ||
|
davidfirst marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| if (!loaded.length) return empty; | ||
|
|
||
| // Fixed-point expansion: add every updateDependent whose deps point at an id already in the | ||
| // cascade set. Repeat until nothing new is added. This handles transitive and cyclic cases. | ||
| 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 = await Promise.all( | ||
| toInclude.map(async (entry) => { | ||
| const comp = await scope.get(entry.id); | ||
| if (!comp) { | ||
| throw new Error(`includeUpdateDependentsInSnap: unable to load component ${entry.id.toString()} from scope`); | ||
| } | ||
| return comp; | ||
| }) | ||
| ); | ||
|
|
||
|
davidfirst marked this conversation as resolved.
Outdated
|
||
| const ids = ComponentIdList.fromArray(components.map((c) => c.id)); | ||
| return { components, ids }; | ||
| } | ||
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
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
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.