Skip to content

Commit 3cbdaa5

Browse files
JamesPatrickGillJamesPatrickGill
authored
Merge pull request #289 from snyk/feat/OSM-3009/handle-missing-optional-deps
feat: [OSM-3009] handle optional dependencies without separate packag…
2 parents dddb47b + 9f37a3c commit 3cbdaa5

File tree

8 files changed

+234
-35
lines changed

8 files changed

+234
-35
lines changed

‎lib/dep-graph-builders/npm-lock-v2/index.ts‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ const getChildNode = (
208208
depInfo: {
209209
version: string;
210210
isDev: boolean;
211+
isOptional?: boolean;
211212
alias?: {
212213
aliasName: string;
213214
aliasTargetDepName: string;
@@ -272,6 +273,19 @@ const getChildNode = (
272273
);
273274

274275
if (!childNodeKey) {
276+
// Handle optional dependencies that don't have separate package entries
277+
if (depInfo.isOptional) {
278+
return {
279+
id: `${name}@${depInfo.version}`,
280+
name: name,
281+
version: depInfo.version,
282+
dependencies: {},
283+
isDev: depInfo.isDev,
284+
missingLockFileEntry: true,
285+
key: '',
286+
};
287+
}
288+
275289
// https://snyksec.atlassian.net/wiki/spaces/SCA/pages/3785687123/NPM+Bundled+Dependencies+Analysis
276290
// Check if this dependency is bundled in the parent package
277291
// Bundled dependencies may not have separate lockfile entries when
@@ -339,17 +353,21 @@ const getChildNode = (
339353
depData = pkgs[depData.resolved as string];
340354
}
341355

342-
const dependencies = getGraphDependencies(
343-
depData.dependencies || {},
344-
depInfo.isDev,
345-
);
356+
const dependencies = getGraphDependencies(depData.dependencies || {}, {
357+
isDev: depInfo.isDev,
358+
});
346359

347360
const devDependencies = includeDevDeps
348-
? getGraphDependencies(depData.devDependencies || {}, depInfo.isDev)
361+
? getGraphDependencies(depData.devDependencies || {}, {
362+
isDev: depInfo.isDev,
363+
})
349364
: {};
350365

351366
const optionalDependencies = includeOptionalDeps
352-
? getGraphDependencies(depData.optionalDependencies || {}, depInfo.isDev)
367+
? getGraphDependencies(depData.optionalDependencies || {}, {
368+
isDev: depInfo.isDev,
369+
isOptional: true,
370+
})
353371
: {};
354372

355373
return {

‎lib/dep-graph-builders/pnpm/utils.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,17 @@ export const getPnpmChildNode = (
5757
}
5858
} else {
5959
const depData = pkgs[childNodeKey];
60-
const dependencies = getGraphDependencies(
61-
depData.dependencies || {},
62-
depInfo.isDev,
63-
);
60+
const dependencies = getGraphDependencies(depData.dependencies || {}, {
61+
isDev: depInfo.isDev,
62+
});
6463
const devDependencies = includeDevDeps
65-
? getGraphDependencies(depData.devDependencies || {}, true)
64+
? getGraphDependencies(depData.devDependencies || {}, { isDev: true })
6665
: {};
6766
const optionalDependencies = includeOptionalDeps
68-
? getGraphDependencies(depData.optionalDependencies || {}, depInfo.isDev)
67+
? getGraphDependencies(depData.optionalDependencies || {}, {
68+
isDev: depInfo.isDev,
69+
isOptional: true,
70+
})
6971
: {};
7072
return {
7173
id: `${depData.name}@${depData.version}`,

‎lib/dep-graph-builders/util.ts‎

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ import { NormalisedPkgs } from './types';
55
import { OutOfSyncError } from '../errors';
66
import { LockfileType } from '../parsers';
77

8-
export type Dependencies = Record<string, { version: string; isDev: boolean }>;
8+
export type Dependencies = Record<
9+
string,
10+
{ version: string; isDev: boolean; isOptional?: boolean }
11+
>;
912

1013
export interface PkgNode {
1114
id: string;
@@ -61,16 +64,23 @@ export const getTopLevelDeps = (
6164
includePeerDeps?: boolean;
6265
},
6366
): Dependencies => {
64-
const prodDeps = getGraphDependencies(pkgJson.dependencies || {}, false);
67+
const prodDeps = getGraphDependencies(pkgJson.dependencies || {}, {
68+
isDev: false,
69+
});
6570

66-
const devDeps = getGraphDependencies(pkgJson.devDependencies || {}, true);
71+
const devDeps = getGraphDependencies(pkgJson.devDependencies || {}, {
72+
isDev: true,
73+
});
6774

6875
const optionalDeps = options.includeOptionalDeps
69-
? getGraphDependencies(pkgJson.optionalDependencies || {}, false)
76+
? getGraphDependencies(pkgJson.optionalDependencies || {}, {
77+
isDev: false,
78+
isOptional: true,
79+
})
7080
: {};
7181

7282
const peerDeps = options.includePeerDeps
73-
? getGraphDependencies(pkgJson.peerDependencies || {}, false)
83+
? getGraphDependencies(pkgJson.peerDependencies || {}, { isDev: false })
7484
: {};
7585

7686
const deps = { ...prodDeps, ...optionalDeps, ...peerDeps };
@@ -108,11 +118,18 @@ export const getTopLevelDeps = (
108118
*/
109119
export const getGraphDependencies = (
110120
dependencies: Record<string, string>,
111-
isDev,
121+
options: {
122+
isDev: boolean;
123+
isOptional?: boolean;
124+
},
112125
): Dependencies => {
113126
return Object.entries(dependencies).reduce(
114127
(pnpmDeps: Dependencies, [name, semver]) => {
115-
pnpmDeps[name] = { version: semver, isDev: isDev };
128+
pnpmDeps[name] = {
129+
version: semver,
130+
isDev: options.isDev,
131+
isOptional: options.isOptional || false,
132+
};
116133
return pnpmDeps;
117134
},
118135
{},
@@ -138,6 +155,7 @@ export const getChildNode = (
138155
depInfo: {
139156
version: string;
140157
isDev: boolean;
158+
isOptional?: boolean;
141159
alias?: {
142160
aliasName: string;
143161
aliasTargetDepName: string;
@@ -153,7 +171,18 @@ export const getChildNode = (
153171
let childNode: PkgNode;
154172

155173
if (!pkgs[childNodeKey]) {
156-
if (strictOutOfSync && !/^file:/.test(depInfo.version)) {
174+
// Handle optional dependencies that don't have separate package entries
175+
if (depInfo.isOptional) {
176+
childNode = {
177+
id: childNodeKey,
178+
name: depInfo.alias?.aliasTargetDepName ?? name,
179+
version: depInfo.version,
180+
dependencies: {},
181+
isDev: depInfo.isDev,
182+
missingLockFileEntry: true,
183+
alias: depInfo.alias,
184+
};
185+
} else if (strictOutOfSync && !/^file:/.test(depInfo.version)) {
157186
throw new OutOfSyncError(childNodeKey, LockfileType.yarn);
158187
} else {
159188
childNode = {
@@ -168,12 +197,14 @@ export const getChildNode = (
168197
}
169198
} else {
170199
const depData = pkgs[childNodeKey];
171-
const dependencies = getGraphDependencies(
172-
depData.dependencies || {},
173-
depInfo.isDev,
174-
);
200+
const dependencies = getGraphDependencies(depData.dependencies || {}, {
201+
isDev: depInfo.isDev,
202+
});
175203
const optionalDependencies = includeOptionalDeps
176-
? getGraphDependencies(depData.optionalDependencies || {}, depInfo.isDev)
204+
? getGraphDependencies(depData.optionalDependencies || {}, {
205+
isDev: depInfo.isDev,
206+
isOptional: true,
207+
})
177208
: {};
178209
childNode = {
179210
id: `${name}@${depData.version}`,

‎lib/dep-graph-builders/yarn-lock-v2/utils.ts‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -188,12 +188,14 @@ export const getYarnLockV2ChildNode = (
188188
optionalDependencies,
189189
} = pkgs[childNodeKeyFromResolution];
190190

191-
const formattedDependencies = getGraphDependencies(
192-
dependencies || {},
193-
depInfo.isDev,
194-
);
191+
const formattedDependencies = getGraphDependencies(dependencies || {}, {
192+
isDev: depInfo.isDev,
193+
});
195194
const formattedOptionalDependencies = includeOptionalDeps
196-
? getGraphDependencies(optionalDependencies || {}, depInfo.isDev)
195+
? getGraphDependencies(optionalDependencies || {}, {
196+
isDev: depInfo.isDev,
197+
isOptional: true,
198+
})
197199
: {};
198200

199201
return {
@@ -231,12 +233,14 @@ export const getYarnLockV2ChildNode = (
231233
}
232234
} else {
233235
const depData = pkgs[childNodeKey];
234-
const dependencies = getGraphDependencies(
235-
depData.dependencies || {},
236-
depInfo.isDev,
237-
);
236+
const dependencies = getGraphDependencies(depData.dependencies || {}, {
237+
isDev: depInfo.isDev,
238+
});
238239
const optionalDependencies = includeOptionalDeps
239-
? getGraphDependencies(depData.optionalDependencies || {}, depInfo.isDev)
240+
? getGraphDependencies(depData.optionalDependencies || {}, {
241+
isDev: depInfo.isDev,
242+
isOptional: true,
243+
})
240244
: {};
241245
return {
242246
id: `${name}@${depData.version}`,

‎test/jest/dep-graph-builders/fixtures/npm-lock-v2/missing-optional-dep-minimal/expected.json‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
{
2+
"schemaVersion": "1.3.0",
3+
"pkgManager": {
4+
"name": "npm"
5+
},
6+
"pkgs": [
7+
{
8+
"id": "[email protected]",
9+
"info": {
10+
"name": "test-optional-deps",
11+
"version": "1.0.0"
12+
}
13+
},
14+
{
15+
"id": "@parcel/[email protected]",
16+
"info": {
17+
"name": "@parcel/watcher",
18+
"version": "2.5.1"
19+
}
20+
},
21+
{
22+
"id": "@parcel/[email protected]",
23+
"info": {
24+
"name": "@parcel/watcher-darwin-x64",
25+
"version": "2.5.1"
26+
}
27+
},
28+
{
29+
"id": "@parcel/[email protected]",
30+
"info": {
31+
"name": "@parcel/watcher-darwin-arm64",
32+
"version": "2.5.1"
33+
}
34+
},
35+
{
36+
"id": "@parcel/[email protected]",
37+
"info": {
38+
"name": "@parcel/watcher-linux-x64-glibc",
39+
"version": "2.5.1"
40+
}
41+
}
42+
],
43+
"graph": {
44+
"rootNodeId": "root-node",
45+
"nodes": [
46+
{
47+
"nodeId": "root-node",
48+
"pkgId": "[email protected]",
49+
"deps": [
50+
{
51+
"nodeId": "@parcel/[email protected]"
52+
}
53+
]
54+
},
55+
{
56+
"nodeId": "@parcel/[email protected]",
57+
"pkgId": "@parcel/[email protected]",
58+
"deps": [
59+
{
60+
"nodeId": "@parcel/[email protected]"
61+
},
62+
{
63+
"nodeId": "@parcel/[email protected]"
64+
},
65+
{
66+
"nodeId": "@parcel/[email protected]"
67+
}
68+
],
69+
"info": {
70+
"labels": {
71+
"scope": "prod"
72+
}
73+
}
74+
},
75+
{
76+
"nodeId": "@parcel/[email protected]",
77+
"pkgId": "@parcel/[email protected]",
78+
"deps": [],
79+
"info": {
80+
"labels": {
81+
"scope": "prod",
82+
"missingLockFileEntry": "true"
83+
}
84+
}
85+
},
86+
{
87+
"nodeId": "@parcel/[email protected]",
88+
"pkgId": "@parcel/[email protected]",
89+
"deps": [],
90+
"info": {
91+
"labels": {
92+
"scope": "prod",
93+
"missingLockFileEntry": "true"
94+
}
95+
}
96+
},
97+
{
98+
"nodeId": "@parcel/[email protected]",
99+
"pkgId": "@parcel/[email protected]",
100+
"deps": [],
101+
"info": {
102+
"labels": {
103+
"scope": "prod",
104+
"missingLockFileEntry": "true"
105+
}
106+
}
107+
}
108+
]
109+
}
110+
}

‎test/jest/dep-graph-builders/fixtures/npm-lock-v2/missing-optional-dep-minimal/package-lock.json‎

Lines changed: 25 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/jest/dep-graph-builders/fixtures/npm-lock-v2/missing-optional-dep-minimal/package.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"name": "test-optional-deps",
3+
"version": "1.0.0",
4+
"description": "Minimal test for optional dependencies without separate package entries",
5+
"dependencies": {
6+
"@parcel/watcher": "^2.4.1"
7+
}
8+
}

‎test/jest/dep-graph-builders/npm-lock-v2.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ describe('dep-graph-builder npm-lock-v2', () => {
2323
'local-pkg-without-workspaces',
2424
'dist-tag-sub-dependency',
2525
'bundled-top-level-dep',
26+
'missing-optional-dep-minimal',
2627
])('[simple tests] project: %s ', (fixtureName) => {
2728
it('matches expected', async () => {
2829
const pkgJsonContent = readFileSync(

0 commit comments

Comments
 (0)