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
59 changes: 59 additions & 0 deletions e2e/harmony/build-cmd.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,65 @@ describe('build command', function () {
});
}
);
(supportNpmCiRegistryTesting ? describe : describe.skip)(
'optimized capsule creation with a package dependency that points back into the capsule graph',
() => {
let npmCiRegistry: NpmCiRegistry;
before(async () => {
helper = new Helper({ scopesOptions: { remoteScopeWithDot: true } });
helper.scopeHelper.setWorkspaceWithRemoteScope();
npmCiRegistry = new NpmCiRegistry(helper);
await npmCiRegistry.init();
npmCiRegistry.configureCiInPackageJsonHarmony();

// Build this graph, where comp3 is deliberately NOT a direct dev-dependency of the seeder:
//
// comp1 -(dev)-> comp2 -(prod)-> comp3 -(prod)-> comp4
// | ^
// `--------------------(dev)---------------------'
//
// comp4 is modified, so it must be a local capsule. Installing comp3 from the registry
// would make that package load its older comp4 dependency while comp1 loads the local
// comp4 capsule. The two SharedClass constructors then have different identities.
helper.fixtures.populateComponents(4);
helper.fs.outputFile('comp1/index.js', `module.exports = () => 'comp1';`);
const comp2PackageName = helper.general.getPackageNameByCompName('comp2', true);
const comp3PackageName = helper.general.getPackageNameByCompName('comp3', true);
const comp4PackageName = helper.general.getPackageNameByCompName('comp4', true);
helper.fs.outputFile('comp4/index.js', 'module.exports = class SharedClass {};');
helper.fs.outputFile(
'comp3/index.js',
`const SharedClass = require('${comp4PackageName}'); module.exports = /** @type {any} */ (new SharedClass());`
);
helper.fs.outputFile('comp2/index.js', `module.exports = /** @type {any} */ (require('${comp3PackageName}'));`);
helper.fs.outputFile(
'comp1/comp1.spec.js',
`const valueFromPackageClosure = require('${comp2PackageName}');
const SharedClass = require('${comp4PackageName}');

describe('package and capsule generation identity', () => {
it('uses the capsule generation throughout the dependency closure', () => {
expect(valueFromPackageClosure).toBeInstanceOf(SharedClass);
});
});
`
);

helper.command.tagAllComponents();
helper.command.export();
helper.fs.appendFile('comp1/index.js', '\n// modification to comp1');
helper.fs.appendFile('comp4/index.js', '\n// modification to comp4');
helper.command.build('comp1');
});
after(() => {
npmCiRegistry.destroy();
});
it('keeps the transitive package dependency on the capsule side of the graph boundary', () => {
const comp3Capsule = helper.command.getCapsuleOfComponent(`${helper.scopes.remote}/comp3@0.0.1`);
expect(comp3Capsule).to.be.a.directory();
});
}
);
describe('optimized capsule creation for exported dependencies for self hosting', () => {
before(async () => {
helper = new Helper();
Expand Down
22 changes: 19 additions & 3 deletions e2e/harmony/ci-commands.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,22 +278,35 @@ describe('ci commands', function () {
describe('bit ci pr reuses the existing remote lane across subsequent PR commits', () => {
let firstPrOutput: string;
let secondPrOutput: string;
let comp3HeadAfterFirstRun: string;
let comp3HeadAfterSecondRun: string;
before(() => {
helper.scopeHelper.setWorkspaceWithRemoteScope();
setupGitRemote();
setupComponentsAndInitialCommit();
setupComponentsAndInitialCommit(3);

const getComp3RemoteLaneHead = () => {
const lane = JSON.parse(
helper.command.runCmd(`bit lane show ${helper.scopes.remote}/feature-reuse-lane-test --remote --json`)
);
const comp3 = lane.components.find((component: any) => component.id.endsWith('/comp3'));
if (!comp3) throw new Error(`comp3 was not found on the reused lane: ${JSON.stringify(lane.components)}`);
return comp3.head;
};

helper.command.runCmd('git checkout -b feature/reuse-lane-test');

helper.fs.outputFile('comp1/comp1.js', 'console.log("first commit");');
helper.command.runCmd('git add comp1/comp1.js');
helper.fs.outputFile('comp3/comp3.js', 'console.log("first commit");');
helper.command.runCmd('git add comp3/comp3.js');
helper.command.runCmd('git commit -m "feat: first commit"');
firstPrOutput = helper.command.runCmd('bit ci pr --keep-lane --message "first"');
comp3HeadAfterFirstRun = getComp3RemoteLaneHead();

helper.fs.outputFile('comp2/comp2.js', 'console.log("second commit");');
helper.command.runCmd('git add comp2/comp2.js');
helper.command.runCmd('git commit -m "feat: second commit"');
secondPrOutput = helper.command.runCmd('bit ci pr --keep-lane --message "second"');
comp3HeadAfterSecondRun = getComp3RemoteLaneHead();
});
it('should report that the lane was reused on the second run', () => {
const cleanOutput = removeChalkCharacters(secondPrOutput) as string;
Expand All @@ -310,6 +323,9 @@ describe('ci commands', function () {
const matching = remoteLanes.lanes.filter((l: any) => l.name === 'feature-reuse-lane-test');
expect(matching).to.have.lengthOf(1);
});
it('should preserve the head of a component that was unchanged in the second run', () => {
expect(comp3HeadAfterSecondRun).to.equal(comp3HeadAfterFirstRun);
});
});

/**
Expand Down
32 changes: 32 additions & 0 deletions scopes/component/isolator/dependency-closed-package-set.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { expect } from 'chai';
import { ComponentID } from '@teambit/component-id';
import type { ComponentIdGraph } from '@teambit/graph';
import { enforceDependencyClosedPackageSet } from './dependency-closed-package-set';

describe('enforceDependencyClosedPackageSet', () => {
it('keeps closure propagation version-accurate when the graph contains multiple component versions', () => {
const consumerOfV1 = ComponentID.fromString('scope/consumer-v1@1.0.0');
const consumerOfV2 = ComponentID.fromString('scope/consumer-v2@1.0.0');
const dependencyV1 = ComponentID.fromString('scope/dependency@1.0.0');
const dependencyV2 = ComponentID.fromString('scope/dependency@2.0.0');
const nodes = new Map(
[consumerOfV1, consumerOfV2, dependencyV1, dependencyV2].map((id) => [id.toString(), { attr: id }])
);
const graph = {
edges: [
{ sourceId: consumerOfV1.toString(), targetId: dependencyV1.toString(), attr: 'prod' },
{ sourceId: consumerOfV2.toString(), targetId: dependencyV2.toString(), attr: 'prod' },
],
node: (id: string) => nodes.get(id),
} as unknown as ComponentIdGraph;
const capsuleIds = new Set([dependencyV2.toString()]);
const packageCandidateIds = new Set([consumerOfV1.toString(), consumerOfV2.toString(), dependencyV1.toString()]);

enforceDependencyClosedPackageSet(graph, capsuleIds, packageCandidateIds);

expect(capsuleIds).to.include(consumerOfV2.toString());
expect(capsuleIds).to.not.include(consumerOfV1.toString());
expect(packageCandidateIds).to.include(consumerOfV1.toString());
expect(packageCandidateIds).to.include(dependencyV1.toString());
});
});
40 changes: 40 additions & 0 deletions scopes/component/isolator/dependency-closed-package-set.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import type { ComponentIdGraph } from '@teambit/graph';

export type CapsulePromotion = {
dependentId: string;
dependencyId: string;
};

/**
* Mutate the capsule/package partition until every dependency of a package candidate is also a package candidate.
* IDs must include versions so different versions of the same component remain independent graph nodes.
*/
export function enforceDependencyClosedPackageSet(
graph: ComponentIdGraph,
capsuleIds: Set<string>,
packageCandidateIds: Set<string>
): CapsulePromotion[] {
const dependentsByDependencyId = new Map<string, Set<string>>();
graph.edges.forEach((edge) => {
const sourceId = graph.node(edge.sourceId)?.attr?.toString();
const targetId = graph.node(edge.targetId)?.attr?.toString();
if (!sourceId || !targetId) return;
const dependents = dependentsByDependencyId.get(targetId) ?? new Set<string>();
dependents.add(sourceId);
dependentsByDependencyId.set(targetId, dependents);
});

const promotions: CapsulePromotion[] = [];
const capsuleQueue = [...capsuleIds];
for (let queueIndex = 0; queueIndex < capsuleQueue.length; queueIndex += 1) {
const dependencyId = capsuleQueue[queueIndex];
dependentsByDependencyId.get(dependencyId)?.forEach((dependentId) => {
if (!packageCandidateIds.delete(dependentId)) return;
capsuleIds.add(dependentId);
capsuleQueue.push(dependentId);
promotions.push({ dependentId, dependencyId });
});
}

return promotions;
}
42 changes: 35 additions & 7 deletions scopes/component/isolator/isolator.main.runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { ComponentMap, ComponentAspect } from '@teambit/component';
import type { ComponentMain, ComponentFactory, Component } from '@teambit/component';
import { getComponentPackageVersion, snapToSemver } from '@teambit/component-package-version';
import { createLinks } from '@teambit/dependencies.fs.linked-dependencies';
import type { GraphMain } from '@teambit/graph';
import type { ComponentIdGraph, GraphMain } from '@teambit/graph';
import { GraphAspect } from '@teambit/graph';
import type { SlotRegistry } from '@teambit/harmony';
import { Slot } from '@teambit/harmony';
Expand Down Expand Up @@ -68,6 +68,7 @@ import pMap from 'p-map';
import { Capsule } from './capsule';
import CapsuleList from './capsule-list';
import { CapsuleCache } from './capsule-cache';
import { enforceDependencyClosedPackageSet } from './dependency-closed-package-set';
import type { CapsuleKind, PruneCapsulesOptions, PruneCapsulesReport } from './capsule-cache';
import { IsolatorAspect } from './isolator.aspect';
import { symlinkOnCapsuleRoot, symlinkDependenciesToCapsules } from './symlink-dependencies-to-capsules';
Expand Down Expand Up @@ -483,6 +484,7 @@ export class IsolatorMain {
filteredComps,
seeders,
host,
successorsSubgraph,
opts.originalSeeders
);
this.logger.debug(
Expand Down Expand Up @@ -1696,6 +1698,7 @@ export class IsolatorMain {
components: Component[],
seederIds: ComponentID[],
host: ComponentFactory,
graph: ComponentIdGraph,
originalSeeders?: ComponentID[]
): Promise<Component[]> {
this.logger.debug(`filterUnmodifiedExportedDependencies: filtering ${components.length} components`);
Expand All @@ -1704,7 +1707,8 @@ export class IsolatorMain {
// @ts-ignore it's there, but we can't have the type of ScopeMain here to not create a circular dependency
const remotes = await scope.getRemoteScopes();

const filtered: Component[] = [];
const capsuleIds = new Set<string>();
const packageCandidateIds = new Set<string>();

// Precompute version-normalized ID sets so membership is O(1) per component instead of a linear scan.
// `toStringWithoutVersion()` is the string equivalent of `isEqual(id, { ignoreVersion: true })`.
Expand All @@ -1719,7 +1723,7 @@ export class IsolatorMain {

if (seederIdsNoVersion.has(componentIdNoVersion)) {
// Always include seeders (modified components and their dependents)
filtered.push(component);
capsuleIds.add(componentIdStr);
continue;
}

Expand All @@ -1729,7 +1733,7 @@ export class IsolatorMain {
// then have to resolve it as an installed package instead of referencing the freshly-built capsule.
// Keep it so the graph stays consistent across `bit build` and `bit tag` (both include it as a capsule).
if (originalSeederIdsNoVersion?.has(componentIdNoVersion)) {
filtered.push(component);
capsuleIds.add(componentIdStr);
continue;
}
// For dependencies, check if they are exported and unmodified
Expand All @@ -1741,7 +1745,7 @@ export class IsolatorMain {
const isModified = await component.isModified();
if (isModified) {
// Always include modified components
filtered.push(component);
capsuleIds.add(componentIdStr);
continue;
}

Expand All @@ -1763,12 +1767,36 @@ export class IsolatorMain {
wasPublished;

if (canBeInstalled) {
this.logger.debug(`[OPTIMIZATION] Excluding unmodified exported dependency: ${componentIdStr}`);
packageCandidateIds.add(componentIdStr);
} else {
filtered.push(component);
capsuleIds.add(componentIdStr);
}
}

/**
* The package side of the optimized graph must be dependency-closed. If an installed package
* points back to a component kept as a capsule, Node can load both the package's recorded
* dependency version and the locally linked capsule version in one process. For stateful Bit
* modules this breaks class identity (`instanceof`), Harmony singletons, remotes, and caches.
*
* Start with components that must be capsules, then propagate backwards through every graph
* edge until no package candidate depends on a capsule. This handles runtime, dev, peer, and
* transitive crossings without disabling optimization for a dependency subtree that is wholly
* installable from the registry.
*/
const promotions = enforceDependencyClosedPackageSet(graph, capsuleIds, packageCandidateIds);
promotions.forEach(({ dependentId, dependencyId }) =>
this.logger.debug(
`[OPTIMIZATION] Keeping ${dependentId} in capsule graph because it depends on capsule ${dependencyId}`
)
);

const filtered = components.filter((component) => {
if (capsuleIds.has(component.id.toString())) return true;
this.logger.debug(`[OPTIMIZATION] Excluding unmodified exported dependency: ${component.id.toString()}`);
return false;
});

this.logger.debug(
`filterUnmodifiedExportedDependencies: kept ${filtered.length} out of ${components.length} components`
);
Expand Down