From abfdae3c88e9fb3015ff79984dd2d2a9ca220f34 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Tue, 30 Jun 2020 12:05:44 +0200
Subject: [PATCH] refactor(npm): skip internal packages earlier (#6639)

---
 lib/manager/common.ts                         |  2 +-
 .../__snapshots__/monorepo.spec.ts.snap       | 57 ++++++++++---------
 lib/manager/npm/extract/monorepo.spec.ts      | 29 +++++++++-
 lib/manager/npm/extract/monorepo.ts           | 18 ++++--
 .../process/__snapshots__/fetch.spec.ts.snap  |  8 ---
 lib/workers/repository/process/fetch.spec.ts  |  8 +--
 lib/workers/repository/process/fetch.ts       | 10 +---
 7 files changed, 71 insertions(+), 61 deletions(-)

diff --git a/lib/manager/common.ts b/lib/manager/common.ts
index 63543e2547..5d4319e4d0 100644
--- a/lib/manager/common.ts
+++ b/lib/manager/common.ts
@@ -76,7 +76,7 @@ export interface PackageFile<T = Record<string, any>>
   extends NpmLockFiles,
     ManagerData<T> {
   hasYarnWorkspaces?: boolean;
-  internalPackages?: string[];
+  internalPackages?: string[]; // TODO: remove
   compatibility?: Record<string, string>;
   datasource?: string;
   registryUrls?: string[];
diff --git a/lib/manager/npm/extract/__snapshots__/monorepo.spec.ts.snap b/lib/manager/npm/extract/__snapshots__/monorepo.spec.ts.snap
index cb89f93e00..d8f26c155c 100644
--- a/lib/manager/npm/extract/__snapshots__/monorepo.spec.ts.snap
+++ b/lib/manager/npm/extract/__snapshots__/monorepo.spec.ts.snap
@@ -3,19 +3,43 @@
 exports[`manager/npm/extract .extractPackageFile() uses lerna package settings 1`] = `
 Array [
   Object {
-    "internalPackages": Array [
-      "@org/a",
-      "@org/b",
+    "deps": Array [
+      Object {
+        "depName": "@org/a",
+        "skipReason": "internal-package",
+      },
+      Object {
+        "depName": "@org/b",
+        "skipReason": "internal-package",
+      },
+      Object {
+        "depName": "@org/c",
+      },
+      Object {
+        "depName": "foo",
+      },
     ],
     "lernaDir": ".",
     "lernaPackages": Array [
       "packages/*",
     ],
     "packageFile": "package.json",
+    "packages": Array [
+      "packages/*",
+    ],
   },
   Object {
-    "internalPackages": Array [
-      "@org/b",
+    "deps": Array [
+      Object {
+        "depName": "@org/b",
+        "skipReason": "internal-package",
+      },
+      Object {
+        "depName": "@org/c",
+      },
+      Object {
+        "depName": "bar",
+      },
     ],
     "lernaClient": undefined,
     "lernaDir": ".",
@@ -25,9 +49,6 @@ Array [
     "yarnLock": undefined,
   },
   Object {
-    "internalPackages": Array [
-      "@org/a",
-    ],
     "lernaClient": undefined,
     "lernaDir": ".",
     "npmLock": undefined,
@@ -41,10 +62,6 @@ Array [
 exports[`manager/npm/extract .extractPackageFile() uses yarn workspaces package settings with lerna 1`] = `
 Array [
   Object {
-    "internalPackages": Array [
-      "@org/a",
-      "@org/b",
-    ],
     "lernaClient": "yarn",
     "lernaDir": ".",
     "lernaPackages": Array [
@@ -56,9 +73,6 @@ Array [
     ],
   },
   Object {
-    "internalPackages": Array [
-      "@org/b",
-    ],
     "lernaClient": "yarn",
     "lernaDir": ".",
     "npmLock": undefined,
@@ -67,9 +81,6 @@ Array [
     "yarnLock": undefined,
   },
   Object {
-    "internalPackages": Array [
-      "@org/a",
-    ],
     "lernaClient": "yarn",
     "lernaDir": ".",
     "npmLock": undefined,
@@ -83,18 +94,11 @@ Array [
 exports[`manager/npm/extract .extractPackageFile() uses yarn workspaces package settings without lerna 1`] = `
 Array [
   Object {
-    "internalPackages": Array [
-      "@org/a",
-      "@org/b",
-    ],
     "packageFile": "package.json",
     "yarnWorkspacesPackages": "packages/*",
   },
   Object {
     "hasYarnWorkspaces": true,
-    "internalPackages": Array [
-      "@org/b",
-    ],
     "lernaClient": undefined,
     "lernaDir": undefined,
     "npmLock": undefined,
@@ -103,9 +107,6 @@ Array [
     "yarnLock": "yarn.lock",
   },
   Object {
-    "internalPackages": Array [
-      "@org/a",
-    ],
     "lernaClient": undefined,
     "lernaDir": undefined,
     "npmLock": undefined,
diff --git a/lib/manager/npm/extract/monorepo.spec.ts b/lib/manager/npm/extract/monorepo.spec.ts
index b99ca7f3c1..ce0ad51ab9 100644
--- a/lib/manager/npm/extract/monorepo.spec.ts
+++ b/lib/manager/npm/extract/monorepo.spec.ts
@@ -8,10 +8,36 @@ describe('manager/npm/extract', () => {
           packageFile: 'package.json',
           lernaDir: '.',
           lernaPackages: ['packages/*'],
+          packages: ['packages/*'],
+          deps: [
+            {
+              depName: '@org/a',
+            },
+            {
+              depName: '@org/b',
+            },
+            {
+              depName: '@org/c',
+            },
+            {
+              depName: 'foo',
+            },
+          ],
         },
         {
           packageFile: 'packages/a/package.json',
           packageJsonName: '@org/a',
+          deps: [
+            {
+              depName: '@org/b',
+            },
+            {
+              depName: '@org/c',
+            },
+            {
+              depName: 'bar',
+            },
+          ],
         },
         {
           packageFile: 'packages/b/package.json',
@@ -21,7 +47,6 @@ describe('manager/npm/extract', () => {
       detectMonorepos(packageFiles);
       expect(packageFiles).toMatchSnapshot();
       expect(packageFiles[1].lernaDir).toEqual('.');
-      expect((packageFiles[1] as any).internalPackages).toEqual(['@org/b']);
     });
     it('uses yarn workspaces package settings with lerna', () => {
       const packageFiles = [
@@ -44,7 +69,6 @@ describe('manager/npm/extract', () => {
       detectMonorepos(packageFiles);
       expect(packageFiles).toMatchSnapshot();
       expect(packageFiles[1].lernaDir).toEqual('.');
-      expect((packageFiles[1] as any).internalPackages).toEqual(['@org/b']);
     });
     it('uses yarn workspaces package settings without lerna', () => {
       const packageFiles = [
@@ -64,7 +88,6 @@ describe('manager/npm/extract', () => {
       ];
       detectMonorepos(packageFiles);
       expect(packageFiles).toMatchSnapshot();
-      expect((packageFiles[1] as any).internalPackages).toEqual(['@org/b']);
     });
   });
 });
diff --git a/lib/manager/npm/extract/monorepo.ts b/lib/manager/npm/extract/monorepo.ts
index 625a199801..2820d34fb6 100644
--- a/lib/manager/npm/extract/monorepo.ts
+++ b/lib/manager/npm/extract/monorepo.ts
@@ -4,6 +4,7 @@ import is from '@sindresorhus/is';
 import minimatch from 'minimatch';
 import upath from 'upath';
 import { logger } from '../../../logger';
+import { SkipReason } from '../../../types';
 import { PackageFile } from '../../common';
 
 function matchesAnyPattern(val: string, patterns: string[]): boolean {
@@ -40,15 +41,15 @@ export function detectMonorepos(packageFiles: Partial<PackageFile>[]): void {
       const internalPackageFiles = packageFiles.filter((sp) =>
         matchesAnyPattern(path.dirname(sp.packageFile), internalPackagePatterns)
       );
-      const internalPackages = internalPackageFiles
+      const internalPackageNames = internalPackageFiles
         .map((sp) => sp.packageJsonName)
         .filter(Boolean);
-      // add all names to main package.json
-      p.internalPackages = internalPackages;
+      p.deps?.forEach((dep) => {
+        if (internalPackageNames.includes(dep.depName)) {
+          dep.skipReason = SkipReason.InternalPackage; // eslint-disable-line no-param-reassign
+        }
+      });
       for (const subPackage of internalPackageFiles) {
-        subPackage.internalPackages = internalPackages.filter(
-          (name) => name !== subPackage.packageJsonName
-        );
         subPackage.lernaDir = lernaDir;
         subPackage.lernaClient = lernaClient;
         subPackage.yarnLock = subPackage.yarnLock || yarnLock;
@@ -56,6 +57,11 @@ export function detectMonorepos(packageFiles: Partial<PackageFile>[]): void {
         if (subPackage.yarnLock) {
           subPackage.hasYarnWorkspaces = !!yarnWorkspacesPackages;
         }
+        subPackage.deps?.forEach((dep) => {
+          if (internalPackageNames.includes(dep.depName)) {
+            dep.skipReason = SkipReason.InternalPackage; // eslint-disable-line no-param-reassign
+          }
+        });
       }
     }
   }
diff --git a/lib/workers/repository/process/__snapshots__/fetch.spec.ts.snap b/lib/workers/repository/process/__snapshots__/fetch.spec.ts.snap
index cda4633d9e..3ab4788ee1 100644
--- a/lib/workers/repository/process/__snapshots__/fetch.spec.ts.snap
+++ b/lib/workers/repository/process/__snapshots__/fetch.spec.ts.snap
@@ -66,11 +66,6 @@ Object {
           "skipReason": "ignored",
           "updates": Array [],
         },
-        Object {
-          "depName": "zzzz",
-          "skipReason": "internal-package",
-          "updates": Array [],
-        },
         Object {
           "depName": "foo",
           "skipReason": "disabled",
@@ -82,9 +77,6 @@ Object {
           "updates": Array [],
         },
       ],
-      "internalPackages": Array [
-        "zzzz",
-      ],
       "packageFile": "package.json",
     },
   ],
diff --git a/lib/workers/repository/process/fetch.spec.ts b/lib/workers/repository/process/fetch.spec.ts
index d71d727ca6..3481de8b1b 100644
--- a/lib/workers/repository/process/fetch.spec.ts
+++ b/lib/workers/repository/process/fetch.spec.ts
@@ -39,11 +39,9 @@ describe('workers/repository/process/fetch', () => {
             packageFile: 'package.json',
             deps: [
               { depName: 'abcd' },
-              { depName: 'zzzz' },
               { depName: 'foo' },
               { depName: 'skipped', skipReason: 'some-reason' as never },
             ],
-            internalPackages: ['zzzz'],
           },
         ],
       };
@@ -51,12 +49,8 @@ describe('workers/repository/process/fetch', () => {
       expect(packageFiles).toMatchSnapshot();
       expect(packageFiles.npm[0].deps[0].skipReason).toEqual('ignored');
       expect(packageFiles.npm[0].deps[0].updates).toHaveLength(0);
-      expect(packageFiles.npm[0].deps[1].skipReason).toEqual(
-        'internal-package'
-      );
+      expect(packageFiles.npm[0].deps[1].skipReason).toEqual('disabled');
       expect(packageFiles.npm[0].deps[1].updates).toHaveLength(0);
-      expect(packageFiles.npm[0].deps[2].skipReason).toEqual('disabled');
-      expect(packageFiles.npm[0].deps[2].updates).toHaveLength(0);
     });
     it('fetches updates', async () => {
       config.rangeStrategy = 'auto';
diff --git a/lib/workers/repository/process/fetch.ts b/lib/workers/repository/process/fetch.ts
index d1be15d04f..237759d6cd 100644
--- a/lib/workers/repository/process/fetch.ts
+++ b/lib/workers/repository/process/fetch.ts
@@ -33,14 +33,8 @@ async function fetchDepUpdates(
   if (depConfig.ignoreDeps.includes(depName)) {
     logger.debug({ dependency: dep.depName }, 'Dependency is ignored');
     dep.skipReason = SkipReason.Ignored;
-  } else if (
-    depConfig.internalPackages &&
-    depConfig.internalPackages.includes(depName)
-  ) {
-    logger.debug(
-      { dependency: dep.depName },
-      'Dependency is ignored due to being internal'
-    );
+  } else if (depConfig.internalPackages?.includes(depName)) {
+    // istanbul ignore next
     dep.skipReason = SkipReason.InternalPackage;
   } else if (depConfig.enabled === false) {
     logger.debug({ dependency: dep.depName }, 'Dependency is disabled');
-- 
GitLab