fix: Missing Packages After bit new with Forked Env#10201
fix: Missing Packages After bit new with Forked Env#10201zkochan wants to merge 12 commits intoteambit:masterfrom
Conversation
…egression) Reproduces the chicken-and-egg problem where forking an env with "+" dep-resolver entries into a fresh workspace causes MissingManuallyConfiguredPackages because the usedPeerDependencies filter excludes packages that aren't yet installed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Two fixes for the chicken-and-egg problem where "+" dependency markers on a forked env couldn't resolve in a fresh workspace: 1. workspace-manifest-factory.ts: widen usedPeerDependencies filter to include packages explicitly listed in the component's dep-resolver config, so pnpm will install them even before they appear in the resolved dependency manifest. 2. apply-overrides.ts: in getDependenciesToAddManually(), when _manuallyAddPackage() can't resolve "+" from workspace package.json, check if the package was already resolved by applyAutoDetectedPeersFromEnvOnEnvItSelf() and remove it from missingPackageDependencies to prevent a false MissingManuallyConfiguredPackages issue. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes a regression where a fresh workspace created via bit new / bit fork with a forked env can end up with manually-configured "+" dependencies never getting installed (and repeatedly reported as missing), by ensuring those deps make it into the generated manifests and by reducing a related false-positive missing-deps report.
Changes:
- Include env peer-deps in
usedPeerDependencieswhen the package is explicitly present in the component’s dep-resolver policy (to break the"+"chicken-and-egg cycle). - Avoid reporting
MissingManuallyConfiguredPackageswhen a+sentinel can’t be resolved but the package is already present in resolved package dependencies. - Add an e2e regression test covering the forked-env
+marker scenario.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
scopes/dependencies/dependency-resolver/manifest/workspace-manifest-factory.ts |
Expands the usedPeerDependencies filter to also consider explicitly-configured dep-resolver policy packages. |
scopes/dependencies/dependencies/dependencies-loader/apply-overrides.ts |
Removes a dependency from missingPackageDependencies when it’s already present in resolved package deps. |
e2e/harmony/dependencies/forked-env-missing-deps.e2e.ts |
Adds regression coverage for the forked-env + dependency marker installation cycle. |
scopes/dependencies/dependency-resolver/manifest/workspace-manifest-factory.ts
Outdated
Show resolved
Hide resolved
scopes/dependencies/dependencies/dependencies-loader/apply-overrides.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
scopes/dependencies/dependencies/dependencies-loader/apply-overrides.ts
Outdated
Show resolved
Hide resolved
scopes/dependencies/dependency-resolver/manifest/workspace-manifest-factory.ts
Outdated
Show resolved
Hide resolved
scopes/dependencies/dependency-resolver/manifest/workspace-manifest-factory.ts
Outdated
Show resolved
Hide resolved
| private getComponentExplicitPackages(component: Component): Set<string> { | ||
| const depResolverEntry = component.get(DependencyResolverAspect.id); | ||
| const explicitPolicy = depResolverEntry?.config?.policy ?? {}; | ||
| return new Set<string>([ | ||
| ...nonRemovedEntryNames(explicitPolicy.dependencies), | ||
| ...nonRemovedEntryNames(explicitPolicy.devDependencies), | ||
| ...nonRemovedEntryNames(explicitPolicy.peerDependencies), | ||
| ]); | ||
|
|
||
| function nonRemovedEntryNames(policySection?: Record<string, VariantPolicyConfigEntryValue>): string[] { | ||
| if (!policySection) return []; | ||
| const names: string[] = []; | ||
| for (const [name, versionSpec] of Object.entries(policySection)) { | ||
| // Skip explicit removals expressed as "-" or as removal objects. | ||
| if (versionSpec !== '-' && !isRemovalObject(versionSpec)) { |
There was a problem hiding this comment.
In getComponentExplicitPackages(), the helper functions (nonRemovedEntryNames / isRemovalObject) are declared after the return statement. Although function declarations are hoisted, this layout is easy to misread as unreachable/dead code and can confuse future maintenance. Consider moving the helper function declarations above the return, or extracting them to private static methods outside this function.
| return names; | ||
| } | ||
|
|
||
| function isRemovalObject(val: VariantPolicyConfigEntryValue): val is VariantPolicyEntryValue { |
There was a problem hiding this comment.
isRemovalObject() is typed as a type guard returning val is VariantPolicyEntryValue, but it checks val.version === '-'. VariantPolicyEntryValue.version is typed as a semver string, so this guard is semantically inconsistent and may mislead TypeScript narrowing. Consider changing the guard to match the actual removal-object shape (e.g. val is { version: '-' }) or avoid the type predicate and just do a runtime check for a version field equal to '-'.
| function isRemovalObject(val: VariantPolicyConfigEntryValue): val is VariantPolicyEntryValue { | |
| function isRemovalObject(val: VariantPolicyConfigEntryValue): boolean { |
close #10200
Two fixes for the chicken-and-egg problem where "+" dependency markers on a forked env couldn't resolve in a fresh workspace:
workspace-manifest-factory.ts: widenusedPeerDependenciesfilter to include packages explicitly listed in the component's dep-resolver config, so pnpm will install them even before they appear in the resolved dependency manifest.apply-overrides.ts: ingetDependenciesToAddManually(), when_manuallyAddPackage()can't resolve"+"from workspacepackage.json, check if the package was already resolved byapplyAutoDetectedPeersFromEnvOnEnvItSelf()and remove it frommissingPackageDependenciesto prevent a falseMissingManuallyConfiguredPackagesissue.Proposed Changes