fix(npm): don't delete lockfiles early (#28472)

renovatebot · Apr 18, 2024 · b357e4d · b357e4d
1 parent d6d1e57
commit b357e4d
Show file tree

Hide file tree

Showing 9 changed files with 67 additions and 110 deletions.
diff --git a/docs/usage/self-hosted-experimental.md b/docs/usage/self-hosted-experimental.md
@@ -32,10 +32,6 @@ Skipping the check will speed things up, but may result in versions being return
 
 If set to any value, Renovate will always paginate requests to GitHub fully, instead of stopping after 10 pages.
 
-## `RENOVATE_REUSE_PACKAGE_LOCK`
-
-If set to "false" (string), Renovate will remove any existing `package-lock.json` before trying to update it.
-
 ## `RENOVATE_USER_AGENT`
 
 If set to any string, Renovate will use this as the `user-agent` it sends with HTTP requests.

diff --git a/lib/modules/manager/npm/extract/index.spec.ts b/lib/modules/manager/npm/extract/index.spec.ts
@@ -1,4 +1,5 @@
 import { codeBlock } from 'common-tags';
+import { extractAllPackageFiles } from '..';
 import { Fixtures } from '../../../../../test/fixtures';
 import { fs } from '../../../../../test/util';
 import { logger } from '../../../../logger';
@@ -980,10 +981,9 @@ describe('modules/manager/npm/extract/index', () => {
   describe('.extractAllPackageFiles()', () => {
     it('runs', async () => {
       fs.readLocalFile.mockResolvedValueOnce(input02Content);
-      const res = await npmExtract.extractAllPackageFiles(
-        defaultExtractConfig,
-        ['package.json'],
-      );
+      const res = await extractAllPackageFiles(defaultExtractConfig, [
+        'package.json',
+      ]);
       expect(res).toEqual([
         {
           deps: [

diff --git a/lib/modules/manager/npm/post-update/index.spec.ts b/lib/modules/manager/npm/post-update/index.spec.ts
@@ -204,22 +204,6 @@ describe('modules/manager/npm/post-update/index', () => {
       expect(git.getFile).toHaveBeenCalledOnce();
     });
 
-    it('works no reuse lockfiles', async () => {
-      await expect(
-        writeExistingFiles(
-          { ...updateConfig, reuseLockFiles: false },
-          additionalFiles,
-        ),
-      ).resolves.toBeUndefined();
-
-      expect(fs.writeLocalFile).toHaveBeenCalledOnce();
-      expect(fs.deleteLocalFile.mock.calls).toEqual([
-        ['package-lock.json'],
-        ['yarn.lock'],
-        ['yarn.lock'],
-      ]);
-    });
-
     it('writes .npmrc files', async () => {
       await writeExistingFiles(updateConfig, {
         npm: [

diff --git a/lib/modules/manager/npm/post-update/index.ts b/lib/modules/manager/npm/post-update/index.ts
@@ -141,91 +141,76 @@ export async function writeExistingFiles(
     const npmLock = packageFile.managerData.npmLock;
     if (npmLock) {
       const npmLockPath = npmLock;
-      if (
-        process.env.RENOVATE_REUSE_PACKAGE_LOCK === 'false' ||
-        config.reuseLockFiles === false
-      ) {
-        logger.debug(`Ensuring ${npmLock} is removed`);
-        await deleteLocalFile(npmLockPath);
-      } else {
-        logger.debug(`Writing ${npmLock}`);
-        let existingNpmLock: string;
-        try {
-          existingNpmLock = (await getFile(npmLock)) ?? '';
-        } catch (err) /* istanbul ignore next */ {
-          logger.warn({ err }, 'Error reading npm lock file');
-          existingNpmLock = '';
-        }
-        const { detectedIndent, lockFileParsed: npmLockParsed } =
-          parseLockFile(existingNpmLock);
-        if (npmLockParsed) {
-          const packageNames =
-            'packages' in npmLockParsed
-              ? Object.keys(npmLockParsed.packages)
-              : [];
-          const widens: string[] = [];
-          let lockFileChanged = false;
-          for (const upgrade of config.upgrades) {
-            if (upgrade.lockFiles && !upgrade.lockFiles.includes(npmLock)) {
-              continue;
-            }
-            if (!upgrade.managerData) {
-              continue;
-            }
+      logger.debug(`Writing ${npmLock}`);
+      let existingNpmLock: string;
+      try {
+        existingNpmLock = (await getFile(npmLock)) ?? '';
+      } catch (err) /* istanbul ignore next */ {
+        logger.warn({ err }, 'Error reading npm lock file');
+        existingNpmLock = '';
+      }
+      const { detectedIndent, lockFileParsed: npmLockParsed } =
+        parseLockFile(existingNpmLock);
+      if (npmLockParsed) {
+        const packageNames =
+          'packages' in npmLockParsed
+            ? Object.keys(npmLockParsed.packages)
+            : [];
+        const widens: string[] = [];
+        let lockFileChanged = false;
+        for (const upgrade of config.upgrades) {
+          if (upgrade.lockFiles && !upgrade.lockFiles.includes(npmLock)) {
+            continue;
+          }
+          if (!upgrade.managerData) {
+            continue;
+          }
+          if (
+            upgrade.rangeStrategy === 'widen' &&
+            upgrade.managerData.npmLock === npmLock
+          ) {
+            // TODO #22198
+            widens.push(upgrade.depName!);
+          }
+          const { depName } = upgrade;
+          for (const packageName of packageNames) {
             if (
-              upgrade.rangeStrategy === 'widen' &&
-              upgrade.managerData.npmLock === npmLock
+              'packages' in npmLockParsed &&
+              (packageName === `node_modules/${depName}` ||
+                packageName.startsWith(`node_modules/${depName}/`))
             ) {
-              // TODO #22198
-              widens.push(upgrade.depName!);
-            }
-            const { depName } = upgrade;
-            for (const packageName of packageNames) {
-              if (
-                'packages' in npmLockParsed &&
-                (packageName === `node_modules/${depName}` ||
-                  packageName.startsWith(`node_modules/${depName}/`))
-              ) {
-                logger.trace({ packageName }, 'Massaging out package name');
-                lockFileChanged = true;
-                delete npmLockParsed.packages[packageName];
-              }
+              logger.trace({ packageName }, 'Massaging out package name');
+              lockFileChanged = true;
+              delete npmLockParsed.packages[packageName];
             }
           }
-          if (widens.length) {
-            logger.debug(
-              `Removing ${String(widens)} from ${npmLock} to force an update`,
-            );
-            lockFileChanged = true;
-            try {
-              if (
-                'dependencies' in npmLockParsed &&
-                npmLockParsed.dependencies
-              ) {
-                widens.forEach((depName) => {
-                  // TODO #22198
-                  delete npmLockParsed.dependencies![depName];
-                });
-              }
-            } catch (err) /* istanbul ignore next */ {
-              logger.warn(
-                { npmLock },
-                'Error massaging package-lock.json for widen',
-              );
+        }
+        if (widens.length) {
+          logger.debug(
+            `Removing ${String(widens)} from ${npmLock} to force an update`,
+          );
+          lockFileChanged = true;
+          try {
+            if ('dependencies' in npmLockParsed && npmLockParsed.dependencies) {
+              widens.forEach((depName) => {
+                // TODO #22198
+                delete npmLockParsed.dependencies![depName];
+              });
             }
+          } catch (err) /* istanbul ignore next */ {
+            logger.warn(
+              { npmLock },
+              'Error massaging package-lock.json for widen',
+            );
           }
-          if (lockFileChanged) {
-            logger.debug('Massaging npm lock file before writing to disk');
-            existingNpmLock = composeLockFile(npmLockParsed, detectedIndent);
-          }
-          await writeLocalFile(npmLockPath, existingNpmLock);
         }
+        if (lockFileChanged) {
+          logger.debug('Massaging npm lock file before writing to disk');
+          existingNpmLock = composeLockFile(npmLockParsed, detectedIndent);
+        }
+        await writeLocalFile(npmLockPath, existingNpmLock);
       }
     }
-    const { yarnLock } = packageFile.managerData;
-    if (yarnLock && config.reuseLockFiles === false) {
-      await deleteLocalFile(yarnLock);
-    }
   }
 }
 

diff --git a/lib/modules/manager/npm/post-update/npm.ts b/lib/modules/manager/npm/post-update/npm.ts
@@ -191,7 +191,7 @@ export async function generateLockFile(
       } catch (err) /* istanbul ignore next */ {
         logger.debug(
           { err, lockFileName },
-          'Error removing package-lock.json for lock file maintenance',
+          'Error removing `package-lock.json` for lock file maintenance',
         );
       }
     }

diff --git a/lib/modules/manager/npm/post-update/pnpm.ts b/lib/modules/manager/npm/post-update/pnpm.ts
@@ -111,7 +111,7 @@ export async function generateLockFile(
       } catch (err) /* istanbul ignore next */ {
         logger.debug(
           { err, lockFileName },
-          'Error removing yarn.lock for lock file maintenance',
+          'Error removing `pnpm-lock.yaml` for lock file maintenance',
         );
       }
     }

diff --git a/lib/workers/repository/updates/__snapshots__/generate.spec.ts.snap b/lib/workers/repository/updates/__snapshots__/generate.spec.ts.snap
@@ -25,7 +25,6 @@ exports[`workers/repository/updates/generate generateBranchConfig() handles @typ
   "prettyDepType": "dependency",
   "recreateClosed": false,
   "releaseTimestamp": undefined,
-  "reuseLockFiles": true,
   "upgrades": [
     {
       "branchName": "some-branch",
@@ -108,7 +107,6 @@ exports[`workers/repository/updates/generate generateBranchConfig() handles @typ
   "prettyDepType": "dependency",
   "recreateClosed": false,
   "releaseTimestamp": undefined,
-  "reuseLockFiles": true,
   "upgrades": [
     {
       "branchName": "some-branch",
@@ -186,7 +184,6 @@ exports[`workers/repository/updates/generate generateBranchConfig() handles lock
   "prettyDepType": "dependency",
   "recreateClosed": true,
   "releaseTimestamp": undefined,
-  "reuseLockFiles": true,
   "upgrades": [
     {
       "branchName": "some-branch",
@@ -231,7 +228,6 @@ exports[`workers/repository/updates/generate generateBranchConfig() handles lock
   "prettyNewVersion": "v1.0.1",
   "recreateClosed": false,
   "releaseTimestamp": undefined,
-  "reuseLockFiles": true,
   "upgrades": [
     {
       "branchName": "some-branch",

diff --git a/lib/workers/repository/updates/generate.spec.ts b/lib/workers/repository/updates/generate.spec.ts
@@ -97,7 +97,6 @@ describe('workers/repository/updates/generate', () => {
         lockedVersion: '1.0.0',
         newValue: '^1.0.0',
         newVersion: '1.0.1',
-        reuseLockFiles: true,
         prettyNewVersion: 'v1.0.1',
         upgrades: [
           {

diff --git a/lib/workers/repository/updates/generate.ts b/lib/workers/repository/updates/generate.ts
@@ -338,9 +338,6 @@ export function generateBranchConfig(
     ...config.upgrades[0],
     releaseTimestamp: releaseTimestamp!,
   }; // TODO: fixme (#9666)
-  config.reuseLockFiles = config.upgrades.every(
-    (upgrade) => upgrade.updateType !== 'lockFileMaintenance',
-  );
   config.dependencyDashboardApproval = config.upgrades.some(
     (upgrade) => upgrade.dependencyDashboardApproval,
   );