Skip to content
Draft
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"comment": "In PnpmShrinkwrapFile getIntegrityForImporter, remove the external filter and include workspace-local link: dependencies by recursing into their importer entries, so shrinkwrap-deps.json hashes cover the full dependency tree.",
"type": "patch",
"packageName": "@microsoft/rush"
}
],
"packageName": "@microsoft/rush"
}
52 changes: 34 additions & 18 deletions libraries/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -911,36 +911,57 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile {
if (!integrityMap) {
const importer: IPnpmShrinkwrapImporterYaml | undefined = this.getImporter(importerKey);
if (importer) {
integrityMap = new Map();
this._integrities.set(importerKey, integrityMap);
const resolvedIntegrityMap: Map<string, string> = new Map();
integrityMap = resolvedIntegrityMap;
this._integrities.set(importerKey, resolvedIntegrityMap);

const sha256Digest: string = crypto
.createHash('sha256')
.update(JSON.stringify(importer))
.digest('base64');
const selfIntegrity: string = `${importerKey}:${sha256Digest}:`;
integrityMap.set(importerKey, selfIntegrity);
resolvedIntegrityMap.set(importerKey, selfIntegrity);

const { dependencies, devDependencies, optionalDependencies } = importer;

const externalFilter: (name: string, version: IPnpmVersionSpecifier) => boolean = (
name: string,
versionSpecifier: IPnpmVersionSpecifier
): boolean => {
const version: string = normalizePnpmVersionSpecifier(versionSpecifier);
return !version.includes('link:');
const processCollection = (
collection: Record<string, IPnpmVersionSpecifier>,
optional: boolean
): void => {
const externalDeps: Record<string, IPnpmVersionSpecifier> = {};
for (const [name, versionSpecifier] of Object.entries(collection)) {
const version: string = normalizePnpmVersionSpecifier(versionSpecifier);
if (version.startsWith('link:')) {
// This is a workspace-local dependency; resolve it to an importer key and recurse.
// The link: path is relative to the project folder (which is the importer key itself),
// so we join the importer key with the link path (not dirname).
// Lockfile paths are always POSIX, so we use path.posix helpers.
const linkPath: string = version.slice('link:'.length);
const targetKey: string = path.posix.normalize(path.posix.join(importerKey, linkPath));
const linkedIntegrities: Map<string, string> | undefined =
this.getIntegrityForImporter(targetKey);
if (linkedIntegrities) {
for (const [dep, integrity] of linkedIntegrities) {
resolvedIntegrityMap.set(dep, integrity);
}
}
} else {
externalDeps[name] = versionSpecifier;
}
}
this._addIntegrities(resolvedIntegrityMap, externalDeps, optional);
};

if (dependencies) {
this._addIntegrities(integrityMap, dependencies, false, externalFilter);
processCollection(dependencies, false);
}

if (devDependencies) {
this._addIntegrities(integrityMap, devDependencies, false, externalFilter);
processCollection(devDependencies, false);
}

if (optionalDependencies) {
this._addIntegrities(integrityMap, optionalDependencies, true, externalFilter);
processCollection(optionalDependencies, true);
}
}
}
Expand Down Expand Up @@ -1269,14 +1290,9 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile {
private _addIntegrities(
integrityMap: Map<string, string>,
collection: Record<string, IPnpmVersionSpecifier>,
optional: boolean,
filter?: (name: string, version: IPnpmVersionSpecifier) => boolean
optional: boolean
): void {
for (const [name, version] of Object.entries(collection)) {
if (filter && !filter(name, version)) {
continue;
}

const packageId: string = this._getPackageId(name, version);
if (integrityMap.has(packageId)) {
// The entry could already have been added as a nested dependency
Expand Down
228 changes: 228 additions & 0 deletions libraries/rush-lib/src/logic/pnpm/test/PnpmShrinkwrapFile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,234 @@ snapshots:
// because the sub-dependency (bar) resolved to different versions
expect(fooIntegrity1).not.toEqual(fooIntegrity2);
});

it('includes workspace-local link: dependencies by recursing into their importer entries', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Be sure to cover the following scenarios with test cases:

  1. Workspace project 1 -> Workspace project 2 -> External Dep 1 -> External Dep 2
  2. Workspace project 1 -> External Dep 1 -> Workspace project 2 -> External Dep 2

Note that these may not necessarily be the same dependencies between examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added both scenarios in b37088f:

  • Scenario 1 ("workspace project 1 → workspace project 2 → external dep 1 → external dep 2"): project-1 links to project-2, project-2 depends on ext-a, ext-a transitively depends on ext-b — all four entries verified in project-1's integrity map.
  • Scenario 2 ("workspace project 1 → external dep 1 and workspace project 2 → external dep 2"): project-1 has both a direct external dep (ext-x) and a workspace link dep (project-2), project-2 depends on ext-y — all four entries verified.

// This test verifies that link: (workspace-local) dependencies are no longer filtered out.
// The shrinkwrap-deps.json for an importer should include hashes from its workspace
// dependencies' importer sections, all the way down the tree.
//
// In a real Rush repo (no subspaces), importer keys start with '../../' and
// link: paths start with '../'.
//
// Topology: project-1 -> (link:) project-2 -> lodash@4.17.21

const shrinkwrapContent: string = `
lockfileVersion: '9.0'
settings:
autoInstallPeers: true
excludeLinksFromLockfile: false
importers:
.:
{}
../../project-1:
dependencies:
project-2:
specifier: workspace:*
version: link:../project-2
../../project-2:
dependencies:
lodash:
specifier: ^4.17.0
version: 4.17.21
packages:
lodash@4.17.21:
resolution:
integrity: sha512-lodash==
snapshots:
lodash@4.17.21: {}
`;

const shrinkwrapFile = PnpmShrinkwrapFile.loadFromString(shrinkwrapContent, {
subspaceHasNoProjects: false
});

PnpmShrinkwrapFile.clearCache();

const proj1IntegrityMap = shrinkwrapFile.getIntegrityForImporter('../../project-1');

expect(proj1IntegrityMap).toBeDefined();

// project-1's integrity map should include project-2's importer entry
expect(proj1IntegrityMap!.has('../../project-2')).toBe(true);

// It should also include the transitive external dependency of project-2
expect(proj1IntegrityMap!.has('lodash@4.17.21')).toBe(true);

// The integrity map for project-2 itself should also be populated
const proj2IntegrityMap = shrinkwrapFile.getIntegrityForImporter('../../project-2');
expect(proj2IntegrityMap).toBeDefined();
expect(proj2IntegrityMap!.has('../../project-2')).toBe(true);
expect(proj2IntegrityMap!.has('lodash@4.17.21')).toBe(true);
});

it('produces different hashes when a workspace-local dependency changes', () => {
// This test verifies that changing the dependencies of a workspace-local package
// causes the dependent importer's integrity to differ.
//
// Topology: project-1 -> (link:) project-2 -> lodash@4.17.x (version differs between cases)

const buildContent = (lodashVersion: string): string => `
lockfileVersion: '9.0'
settings:
autoInstallPeers: true
excludeLinksFromLockfile: false
importers:
.:
{}
../../project-1:
dependencies:
project-2:
specifier: workspace:*
version: link:../project-2
../../project-2:
dependencies:
lodash:
specifier: ^4.17.0
version: ${lodashVersion}
packages:
lodash@${lodashVersion}:
resolution:
integrity: sha512-lodash-${lodashVersion}==
snapshots:
lodash@${lodashVersion}: {}
`;

const shrinkwrapFile1 = PnpmShrinkwrapFile.loadFromString(buildContent('4.17.21'), {
subspaceHasNoProjects: false
});
const shrinkwrapFile2 = PnpmShrinkwrapFile.loadFromString(buildContent('4.17.20'), {
subspaceHasNoProjects: false
});

PnpmShrinkwrapFile.clearCache();

const proj1IntegrityMap1 = shrinkwrapFile1.getIntegrityForImporter('../../project-1');
const proj1IntegrityMap2 = shrinkwrapFile2.getIntegrityForImporter('../../project-1');

expect(proj1IntegrityMap1).toBeDefined();
expect(proj1IntegrityMap2).toBeDefined();

// The self-hash of project-1 does NOT change because the root importer object itself is
// identical in both cases (it still references link:../project-2). However, project-2's
// integrity hash should differ because its lodash dependency resolved to a different version.
const proj2Integrity1 = proj1IntegrityMap1!.get('../../project-2');
const proj2Integrity2 = proj1IntegrityMap2!.get('../../project-2');

expect(proj2Integrity1).toBeDefined();
expect(proj2Integrity2).toBeDefined();
expect(proj2Integrity1).not.toEqual(proj2Integrity2);
});

it('scenario 1: workspace project 1 -> workspace project 2 -> external dep 1 -> external dep 2', () => {
// Tests the full chain: project-1 links to project-2, project-2 depends on ext-a,
// and ext-a transitively depends on ext-b. All four should appear in project-1's integrity map.

const shrinkwrapContent: string = `
lockfileVersion: '9.0'
settings:
autoInstallPeers: true
excludeLinksFromLockfile: false
importers:
.:
{}
../../project-1:
dependencies:
project-2:
specifier: workspace:*
version: link:../project-2
../../project-2:
dependencies:
ext-a:
specifier: ^1.0.0
version: 1.0.0
packages:
ext-a@1.0.0:
resolution:
integrity: sha512-ext-a==
ext-b@1.0.0:
resolution:
integrity: sha512-ext-b==
snapshots:
ext-a@1.0.0:
dependencies:
ext-b: 1.0.0
ext-b@1.0.0: {}
`;

const shrinkwrapFile = PnpmShrinkwrapFile.loadFromString(shrinkwrapContent, {
subspaceHasNoProjects: false
});

PnpmShrinkwrapFile.clearCache();

const proj1IntegrityMap = shrinkwrapFile.getIntegrityForImporter('../../project-1');

expect(proj1IntegrityMap).toBeDefined();
// project-2's importer entry
expect(proj1IntegrityMap!.has('../../project-2')).toBe(true);
// ext-a (direct dep of project-2)
expect(proj1IntegrityMap!.has('ext-a@1.0.0')).toBe(true);
// ext-b (transitive dep through ext-a)
expect(proj1IntegrityMap!.has('ext-b@1.0.0')).toBe(true);
});

it('scenario 2: workspace project 1 -> external dep 1 and workspace project 2 -> external dep 2', () => {
// Tests that when project-1 has both an external direct dependency AND a workspace link
// dependency, both sub-trees are fully captured in the integrity map.
//
// project-1 depends on ext-x (direct external) and project-2 (workspace link).
// project-2 depends on ext-y.
// All four should appear in project-1's integrity map.

const shrinkwrapContent: string = `
lockfileVersion: '9.0'
settings:
autoInstallPeers: true
excludeLinksFromLockfile: false
importers:
.:
{}
../../project-1:
dependencies:
ext-x:
specifier: ^2.0.0
version: 2.0.0
project-2:
specifier: workspace:*
version: link:../project-2
../../project-2:
dependencies:
ext-y:
specifier: ^3.0.0
version: 3.0.0
packages:
ext-x@2.0.0:
resolution:
integrity: sha512-ext-x==
ext-y@3.0.0:
resolution:
integrity: sha512-ext-y==
snapshots:
ext-x@2.0.0: {}
ext-y@3.0.0: {}
`;

const shrinkwrapFile = PnpmShrinkwrapFile.loadFromString(shrinkwrapContent, {
subspaceHasNoProjects: false
});

PnpmShrinkwrapFile.clearCache();

const proj1IntegrityMap = shrinkwrapFile.getIntegrityForImporter('../../project-1');

expect(proj1IntegrityMap).toBeDefined();
// ext-x (direct external dep of project-1)
expect(proj1IntegrityMap!.has('ext-x@2.0.0')).toBe(true);
// project-2 (workspace link dep of project-1)
expect(proj1IntegrityMap!.has('../../project-2')).toBe(true);
// ext-y (external dep of project-2, reached via link)
expect(proj1IntegrityMap!.has('ext-y@3.0.0')).toBe(true);
});
});

describe('Check is workspace project modified', () => {
Expand Down
Loading