feat(install): add --restore flag to rebuild the lockfile from component graphs#10325
Open
zkochan wants to merge 4 commits intoteambit:masterfrom
Open
feat(install): add --restore flag to rebuild the lockfile from component graphs#10325zkochan wants to merge 4 commits intoteambit:masterfrom
zkochan wants to merge 4 commits intoteambit:masterfrom
Conversation
…ent graphs bit install does not normally use the per-component dependency graphs stored on each Version object. If a user deletes pnpm-lock.yaml and runs bit install, pnpm re-resolves every dep from the manifest specifiers and drifts to whatever the registry now considers latest — losing the tag-time version pins that the graph feature is supposed to guarantee. --restore opts into the same code path bit import uses: fetch the dep graph for every component in the bitmap, merge them, and hand the merged graph to the package manager, which then seeds pnpm-lock.yaml from it. If no workspace component has a stored graph, we fall back to a regular install with a warning. Adds e2e coverage that deletes the lockfile + node_modules, bumps the registry so fresh resolve would drift, runs bit install --restore, and asserts the components stay locked to their graph versions.
zkochan
force-pushed
the
install-restore-flag
branch
from
05f062c to
0a03288
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new bit install --restore mode intended to rebuild pnpm-lock.yaml from the dependency graphs stored on component Versions, preventing lockfile drift when the lockfile is deleted and dependencies would otherwise be re-resolved from manifest ranges.
Changes:
- Added
--restoreCLI flag and threaded it into workspace install options (restoreFromDependenciesGraph). - Implemented
resolveDependenciesGraph()to pick between an explicitly provided graph, a restored merged graph from workspace components, or no graph (with a warning). - Added an e2e scenario verifying
bit install --restorekeeps versions pinned to the stored graphs after the registry “latest” dist-tag is bumped.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
scopes/workspace/install/install.cmd.tsx |
Exposes --restore and passes it into WorkspaceInstallOptions.restoreFromDependenciesGraph. |
scopes/workspace/install/install.main.runtime.ts |
Adds restore option + graph-resolution helper and wires the resolved graph into package-manager install options. |
e2e/harmony/deps-graph.e2e.ts |
Adds e2e coverage validating lockfile reconstruction from stored dependency graphs via bit install --restore. |
…oggle
The flag's purpose is to gate the *providers* that fetch and attach dep graphs:
tag-time generation on version-maker, and import-time fetching in the component
writer. Once a graph is explicitly supplied to the package manager, there's no
reason to also gate the *consumer*. --restore is an explicit user opt-in, so we
shouldn't silently ignore it because the workspace didn't flip BIT_FEATURES.
- Scope.getDependenciesGraphByComponentIds (both the main-runtime wrapper and
the legacy implementation) take an { ignoreFeatureToggle } option that skips
the isFeatureEnabled(DEPS_GRAPH) short-circuit.
- resolveDependenciesGraph passes it when options.restoreFromDependenciesGraph
is true.
- PnpmPackageManager.install drops the isFeatureEnabled(DEPS_GRAPH) check from
its graph-to-lockfile branch. The caller already decided to seed from a graph
by supplying it; the flag no longer needs to second-guess that.
Adds an e2e case that tags with the flag on, then resets features before import
and --restore, and asserts the lockfile is still restored from the stored graph
despite the consumer running with the flag off.
Copilot review caught that PnpmPackageManager.install only seeds the lockfile from a supplied graph when rootComponents (or rootComponentsForCapsules) is true. Without that, --restore would silently no-op and re-resolve from manifest specifiers. Surface the incompatibility up front: log a yellow warning from resolveDependenciesGraph and return undefined, so the command behaves as if --restore wasn't passed instead of pretending it worked. hasRootComponents is computed in _installModules; threaded in as context so the resolver doesn't have to re-read it.
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
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.
Summary
bit installdoes not normally use the per-component dependency graphs stored on eachVersionobject. If a user deletespnpm-lock.yamland runsbit install, pnpm re-resolves every dep from manifest specifiers and drifts to whatever the registry now considers latest — losing the tag-time version pins the graph feature is supposed to guarantee.--restoreopts into the same code pathbit importuses: fetch the dep graph for every component in the bitmap, merge them, and hand the merged graph to the package manager, which seedspnpm-lock.yamlfrom it. If no workspace component has a stored graph, we fall back to a regular install with a warning.--restoreis an explicit user opt-in, so it bypasses theDEPS_GRAPHfeature toggle. The flag's purpose is to gate the providers that fetch and attach graphs (tag-time generation; import-time auto-restore). Once a graph is explicitly supplied to the package manager, there's no reason to also gate the consumer.Changes
scopes/workspace/install/install.cmd.tsxNew
--restoreflag, threaded through toWorkspaceInstallOptions.restoreFromDependenciesGraph.scopes/workspace/install/install.main.runtime.tsWorkspaceInstallOptionsgetsrestoreFromDependenciesGraph.resolveDependenciesGraphhelper centralizes the choice between an explicitoptions.dependenciesGraph, the restore path (workspace.scope.getDependenciesGraphByComponentIds(workspace.listIds(), { ignoreFeatureToggle: true })), and no graph. Extracted as a method because inlining pushed_installModulesover the complexity limit.scopes/scope/scope/scope.main.runtime.tsandcomponents/legacy/scope/scope.tsgetDependenciesGraphByComponentIdstakes an optional{ ignoreFeatureToggle }flag that skips theDEPS_GRAPHshort-circuit. Used only from the--restorepath today.scopes/dependencies/pnpm/pnpm.package-manager.tsRemoved the
isFeatureEnabled(DEPS_GRAPH)check from the graph-to-lockfile branch. IfinstallOptions.dependenciesGraphis set, the caller already decided to seed from it.e2e/harmony/deps-graph.e2e.tsTwo e2e scenarios:
--restorewith the feature flag on: delete lockfile + node_modules after an import, bump the registry, runbit install --restore, and assert both components stay locked to their graph versions.--restorewith the feature flag off: tag with the flag on so the graphs are stored, then reset features before import +--restore, and assert the lockfile is still restored from the stored graph.Test plan
npm run lintclean.bd(dev-linked bit).