fix(cli): #1719 correct dynamic route segment key parsing and fail fast on multi-segment routes

packages/cli/src/lib/url-utils.js

@@ -1,12 +1,12 @@
 // get the dynamic segments from a dynamic route, e.g. pages/blog/[slug].js
-function getDynamicSegmentsFromRoute({ route, relativePagePath, extension }) {
-  const dynamicRoute = route.replace("[", ":").replace("]", "");
-  const segmentKey = relativePagePath
-    .split("/")
-    [relativePagePath.split("/").length - 1].replace(extension, "")
-    .replace("[", "")
-    .replace("]", "")
-    .replace(".", "");
+function getDynamicSegmentsFromRoute({ route }) {
+  // convert every bracket pair so a nested route doesn't leak a literal "[..]" into the URLPattern
+  // https://github.com/ProjectEvergreen/greenwood/issues/1719
+  const dynamicRoute = route.replace(/\[([^\]]+)\]/g, ":$1");
+  // derive the key from the bracket in the route itself; stripping the extension substring
+  // mangles params whose name contains it (e.g. [json].js -> "onjs", [posts].ts)
+  // https://github.com/ProjectEvergreen/greenwood/issues/1719
+  const segmentKey = route.match(/\[([^\]]+)\]/)?.[1] ?? "";
 
   return { segmentKey, dynamicRoute };
 }

packages/cli/src/lifecycles/graph.js

@@ -111,12 +111,7 @@ const generateGraph = async (compilation) => {
           // is there a better way to detect for isolation export without having to actually import() the module?
           const contents = await fs.readFile(filenameUrl, "utf8");
           const isolation = contents.indexOf("export const isolation = true;") >= 0;
-          const { segmentKey, dynamicRoute } = getDynamicSegmentsFromRoute({
-            route,
-            relativePagePath,
-            extension,
-            basePath,
-          });
+          const { segmentKey, dynamicRoute } = getDynamicSegmentsFromRoute({ route });
 
           /*
            * API Properties (per route)
@@ -276,12 +271,16 @@ const generateGraph = async (compilation) => {
            * hasStaticParams: if getStaticProps is present on the route
            */
 
-          const { segmentKey, dynamicRoute } = getDynamicSegmentsFromRoute({
-            route,
-            relativePagePath,
-            extension,
-            basePath,
-          });
+          const { segmentKey, dynamicRoute } = getDynamicSegmentsFromRoute({ route });
+
+          // multiple dynamic segments (e.g. [a]/[b].js) are not supported yet; fail fast with a
+          // clear message instead of emitting literal "[a]" dirs then a downstream TypeError
+          // https://github.com/ProjectEvergreen/greenwood/issues/1719
+          if ((route.match(/\[[^\]]+\]/g) ?? []).length > 1) {
+            throw new Error(
+              `Multiple dynamic route segments are not currently supported (found in ${relativePagePath.replace("./", "")}). Use a single [param] per route.`,
+            );
+          }
 
           const page = {
             id: decodeURIComponent(getIdFromRelativePathPath(relativePagePath, extension)),

packages/cli/test/cases/build.default.dynamic-routing-multi-segment/build.default.dynamic-routing-multi-segment.spec.js

@@ -0,0 +1,52 @@
+/*
+ * Use Case
+ * Run Greenwood build command for a route with more than one dynamic segment (e.g. [a]/[b].js).
+ *
+ * User Result
+ * Should fail fast at graph build time with a clear, actionable error instead of
+ * emitting literal /[a]/ directories and then crashing with an unrelated TypeError.
+ *
+ * User Command
+ * greenwood build
+ *
+ * User Config
+ * None
+ *
+ * User Workspace
+ *  src/
+ *   pages/
+ *     [a]/
+ *       [b].js
+ */
+import * as chai from "chai";
+import { expect } from "chai";
+import chaiAsPromised from "chai-as-promised";
+import path from "node:path";
+import { Runner } from "gallinago";
+import { fileURLToPath } from "node:url";
+
+chai.use(chaiAsPromised);
+
+// https://github.com/ProjectEvergreen/greenwood/issues/1719
+describe("Build Greenwood With: ", function () {
+  const cliPath = path.join(process.cwd(), "packages/cli/src/bin.js");
+  const outputPath = fileURLToPath(new URL(".", import.meta.url));
+  let runner;
+
+  before(function () {
+    this.context = {
+      publicDir: path.join(outputPath, "public"),
+    };
+    runner = new Runner();
+  });
+
+  describe("A route with more than one dynamic segment", function () {
+    it("should throw a clear error that multiple dynamic route segments are not supported", async function () {
+      await runner.setup(outputPath);
+
+      await expect(runner.runCommand(cliPath, "build")).to.be.rejectedWith(
+        "Multiple dynamic route segments are not currently supported",
+      );
+    });
+  });
+});

packages/cli/test/cases/build.default.dynamic-routing-multi-segment/src/pages/[a]/[b].js

@@ -0,0 +1,7 @@
+export async function getStaticPaths() {
+  return [{ params: { a: "x", b: "1" } }, { params: { a: "y", b: "2" } }];
+}
+
+export async function getBody(compilation, page, request, params) {
+  return `<h2>params=${JSON.stringify(params)}</h2>`;
+}

packages/cli/test/cases/build.default.dynamic-routing-param-name-ext/build.default.dynamic-routing-param-name-ext.spec.js

@@ -0,0 +1,86 @@
+/*
+ * Use Case
+ * Run Greenwood build command for a dynamic route whose param name contains the
+ * file extension as a substring (e.g. [json].js), returning static paths from getStaticPaths.
+ *
+ * User Result
+ * Should generate the static path with the correct param name / value, emitting
+ * /alpha/index.html (not a literal /[json]/index.html with garbage params).
+ *
+ * User Command
+ * greenwood build
+ *
+ * User Config
+ * None
+ *
+ * User Workspace
+ *  src/
+ *   pages/
+ *     [json].js
+ */
+import { expect } from "chai";
+import fs from "node:fs/promises";
+import { JSDOM } from "jsdom";
+import path from "node:path";
+import { getOutputTeardownFiles } from "../../../../../test/utils.js";
+import { Runner } from "gallinago";
+import { fileURLToPath } from "node:url";
+
+// https://github.com/ProjectEvergreen/greenwood/issues/1719
+describe("Build Greenwood With: ", function () {
+  const LABEL = "A dynamic route whose param name contains the file extension substring";
+  const cliPath = path.join(process.cwd(), "packages/cli/src/bin.js");
+  const outputPath = fileURLToPath(new URL(".", import.meta.url));
+  let runner;
+
+  before(function () {
+    this.context = {
+      publicDir: path.join(outputPath, "public"),
+    };
+    runner = new Runner();
+  });
+
+  describe(LABEL, function () {
+    before(async function () {
+      await runner.setup(outputPath);
+      await runner.runCommand(cliPath, "build");
+    });
+
+    describe("Static generation for the [json].js dynamic route", function () {
+      let dom;
+
+      before(async function () {
+        const html = await fs.readFile(path.join(outputPath, "public/alpha/index.html"), "utf-8");
+        dom = new JSDOM(html);
+      });
+
+      it("should emit the static path at /alpha/index.html", async function () {
+        const contents = await fs.readFile(
+          path.join(outputPath, "public/alpha/index.html"),
+          "utf-8",
+        );
+
+        expect(contents).to.not.equal(undefined);
+      });
+
+      it("should NOT emit a literal /[json]/index.html directory", async function () {
+        const files = await Array.fromAsync(
+          fs.glob("[[]json[]]/index.html", { cwd: new URL("./public/", import.meta.url) }),
+        );
+
+        expect(files.length).to.equal(0);
+      });
+
+      it("should pass the correct param name and value to getBody", function () {
+        const headings = dom.window.document.querySelectorAll("body > h2");
+
+        expect(headings.length).to.equal(1);
+        expect(headings[0].textContent).to.equal('params={"json":"alpha"}');
+      });
+    });
+  });
+
+  after(async function () {
+    await runner.teardown(getOutputTeardownFiles(outputPath));
+  });
+});

packages/cli/test/cases/build.default.dynamic-routing-param-name-ext/src/pages/[json].js

@@ -0,0 +1,7 @@
+export async function getStaticPaths() {
+  return [{ params: { json: "alpha" } }];
+}
+
+export async function getBody(compilation, page, request, params) {
+  return `<h2>params=${JSON.stringify(params)}</h2>`;
+}