fix: handle trailing commas and comments in lerna.json files (#3603)

.github/actions/install-node-and-dependencies/action.yml

@@ -13,9 +13,13 @@ runs:
   using: "composite"
   steps:
     - name: Install node and npm based on the given values (or the volta config in package.json)
-      uses: volta-cli/action@v4
+      uses: actions/setup-node@v3
       with:
-        node-version: ${{ inputs.node-version }}
+        node-version-file: "package.json"
+      # The volta action seems to failing to install/unpack a lot recently https://github.com/volta-cli/action/issues/77#issuecomment-1481045178
+      # uses: volta-cli/action@v4
+      # with:
+      #   node-version: ${{ inputs.node-version }}
 
     # Github actions/cache now recommends _not_ caching node_modules when using npm ci
     # https://github.com/actions/cache/blob/940f3d7cf195ba83374c77632d1e2cbb2f24ae68/examples.md#node---npm

libs/core/src/lib/project/__fixtures__/invalid-json/lerna.json → libs/core/src/lib/project/__fixtures__/invalid-lerna-json-irrecoverable/lerna.json

@@ -1,3 +1,4 @@
 {
+  233434
   "version": "1.0.0",
 }

libs/core/src/lib/project/__fixtures__/invalid-json/package.json → libs/core/src/lib/project/__fixtures__/invalid-lerna-json-irrecoverable/package.json

@@ -1,6 +1,6 @@
 {
   "name": "test",
   "devDependencies": {
-    "lerna": "500.0.0",
+    "lerna": "500.0.0"
   }
 }

libs/core/src/lib/project/__fixtures__/invalid-lerna-json-recoverable/lerna.json

@@ -0,0 +1,4 @@
+{
+  // Trailing commas and comments are recoverable
+  "version": "1.0.0",
+}

libs/core/src/lib/project/__fixtures__/invalid-lerna-json-recoverable/package.json

@@ -0,0 +1,6 @@
+{
+  "name": "test",
+  "devDependencies": {
+    "lerna": "500.0.0"
+  }
+}

libs/core/src/lib/project/__fixtures__/invalid-package-json/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "test",
+  "devDependencies": {
+    "lerna": "500.0.0",
+  },
+  "lerna": {
+    "version": "1.0.0"
+  }
+}

libs/core/src/lib/project/index.spec.ts

@@ -5,6 +5,18 @@ import fs from "fs-extra";
 import { dump } from "js-yaml";
 import path from "path";
 
+// Serialize the JSONError output to be more human readable
+expect.addSnapshotSerializer({
+  serialize(str: string) {
+    // eslint-disable-next-line @typescript-eslint/no-var-requires
+    const stripAnsi = require("strip-ansi");
+    return stripAnsi(str).replace(/Error in: .*lerna\.json/, "Error in: normalized/path/to/lerna.json");
+  },
+  test(val: string) {
+    return val != null && typeof val === "string" && val.includes("Error in: ");
+  },
+});
+
 // helpers
 const initFixture = initFixtureFactory(__dirname);
 
@@ -59,15 +71,37 @@ describe("Project", () => {
       expect(new Project().config).toEqual({});
     });
 
-    it("errors when lerna.json is not valid JSON", async () => {
-      const cwd = await initFixture("invalid-json");
+    it("does not error when lerna.json contains trailing commas and/or comments", async () => {
+      const cwd = await initFixture("invalid-lerna-json-recoverable");
+
+      expect(new Project(cwd).config).toMatchInlineSnapshot(`
+        Object {
+          "version": "1.0.0",
+        }
+      `);
+    });
+
+    it("errors when lerna.json is irrecoverably invalid JSON", async () => {
+      const cwd = await initFixture("invalid-lerna-json-irrecoverable");
 
       expect(() => new Project(cwd)).toThrow(
         expect.objectContaining({
           name: "ValidationError",
           prefix: "JSONError",
         })
       );
+
+      expect(() => new Project(cwd)).toThrowErrorMatchingInlineSnapshot(`
+        Error in: normalized/path/to/lerna.json
+        PropertyNameExpected in JSON at 2:3
+          1 | {
+        > 2 |   233434
+            |   ^^^^^^
+          3 |   "version": "1.0.0",
+          4 | }
+          5 | 
+
+      `);
     });
 
     it("returns parsed rootPkg.lerna", async () => {
@@ -447,7 +481,7 @@ describe("Project", () => {
     });
 
     it("errors when root package.json is not valid JSON", async () => {
-      const cwd = await initFixture("invalid-json");
+      const cwd = await initFixture("invalid-package-json");
 
       expect(() => new Project(cwd)).toThrow(
         expect.objectContaining({

libs/core/src/lib/project/index.ts

@@ -1,5 +1,5 @@
-import { writeJsonFile } from "@nrwl/devkit";
-import { cosmiconfigSync } from "cosmiconfig";
+import { parseJson, writeJsonFile } from "@nrwl/devkit";
+import { cosmiconfigSync, defaultLoaders } from "cosmiconfig";
 import dedent from "dedent";
 import fs from "fs";
 import globParent from "glob-parent";
@@ -75,6 +75,29 @@ export class Project {
 
   constructor(cwd?: string) {
     const explorer = cosmiconfigSync("lerna", {
+      loaders: {
+        ...defaultLoaders,
+        ".json": (filepath, content) => {
+          if (!filepath.endsWith("lerna.json")) {
+            return defaultLoaders[".json"](filepath, content);
+          }
+          /**
+           * This prevents lerna from blowing up on trailing commas and comments in lerna configs,
+           * however it should be noted that we will not be able to respect those things whenever
+           * we perform an automated config migration, e.g. via `lerna repair` and they will be lost.
+           * (Although that will be easy enough for the user to see and updated in their `git diff`)
+           */
+          try {
+            return parseJson(content);
+          } catch (err: unknown) {
+            if (err instanceof Error) {
+              err.name = "JSONError";
+              err.message = `Error in: ${filepath}\n${err.message}`;
+            }
+            throw err;
+          }
+        },
+      },
       searchPlaces: ["lerna.json", "package.json"],
       transform(obj) {
         // cosmiconfig returns null when nothing is found