Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 99 additions & 0 deletions e2e/harmony/dependencies/forked-env-missing-deps.e2e.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { IssuesClasses } from '@teambit/component-issues';
import { Helper } from '@teambit/legacy.e2e-helper';

/**
* Regression test for PR #10150.
*
* When `bit new` (or `bit fork`) creates a workspace with a forked env, the
* forking process copies the dep-resolver aspect config verbatim from the
* source version. If that config contains entries with the "+" sentinel
* (MANUALLY_ADD_DEPENDENCY — meaning "resolve version from the workspace
* package.json"), a chicken-and-egg problem arises on a fresh workspace:
*
* 1. _manuallyAddPackage() tries to resolve "+" from the workspace
* root package.json, but the package is not yet installed → null →
* pushed to missingPackageDependencies (not to packageDependencies).
* 2. The package is therefore absent from depManifestBeforeFiltering.
* 3. The usedPeerDependencies filter (introduced by PR #10150) checks
* depManifestBeforeFiltering and excludes the package.
* 4. pnpm never installs it.
* 5. Every subsequent `bit status` / `bit install` repeats the cycle.
*
* Before PR #10150, `defaultPeerDependencies` was spread unconditionally,
* which broke the cycle on the first install.
*/
describe('forked env with "+" dependency markers (PR #10150 regression)', function () {
this.timeout(0);
let helper: Helper;

before(() => {
helper = new Helper();
});

after(() => {
helper.scopeHelper.destroy();
});

describe('fork an env whose dep-resolver config has "+" entries for env peer packages', () => {
const envName = 'react-based-env';

before(() => {
// ── Source workspace: create the env, tag, and export ──────────────
helper.scopeHelper.setWorkspaceWithRemoteScope();

// Create a custom env with env.jsonc that declares is-positive as a
// peer dependency. For the env component *itself*, this peer entry
// becomes part of the selfPolicy (via getPoliciesFromEnvForItself),
// which feeds into _getDefaultPeerDependencies().
const envPolicy = {
peers: [{ name: 'is-positive', version: '3.1.0', supportedRange: '^3.0.0' }],
};
helper.env.setCustomNewEnv(undefined, undefined, { policy: envPolicy }, true);

// Make sure is-positive is present in the workspace so the "+" marker
// can be resolved during tagging in this (source) workspace.
helper.command.install('is-positive@3.1.0');

// Simulate the original env having an explicit "+" dep-resolver entry
// for is-positive. This is the config that gets copied verbatim when
// the component is forked.
const bitmap = helper.bitMap.read();
bitmap[envName] = bitmap[envName] || {};
bitmap[envName].config = {
...bitmap[envName].config,
'teambit.dependencies/dependency-resolver': {
policy: {
dependencies: {
'is-positive': '+',
},
},
},
};
helper.bitMap.write(bitmap);

helper.command.tagAllWithoutBuild();
helper.command.export();

// ── Fresh workspace: fork the env ─────────────────────────────────
helper.scopeHelper.reInitWorkspace({
// Enable the MissingManuallyConfiguredPackages issue so the test
// can detect it (the default e2e helper suppresses it).
disableMissingManuallyConfiguredPackagesIssue: false,
});
helper.scopeHelper.addRemoteScope();

// Fork the env from the remote scope. The forking process copies the
// dep-resolver config (including the "+" entry for is-positive) from
// the tagged version into the new component's .bitmap.
helper.command.fork(`${helper.scopes.remote}/${envName} my-forked-env`);
});
Comment thread
zkochan marked this conversation as resolved.

it('bit status should not have MissingManuallyConfiguredPackages issue for the forked env', () => {
helper.command.expectStatusToNotHaveIssue(IssuesClasses.MissingManuallyConfiguredPackages.name);
});

it('bit status should not have any component issues', () => {
helper.command.expectStatusToNotHaveIssues();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ export class ApplyOverrides {
if (!overrides) return undefined;
const components = {};
const packages = {};
const missingPkgRemovals = new Set<string>();
DEPENDENCIES_FIELDS.forEach((depField) => {
if (!overrides[depField]) return;
Object.keys(overrides[depField]).forEach((dependency) => {
Expand Down Expand Up @@ -336,9 +337,23 @@ export class ApplyOverrides {
if (componentData && !componentData.packageName) {
this.overridesDependencies.missingPackageDependencies.push(dependency);
}
} else if (
// The "+" sentinel couldn't be resolved from workspace package.json
// (e.g. fresh workspace after bit new/fork), but the package was
// already resolved by applyAutoDetectedPeersFromEnvOnEnvItSelf().
// Remove it from missingPackageDependencies to avoid a false positive.
this.allPackagesDependencies.packageDependencies[dependency] ||
this.allPackagesDependencies.devPackageDependencies[dependency] ||
this.allPackagesDependencies.peerPackageDependencies[dependency]
) {
missingPkgRemovals.add(dependency);
}
});
});
if (missingPkgRemovals.size > 0) {
this.overridesDependencies.missingPackageDependencies =
this.overridesDependencies.missingPackageDependencies.filter((pkg) => !missingPkgRemovals.has(pkg));
}
return { components, packages };
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ import { snapToSemver } from '@teambit/component-package-version';
import type { Logger } from '@teambit/logger';
import type { DependencyList, PackageName } from '../dependencies';
import { ComponentDependency } from '../dependencies';
import type { WorkspacePolicy, EnvPolicy } from '../policy';
import type { WorkspacePolicy, EnvPolicy, VariantPolicyConfigEntryValue, VariantPolicyEntryValue } from '../policy';
import { VariantPolicy } from '../policy';
import { DependencyResolverAspect } from '../dependency-resolver.aspect';
import type { VariantPolicyEntry } from '../policy/variant-policy';
import { createVariantPolicyEntry } from '../policy/variant-policy';
import type { DependencyResolverMain } from '../dependency-resolver.main.runtime';
Expand Down Expand Up @@ -464,14 +465,21 @@ export class WorkspaceManifestFactory {
if (!this.resolveEnvPeersFromRoot) {
// Legacy behavior: inject env peer deps into each component's manifest
const envPeerDependencies = await this._getEnvPeerDependencies(component, packageNames);
// Also include packages that are explicitly listed in the component's
// dep-resolver config (e.g. with version "+"). Without this, a fresh
// workspace after `bit new`/`bit fork` hits a chicken-and-egg problem:
// "+" can't resolve → package absent from manifest → filter excludes it
// → package never installed.
const componentExplicitPkgs = this.getComponentExplicitPackages(component);
if (includeAllEnvPeers ?? true) {
peerDepsForManifest = envPeerDependencies;
} else {
peerDepsForManifest = pickBy(envPeerDependencies, (_val, pkgName) => {
return (
depManifestBeforeFiltering.dependencies[pkgName] ||
depManifestBeforeFiltering.devDependencies[pkgName] ||
depManifestBeforeFiltering.peerDependencies[pkgName]
depManifestBeforeFiltering.peerDependencies[pkgName] ||
componentExplicitPkgs.has(pkgName)
);
});

Expand Down Expand Up @@ -517,6 +525,20 @@ export class WorkspaceManifestFactory {
return result;
}

/**
* Collect package names explicitly listed in the component's dep-resolver policy,
* excluding entries that represent removals ("-" or `{ version: "-" }`).
*/
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),
]);
}

private async _getEnvPeerDependencies(
component: Component,
packageNamesFromWorkspace: string[]
Expand Down Expand Up @@ -675,3 +697,20 @@ async function getMissingPackages(component: Component): Promise<{ devMissings:
runtimeMissings,
};
}

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)) {
names.push(name);
}
}
return names;
}

function isRemovalObject(val: VariantPolicyConfigEntryValue): boolean {
if (!val || typeof val !== 'object') return false;
return (val as VariantPolicyEntryValue).version === '-';
}
1 change: 1 addition & 0 deletions scopes/dependencies/dependency-resolver/policy/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export {
} from './workspace-policy';
export {
VariantPolicy,
VariantPolicyConfigEntryValue,
VariantPolicyEntryValue,
VariantPolicyConfigObject,
SerializedVariantPolicy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export {
VariantPolicyFromConfigObjectOptions,
SerializedVariantPolicy,
VariantPolicyEntry,
VariantPolicyConfigEntryValue,
VariantPolicyEntryValue,
createVariantPolicyEntry,
DependencySource,
Expand Down