Skip to content

Commit e99f3a7

Browse files
zkochanclaudeCopilotGiladShoham
authored
fix: Missing Packages After bit new with Forked Env (#10201)
close #10200 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. ## Proposed Changes - - - --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Gilad Shoham <shoham.gilad@gmail.com>
1 parent 8ec6bc7 commit e99f3a7

5 files changed

Lines changed: 157 additions & 2 deletions

File tree

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
import { IssuesClasses } from '@teambit/component-issues';
2+
import { Helper } from '@teambit/legacy.e2e-helper';
3+
4+
/**
5+
* Regression test for PR #10150.
6+
*
7+
* When `bit new` (or `bit fork`) creates a workspace with a forked env, the
8+
* forking process copies the dep-resolver aspect config verbatim from the
9+
* source version. If that config contains entries with the "+" sentinel
10+
* (MANUALLY_ADD_DEPENDENCY — meaning "resolve version from the workspace
11+
* package.json"), a chicken-and-egg problem arises on a fresh workspace:
12+
*
13+
* 1. _manuallyAddPackage() tries to resolve "+" from the workspace
14+
* root package.json, but the package is not yet installed → null →
15+
* pushed to missingPackageDependencies (not to packageDependencies).
16+
* 2. The package is therefore absent from depManifestBeforeFiltering.
17+
* 3. The usedPeerDependencies filter (introduced by PR #10150) checks
18+
* depManifestBeforeFiltering and excludes the package.
19+
* 4. pnpm never installs it.
20+
* 5. Every subsequent `bit status` / `bit install` repeats the cycle.
21+
*
22+
* Before PR #10150, `defaultPeerDependencies` was spread unconditionally,
23+
* which broke the cycle on the first install.
24+
*/
25+
describe('forked env with "+" dependency markers (PR #10150 regression)', function () {
26+
this.timeout(0);
27+
let helper: Helper;
28+
29+
before(() => {
30+
helper = new Helper();
31+
});
32+
33+
after(() => {
34+
helper.scopeHelper.destroy();
35+
});
36+
37+
describe('fork an env whose dep-resolver config has "+" entries for env peer packages', () => {
38+
const envName = 'react-based-env';
39+
40+
before(() => {
41+
// ── Source workspace: create the env, tag, and export ──────────────
42+
helper.scopeHelper.setWorkspaceWithRemoteScope();
43+
44+
// Create a custom env with env.jsonc that declares is-positive as a
45+
// peer dependency. For the env component *itself*, this peer entry
46+
// becomes part of the selfPolicy (via getPoliciesFromEnvForItself),
47+
// which feeds into _getDefaultPeerDependencies().
48+
const envPolicy = {
49+
peers: [{ name: 'is-positive', version: '3.1.0', supportedRange: '^3.0.0' }],
50+
};
51+
helper.env.setCustomNewEnv(undefined, undefined, { policy: envPolicy }, true);
52+
53+
// Make sure is-positive is present in the workspace so the "+" marker
54+
// can be resolved during tagging in this (source) workspace.
55+
helper.command.install('is-positive@3.1.0');
56+
57+
// Simulate the original env having an explicit "+" dep-resolver entry
58+
// for is-positive. This is the config that gets copied verbatim when
59+
// the component is forked.
60+
const bitmap = helper.bitMap.read();
61+
bitmap[envName] = bitmap[envName] || {};
62+
bitmap[envName].config = {
63+
...bitmap[envName].config,
64+
'teambit.dependencies/dependency-resolver': {
65+
policy: {
66+
dependencies: {
67+
'is-positive': '+',
68+
},
69+
},
70+
},
71+
};
72+
helper.bitMap.write(bitmap);
73+
74+
helper.command.tagAllWithoutBuild();
75+
helper.command.export();
76+
77+
// ── Fresh workspace: fork the env ─────────────────────────────────
78+
helper.scopeHelper.reInitWorkspace({
79+
// Enable the MissingManuallyConfiguredPackages issue so the test
80+
// can detect it (the default e2e helper suppresses it).
81+
disableMissingManuallyConfiguredPackagesIssue: false,
82+
});
83+
helper.scopeHelper.addRemoteScope();
84+
85+
// Fork the env from the remote scope. The forking process copies the
86+
// dep-resolver config (including the "+" entry for is-positive) from
87+
// the tagged version into the new component's .bitmap.
88+
helper.command.fork(`${helper.scopes.remote}/${envName} my-forked-env`);
89+
});
90+
91+
it('bit status should not have MissingManuallyConfiguredPackages issue for the forked env', () => {
92+
helper.command.expectStatusToNotHaveIssue(IssuesClasses.MissingManuallyConfiguredPackages.name);
93+
});
94+
95+
it('bit status should not have any component issues', () => {
96+
helper.command.expectStatusToNotHaveIssues();
97+
});
98+
});
99+
});

scopes/dependencies/dependencies/dependencies-loader/apply-overrides.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,7 @@ export class ApplyOverrides {
304304
if (!overrides) return undefined;
305305
const components = {};
306306
const packages = {};
307+
const missingPkgRemovals = new Set<string>();
307308
DEPENDENCIES_FIELDS.forEach((depField) => {
308309
if (!overrides[depField]) return;
309310
Object.keys(overrides[depField]).forEach((dependency) => {
@@ -336,9 +337,23 @@ export class ApplyOverrides {
336337
if (componentData && !componentData.packageName) {
337338
this.overridesDependencies.missingPackageDependencies.push(dependency);
338339
}
340+
} else if (
341+
// The "+" sentinel couldn't be resolved from workspace package.json
342+
// (e.g. fresh workspace after bit new/fork), but the package was
343+
// already resolved by applyAutoDetectedPeersFromEnvOnEnvItSelf().
344+
// Remove it from missingPackageDependencies to avoid a false positive.
345+
this.allPackagesDependencies.packageDependencies[dependency] ||
346+
this.allPackagesDependencies.devPackageDependencies[dependency] ||
347+
this.allPackagesDependencies.peerPackageDependencies[dependency]
348+
) {
349+
missingPkgRemovals.add(dependency);
339350
}
340351
});
341352
});
353+
if (missingPkgRemovals.size > 0) {
354+
this.overridesDependencies.missingPackageDependencies =
355+
this.overridesDependencies.missingPackageDependencies.filter((pkg) => !missingPkgRemovals.has(pkg));
356+
}
342357
return { components, packages };
343358
}
344359

scopes/dependencies/dependency-resolver/manifest/workspace-manifest-factory.ts

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ import { snapToSemver } from '@teambit/component-package-version';
1010
import type { Logger } from '@teambit/logger';
1111
import type { DependencyList, PackageName } from '../dependencies';
1212
import { ComponentDependency } from '../dependencies';
13-
import type { WorkspacePolicy, EnvPolicy } from '../policy';
13+
import type { WorkspacePolicy, EnvPolicy, VariantPolicyConfigEntryValue, VariantPolicyEntryValue } from '../policy';
1414
import { VariantPolicy } from '../policy';
15+
import { DependencyResolverAspect } from '../dependency-resolver.aspect';
1516
import type { VariantPolicyEntry } from '../policy/variant-policy';
1617
import { createVariantPolicyEntry } from '../policy/variant-policy';
1718
import type { DependencyResolverMain } from '../dependency-resolver.main.runtime';
@@ -464,14 +465,21 @@ export class WorkspaceManifestFactory {
464465
if (!this.resolveEnvPeersFromRoot) {
465466
// Legacy behavior: inject env peer deps into each component's manifest
466467
const envPeerDependencies = await this._getEnvPeerDependencies(component, packageNames);
468+
// Also include packages that are explicitly listed in the component's
469+
// dep-resolver config (e.g. with version "+"). Without this, a fresh
470+
// workspace after `bit new`/`bit fork` hits a chicken-and-egg problem:
471+
// "+" can't resolve → package absent from manifest → filter excludes it
472+
// → package never installed.
473+
const componentExplicitPkgs = this.getComponentExplicitPackages(component);
467474
if (includeAllEnvPeers ?? true) {
468475
peerDepsForManifest = envPeerDependencies;
469476
} else {
470477
peerDepsForManifest = pickBy(envPeerDependencies, (_val, pkgName) => {
471478
return (
472479
depManifestBeforeFiltering.dependencies[pkgName] ||
473480
depManifestBeforeFiltering.devDependencies[pkgName] ||
474-
depManifestBeforeFiltering.peerDependencies[pkgName]
481+
depManifestBeforeFiltering.peerDependencies[pkgName] ||
482+
componentExplicitPkgs.has(pkgName)
475483
);
476484
});
477485

@@ -517,6 +525,20 @@ export class WorkspaceManifestFactory {
517525
return result;
518526
}
519527

528+
/**
529+
* Collect package names explicitly listed in the component's dep-resolver policy,
530+
* excluding entries that represent removals ("-" or `{ version: "-" }`).
531+
*/
532+
private getComponentExplicitPackages(component: Component): Set<string> {
533+
const depResolverEntry = component.get(DependencyResolverAspect.id);
534+
const explicitPolicy = depResolverEntry?.config?.policy ?? {};
535+
return new Set<string>([
536+
...nonRemovedEntryNames(explicitPolicy.dependencies),
537+
...nonRemovedEntryNames(explicitPolicy.devDependencies),
538+
...nonRemovedEntryNames(explicitPolicy.peerDependencies),
539+
]);
540+
}
541+
520542
private async _getEnvPeerDependencies(
521543
component: Component,
522544
packageNamesFromWorkspace: string[]
@@ -675,3 +697,20 @@ async function getMissingPackages(component: Component): Promise<{ devMissings:
675697
runtimeMissings,
676698
};
677699
}
700+
701+
function nonRemovedEntryNames(policySection?: Record<string, VariantPolicyConfigEntryValue>): string[] {
702+
if (!policySection) return [];
703+
const names: string[] = [];
704+
for (const [name, versionSpec] of Object.entries(policySection)) {
705+
// Skip explicit removals expressed as "-" or as removal objects.
706+
if (versionSpec !== '-' && !isRemovalObject(versionSpec)) {
707+
names.push(name);
708+
}
709+
}
710+
return names;
711+
}
712+
713+
function isRemovalObject(val: VariantPolicyConfigEntryValue): boolean {
714+
if (!val || typeof val !== 'object') return false;
715+
return (val as VariantPolicyEntryValue).version === '-';
716+
}

scopes/dependencies/dependency-resolver/policy/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ export {
99
} from './workspace-policy';
1010
export {
1111
VariantPolicy,
12+
VariantPolicyConfigEntryValue,
1213
VariantPolicyEntryValue,
1314
VariantPolicyConfigObject,
1415
SerializedVariantPolicy,

scopes/dependencies/dependency-resolver/policy/variant-policy/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ export {
44
VariantPolicyFromConfigObjectOptions,
55
SerializedVariantPolicy,
66
VariantPolicyEntry,
7+
VariantPolicyConfigEntryValue,
78
VariantPolicyEntryValue,
89
createVariantPolicyEntry,
910
DependencySource,

0 commit comments

Comments
 (0)