diff --git a/packages/cli/src/lib/url-utils.js b/packages/cli/src/lib/url-utils.js index ee803d833..caab75496 100644 --- a/packages/cli/src/lib/url-utils.js +++ b/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 }; } diff --git a/packages/cli/src/lifecycles/graph.js b/packages/cli/src/lifecycles/graph.js index 1187c53f5..5b6803c16 100644 --- a/packages/cli/src/lifecycles/graph.js +++ b/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)), diff --git a/packages/cli/test/cases/build.default.dynamic-routing-multi-segment/build.default.dynamic-routing-multi-segment.spec.js b/packages/cli/test/cases/build.default.dynamic-routing-multi-segment/build.default.dynamic-routing-multi-segment.spec.js new file mode 100644 index 000000000..ec0344956 --- /dev/null +++ b/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", + ); + }); + }); +}); diff --git a/packages/cli/test/cases/build.default.dynamic-routing-multi-segment/src/pages/[a]/[b].js b/packages/cli/test/cases/build.default.dynamic-routing-multi-segment/src/pages/[a]/[b].js new file mode 100644 index 000000000..4eed4be86 --- /dev/null +++ b/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 `

params=${JSON.stringify(params)}

`; +} diff --git a/packages/cli/test/cases/build.default.dynamic-routing-param-name-ext/build.default.dynamic-routing-param-name-ext.spec.js b/packages/cli/test/cases/build.default.dynamic-routing-param-name-ext/build.default.dynamic-routing-param-name-ext.spec.js new file mode 100644 index 000000000..75252fda8 --- /dev/null +++ b/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)); + }); +}); diff --git a/packages/cli/test/cases/build.default.dynamic-routing-param-name-ext/src/pages/[json].js b/packages/cli/test/cases/build.default.dynamic-routing-param-name-ext/src/pages/[json].js new file mode 100644 index 000000000..972edfb84 --- /dev/null +++ b/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 `

params=${JSON.stringify(params)}

`; +}