fix(version): load & write project root lockfile v2 only once

- previous code was opening the project root lockfile over and over (for every workspace package) and this was causing perf issue but also in some cases potential racing condition because loading/writing the json are async when that should only be done just once (loading first, looping through all pkg, then writing final json once)

packages/version/src/__tests__/__snapshots__/version-command.spec.ts.snap

@@ -527,3 +527,61 @@ index SHA..SHA 100644
 -    \\"package-4\\": \\"^1.0.0\\"
 +    \\"package-4\\": \\"^2.0.0\\""
 `;
+
+exports[`VersionCommand with lockfile version 2 should have updated project root lockfile version 2 for every necessary properties 1`] = `
+Object {
+  "dependencies": Object {
+    "@my-workspace/package-1": Object {
+      "requires": Object {
+        "tiny-tarball": "^1.0.0",
+      },
+      "version": "file:packages/package-1",
+    },
+    "@my-workspace/package-2": Object {
+      "requires": Object {
+        "@my-workspace/package-1": "^3.0.0",
+      },
+      "version": "file:packages/package-2",
+    },
+  },
+  "lockfileVersion": 2,
+  "name": "my-workspace",
+  "packages": Object {
+    "": Object {
+      "license": "MIT",
+      "name": "my-workspace",
+      "workspaces": Array [
+        "./packages/package-1",
+        "./packages/package-2",
+      ],
+    },
+    "node_modules/package-1": Object {
+      "link": true,
+      "resolved": "packages/package-1",
+    },
+    "node_modules/package-2": Object {
+      "link": true,
+      "resolved": "packages/package-2",
+    },
+    "packages/package-1": Object {
+      "license": "MIT",
+      "name": "@my-workspace/package-1",
+      "tiny-tarball": Object {
+        "integrity": "sha1-u/EC1a5zr+LFUyleD7AiMCFvZbE=",
+        "resolved": "https://registry.npmjs.org/tiny-tarball/-/tiny-tarball-1.0.0.tgz",
+        "version": "1.0.0",
+      },
+      "version": "3.0.0",
+    },
+    "packages/package-2": Object {
+      "dependencies": Object {
+        "@my-workspace/package-1": "^3.0.0",
+      },
+      "license": "MIT",
+      "name": "@my-workspace/package-2",
+      "version": "3.0.0",
+    },
+  },
+  "requires": true,
+}
+`;

packages/version/src/__tests__/update-lockfile-version.spec.js

@@ -10,7 +10,7 @@ const loadJsonFile = require("load-json-file");
 const { getPackages } = require('../../../core/src/project');
 const initFixture = require("@lerna-test/init-fixture")(__dirname);
 
-const { updateClassicLockfileVersion, updateModernLockfileVersion } = require("../lib/update-lockfile-version");
+const { loadPackageLockFileWhenExists, updateClassicLockfileVersion, updateTempModernLockfileVersion, saveUpdatedLockJsonFile } = require("../lib/update-lockfile-version");
 
 describe('npm classic lock file', () => {
   test("updateLockfileVersion with lockfile v1", async () => {
@@ -63,10 +63,13 @@ describe('npm modern lock file', () => {
     const rootLockFilePath = path.join(cwd, "package-lock.json");
     const packages = await getPackages(cwd);
 
-    for (const pkg of packages) {
-      pkg.version = mockVersion;
-      const returnedLockfilePath = await updateModernLockfileVersion(pkg, { rootPath: cwd });
-      expect(returnedLockfilePath).toBe(rootLockFilePath);
+    const lockFileOutput = await loadPackageLockFileWhenExists(cwd);
+    if (lockFileOutput.json) {
+      for (const pkg of packages) {
+        pkg.version = mockVersion;
+        await updateTempModernLockfileVersion(pkg, lockFileOutput.json);
+      }
+      await saveUpdatedLockJsonFile(lockFileOutput.path, lockFileOutput.json);
     }
 
     expect(Array.from(loadJsonFile.registry.keys())).toStrictEqual(["/packages/package-1", "/packages/package-2", "/"]);

packages/version/src/__tests__/version-command.spec.ts

@@ -51,6 +51,7 @@ const { getCommitMessage } = require("@lerna-test/get-commit-message");
 // test command
 import { VersionCommand } from '../version-command';
 const lernaVersion = require("@lerna-test/command-runner")(require("../../../cli/src/cli-commands/cli-version-commands"));
+import { loadPackageLockFileWhenExists } from '../lib/update-lockfile-version';
 
 // file under test
 const yargParser = require('yargs-parser');
@@ -802,6 +803,32 @@ describe("VersionCommand", () => {
     });
   });
 
+  describe('with lockfile version 2', () => {
+    it("should have updated project root lockfile version 2 for every necessary properties", async () => {
+      const cwd = await initFixture("lockfile-version2");
+      await new VersionCommand(createArgv(cwd, "--bump", "major", "--yes"));
+
+      expect(writePkg.updatedVersions()).toEqual({
+        "@my-workspace/package-1": "3.0.0",
+        "@my-workspace/package-2": "3.0.0",
+      });
+
+      const lockfileResponse = await loadPackageLockFileWhenExists(cwd);
+
+      expect(lockfileResponse.json.lockfileVersion).toBe(2);
+      expect(lockfileResponse.json.dependencies['@my-workspace/package-2'].requires).toMatchObject({
+        '@my-workspace/package-1': '^3.0.0',
+      });
+      expect(lockfileResponse.json.packages['packages/package-1'].version).toBe('3.0.0');
+      expect(lockfileResponse.json.packages['packages/package-2'].version).toBe('3.0.0');
+      expect(lockfileResponse.json.packages['packages/package-2'].dependencies).toMatchObject({
+        '@my-workspace/package-1': '^3.0.0',
+      });
+
+      expect(lockfileResponse.json).toMatchSnapshot();
+    });
+  });
+
   describe("with spurious -- arguments", () => {
     it("ignores the extra arguments with cheesy parseConfiguration()", async () => {
       const cwd = await initFixture("lifecycle");

packages/version/src/__tests__/version-lifecycle-scripts.spec.ts

@@ -82,7 +82,7 @@ describe("lifecycle scripts", () => {
     expect(Array.from(loadJsonFile.registry.keys())).toStrictEqual([
       "/packages/package-1",
       "/packages/package-2",
-      "/" // TODO: investigate why the original Lerna doesn't have this extra one in the root
+      "/" // `package-lock.json` project root location
     ]);
   });
 

packages/version/src/lib/update-lockfile-version.ts

@@ -1,14 +1,33 @@
 import path from 'path';
 import loadJsonFile from 'load-json-file';
 import writeJsonFile from 'write-json-file';
-import { Package, Project } from '@lerna-lite/core';
+import { Package } from '@lerna-lite/core';
+
+/**
+ * From a folder path provided, try to load a `package-lock.json` file if it exists.
+ * @param {String} lockFileFolderPath
+ * @returns Promise<{path: string; json: Object; lockFileVersion: number; }>
+ */
+export async function loadPackageLockFileWhenExists<T = any>(lockFileFolderPath: string) {
+  try {
+    const lockFilePath = path.join(lockFileFolderPath, 'package-lock.json');
+    const pkgLockFileObj = await loadJsonFile<T>(lockFilePath);
+    const lockfileVersion = +(pkgLockFileObj?.['lockfileVersion'] ?? 1);
+
+    return {
+      path: lockFilePath,
+      json: pkgLockFileObj,
+      lockfileVersion
+    };
+  } catch (error) { } // eslint-disable-line
+}
 
 /**
  * Update NPM Lock File (when found), the lock file might be version 1 (exist in package folder) or version 2 (exist in workspace root)
  * Depending on the version type, the structure of the lock file will be different and will be updated accordingly
  * @param {Object} pkg
  * @param {Object} project
- * @returns Promise
+ * @returns Promise<string>
  */
 export async function updateClassicLockfileVersion(pkg: Package): Promise<string | undefined> {
   try {
@@ -33,25 +52,38 @@ export async function updateClassicLockfileVersion(pkg: Package): Promise<string
   } catch (error) { } // eslint-disable-line
 }
 
-export async function updateModernLockfileVersion(pkg: Package, project: Project): Promise<string | undefined> {
-  try {
-    // OR "lockfileVersion" >= 2 in the project root, will have a global package lock file located in the root folder and is formatted
-    const projFilePath = path.join(project.rootPath, 'package-lock.json');
-    const projLockFileObj: any = await loadJsonFile(projFilePath);
+/**
+ * Update NPM Lock File (when found), the lock file must be version 2 or higher and is considered as modern lockfile,
+ * its structure is different and all version properties will be updated accordingly
+ * @param {Object} pkg
+ * @param {Object} project
+ * @returns Promise<string>
+ */
+export function updateTempModernLockfileVersion(pkg: Package, projLockFileObj: any) {
+  // OR "lockfileVersion" >= 2 in the project root, will have a global package lock file located in the root folder and is formatted
+  if (projLockFileObj) {
+    updateNpmLockFileVersion2(projLockFileObj, pkg.name, pkg.version);
+  }
+}
 
-    if (projLockFileObj) {
-      updateNpmLockFileVersion2(projLockFileObj, pkg.name, pkg.version);
-      await writeJsonFile(projFilePath, projLockFileObj, {
-        detectIndent: true,
-        indent: 2,
-      });
-      return projFilePath;
-    }
+/**
+ * Save a lockfile by providing a full path and an updated json object
+ * @param {String} filePath
+ * @param {Object} updateLockFileObj
+ * @returns Promise<String | undefined> - file path will be returned when it was found and updated
+ */
+export async function saveUpdatedLockJsonFile(filePath: string, updateLockFileObj: any): Promise<string | undefined> {
+  try {
+    await writeJsonFile(filePath, updateLockFileObj, {
+      detectIndent: true,
+      indent: 2,
+    });
+    return filePath;
   } catch (error) { } // eslint-disable-line
 }
 
 /**
- * Update workspace root NPM Lock File Version Type 2
+ * Update workspace root NPM Lock File Version Type 2 (considerd modern lockfile)
  * @param {Object} obj
  * @param {String} pkgName
  * @param {String} newVersion

packages/version/src/version-command.ts

@@ -37,7 +37,12 @@ import { gitCommit } from './lib/git-commit';
 import { gitTag } from './lib/git-tag';
 import { gitPush } from './lib/git-push';
 import { makePromptVersion } from './lib/prompt-version';
-import { updateClassicLockfileVersion, updateModernLockfileVersion } from './lib/update-lockfile-version';
+import {
+  loadPackageLockFileWhenExists,
+  updateClassicLockfileVersion,
+  updateTempModernLockfileVersion,
+  saveUpdatedLockJsonFile
+} from './lib/update-lockfile-version';
 
 export function factory(argv) {
   return new VersionCommand(argv);
@@ -541,18 +546,14 @@ export class VersionCommand extends Command {
 
         return Promise.all([
           updateClassicLockfileVersion(pkg),
-          updateModernLockfileVersion(pkg, this.project),
           pkg.serialize()
-        ]).then(([lockfilePath, rootLockfilePath]) => {
+        ]).then(([lockfilePath]) => {
           // commit the updated manifest
           changedFiles.add(pkg.manifestLocation);
 
           if (lockfilePath) {
             changedFiles.add(lockfilePath);
           }
-          if (rootLockfilePath) {
-            changedFiles.add(rootLockfilePath);
-          }
 
           return pkg;
         });
@@ -598,6 +599,27 @@ export class VersionCommand extends Command {
       })
     );
 
+    chain = chain.then(() => {
+      // update modern lockfile (version 2 or higher) when exist in the project root
+      loadPackageLockFileWhenExists(rootPath)
+        .then(lockFileResponse => {
+          if (lockFileResponse && lockFileResponse.lockfileVersion >= 2) {
+            for (const pkg of this.packagesToVersion) {
+              this.logger.silly(`lock`, `updating root "package-lock-json" for package "${pkg.name}"`);
+              updateTempModernLockfileVersion(pkg, lockFileResponse.json);
+            }
+
+            // save the lockfile, only once, after all package versions were updated
+            saveUpdatedLockJsonFile(lockFileResponse.path, lockFileResponse.json)
+              .then((lockfilePath) => {
+                if (lockfilePath) {
+                  changedFiles.add(lockfilePath);
+                }
+              });
+          }
+        });
+    });
+
     if (!independentVersions) {
       this.project.version = this.globalVersion;