Skip to content

Commit 36c7f9f

Browse files
authored
fix: Respect missing optional dependencies (#1429)
So far we didn't have the case with optional peer dependencies in the `package-lock.json` that are not present. This fix addresses exactly this case and adds tests to prevent future regressions
1 parent 91fc27e commit 36c7f9f

4 files changed

Lines changed: 72 additions & 4 deletions

File tree

internal/shrinkwrap-extractor/lib/convertPackageLockToShrinkwrap.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,8 @@ function collectDependencies(node, relevantPackageLocations) {
139139
node = node.target;
140140
}
141141
for (const edge of node.edgesOut.values()) {
142-
if (edge.dev) {
142+
if (edge.dev || !edge.to) {
143+
// Skip dev dependencies and optional peer dependencies that are not installed
143144
continue;
144145
}
145146
collectDependencies(edge.to, relevantPackageLocations);

internal/shrinkwrap-extractor/test/expected/package.a/npm-shrinkwrap.json

Lines changed: 23 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/shrinkwrap-extractor/test/fixture/project.a/package-lock.fixture.json

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19247,7 +19247,8 @@
1924719247
"replacestream": "^4.0.3",
1924819248
"router": "^2.2.0",
1924919249
"spdy": "^4.0.2",
19250-
"yesno": "^0.4.0"
19250+
"yesno": "^0.4.0",
19251+
"ws": "^8.21.0"
1925119252
},
1925219253
"devDependencies": {
1925319254
"@eslint/js": "^9.8.0",
@@ -19366,6 +19367,27 @@
1936619367
"funding": {
1936719368
"url": "https://github.com/sponsors/sindresorhus"
1936819369
}
19370+
},
19371+
"node_modules/ws": {
19372+
"version": "8.21.0",
19373+
"resolved": "https://registry.npmjs.org/ws/-/ws-8.21.0.tgz",
19374+
"integrity": "sha512-Vsp28b7DRcimFQvrqu2Wek3z1iYxDCWqHYB8Qsnk/S4RfaCQzPGPyBNuVjJV3cd6UiKtUtp6sNM77gWvzcCH+g==",
19375+
"license": "MIT",
19376+
"engines": {
19377+
"node": ">=10.0.0"
19378+
},
19379+
"peerDependencies": {
19380+
"bufferutil": "^4.0.1",
19381+
"utf-8-validate": ">=5.0.2"
19382+
},
19383+
"peerDependenciesMeta": {
19384+
"bufferutil": {
19385+
"optional": true
19386+
},
19387+
"utf-8-validate": {
19388+
"optional": true
19389+
}
19390+
}
1936919391
}
1937019392
}
19371-
}
19393+
}

internal/shrinkwrap-extractor/test/lib/convertToShrinkwrap.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,29 @@ test("Compare generated shrinkwrap with expected result", async (t) => {
204204
"Generated shrinkwrap packages should match expected");
205205
});
206206

207+
test("Optional peer dependencies with null edges should be excluded", async (t) => {
208+
// Guards against: ws declares bufferutil and utf-8-validate as peerOptional, but they are not
209+
// installed. Arborist represents these as edges with edge.to === null. The generator must skip
210+
// them instead of throwing "Cannot read properties of null (reading 'location')".
211+
const __dirname = import.meta.dirname;
212+
213+
const cwd = path.join(__dirname, "..", "fixture", "project.a");
214+
const symlinkPath = await setupFixtureSymlink(cwd);
215+
t.after(async () => await unlink(symlinkPath).catch(() => {}));
216+
217+
const shrinkwrapJson = await convertPackageLockToShrinkwrap(cwd, "@ui5/cli");
218+
219+
// ws itself must be present (it is a real production dep of @ui5/server)
220+
assert.ok(shrinkwrapJson.packages["node_modules/ws"],
221+
"ws should be included in the shrinkwrap");
222+
223+
// Its optional peer deps are not installed and must NOT appear
224+
assert.equal(shrinkwrapJson.packages["node_modules/bufferutil"], undefined,
225+
"bufferutil (optional peerDep of ws) must not be included");
226+
assert.equal(shrinkwrapJson.packages["node_modules/utf-8-validate"], undefined,
227+
"utf-8-validate (optional peerDep of ws) must not be included");
228+
});
229+
207230
// Error handling tests
208231
test("Error handling - invalid target package name", async (t) => {
209232
const __dirname = import.meta.dirname;

0 commit comments

Comments
 (0)