Merge pull request #185 from snyk/fix/correctly-index-pkg-lock-with-p…

…kgs-with-similar-candidates

fix: correctly index pkg lock with pkgs with similar candidates

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

@@ -93,6 +93,7 @@ export const buildDepGraphNpmLockV2 = (
     visitedMap,
     npmLockPkgs,
     strictOutOfSync,
+    includeDevDeps,
     includeOptionalDeps,
     [],
     pkgKeysByName,
@@ -106,6 +107,7 @@ const dfsVisit = (
   visitedMap: Set<string>,
   npmLockPkgs: Record<string, NpmLockPkg>,
   strictOutOfSync: boolean,
+  includeDevDeps: boolean,
   includeOptionalDeps: boolean,
   ancestry: { name: string; key: string; inBundle: boolean }[],
   pkgKeysByName: Map<string, string[]>,
@@ -118,6 +120,7 @@ const dfsVisit = (
       depInfo,
       npmLockPkgs,
       strictOutOfSync,
+      includeDevDeps,
       includeOptionalDeps,
       [
         ...ancestry,
@@ -138,6 +141,7 @@ const dfsVisit = (
         visitedMap,
         npmLockPkgs,
         strictOutOfSync,
+        includeDevDeps,
         includeOptionalDeps,
         [
           ...ancestry,
@@ -160,6 +164,7 @@ const getChildNode = (
   depInfo: { version: string; isDev: boolean },
   pkgs: Record<string, NpmLockPkg>,
   strictOutOfSync: boolean,
+  includeDevDeps: boolean,
   includeOptionalDeps: boolean,
   ancestry: { name: string; key: string; inBundle: boolean }[],
   pkgKeysByName: Map<string, string[]>,
@@ -204,6 +209,11 @@ const getChildNode = (
     depData.dependencies || {},
     depInfo.isDev,
   );
+
+  const devDependencies = includeDevDeps
+    ? getGraphDependencies(depData.devDependencies || {}, depInfo.isDev)
+    : {};
+
   const optionalDependencies = includeOptionalDeps
     ? getGraphDependencies(depData.optionalDependencies || {}, depInfo.isDev)
     : {};
@@ -212,14 +222,18 @@ const getChildNode = (
     id: `${name}@${depData.version}`,
     name: name,
     version: depData.version,
-    dependencies: { ...dependencies, ...optionalDependencies },
+    dependencies: {
+      ...dependencies,
+      ...devDependencies,
+      ...optionalDependencies,
+    },
     isDev: depInfo.isDev,
     inBundle: depData.inBundle,
     key: childNodeKey,
   };
 };
 
-const getChildNodeKey = (
+export const getChildNodeKey = (
   name: string,
   ancestry: { name: string; key: string; inBundle: boolean }[],
   pkgs: Record<string, NpmLockPkg>,
@@ -299,16 +313,32 @@ const getChildNodeKey = (
     }
   }
 
-  const ancestry_names = ancestry.map((el) => el.name).concat(name);
+  let ancestry_names = ancestry.map((el) => el.name).concat(name);
   while (ancestry_names.length > 0) {
     const possible_key = `node_modules/${ancestry_names.join(
       '/node_modules/',
     )}`;
+
     if (pkgs[possible_key]) {
       return possible_key;
     }
     ancestry_names.shift();
   }
 
+  // This is similar to fetching permutations in the isBundle logic
+  ancestry_names = ancestry.map((el) => el.name).concat(name);
+  const candidateAncestries = candidateKeys.map((el) =>
+    el.replace('node_modules/', '').split('/node_modules/'),
+  );
+  const filteredCandidates = candidateKeys.filter((candidate, idx) => {
+    return candidateAncestries[idx].every((pkg) => {
+      return ancestry_names.includes(pkg);
+    });
+  });
+
+  if (filteredCandidates.length === 1) {
+    return filteredCandidates[0];
+  }
+
   return undefined;
 };

test/jest/dep-graph-builders/npm-lock-v2/get-child-key.test.ts

@@ -0,0 +1,55 @@
+import { getChildNodeKey } from '../../../../lib/dep-graph-builders/npm-lock-v2/index';
+
+describe('npm-lock-v2 getChildNodeKey', () => {
+  it('should filter out candidate keys that have pkgs that are not in ancestry being tested', async () => {
+    const name = 'global-modules';
+    const ancestry = [
+      { name: 'test-root', key: '', inBundle: false },
+      {
+        name: '@storybook/test-runner',
+        key: 'node_modules/@storybook/test-runner',
+        inBundle: false,
+      },
+      {
+        name: 'jest-playwright-preset',
+        key: 'node_modules/@storybook/test-runner/node_modules/jest-playwright-preset',
+        inBundle: false,
+      },
+      {
+        name: 'jest-process-manager',
+        key: 'node_modules/jest-process-manager',
+        inBundle: false,
+      },
+      { name: 'cwd', key: 'node_modules/cwd', inBundle: false },
+      { name: 'find-pkg', key: 'node_modules/find-pkg', inBundle: false },
+      {
+        name: 'find-file-up',
+        key: 'node_modules/find-file-up',
+        inBundle: false,
+      },
+      {
+        name: 'resolve-dir',
+        key: 'node_modules/find-file-up/node_modules/resolve-dir',
+        inBundle: false,
+      },
+    ];
+    const pkgs = {
+      'node_modules/stylelint/node_modules/global-modules': 'exists' as any,
+      'node_modules/find-file-up/node_modules/global-modules': 'exists' as any,
+    };
+    const pkgKeysByName = new Map([
+      [
+        'global-modules',
+        [
+          'node_modules/find-file-up/node_modules/global-modules',
+          'node_modules/stylelint/node_modules/global-modules',
+        ],
+      ],
+    ]);
+
+    const childKey = getChildNodeKey(name, ancestry, pkgs, pkgKeysByName);
+    expect(childKey).toBe(
+      'node_modules/find-file-up/node_modules/global-modules',
+    );
+  });
+});