From 91d89943cda26a197cb7c8d752d7953a97fac338 Mon Sep 17 00:00:00 2001 From: Mexican-Man Date: Mon, 14 Feb 2022 20:04:14 -0600 Subject: [PATCH] fix: do not merge routes with different methods when computing pages routes Fixes #92 --- .changeset/hungry-pugs-relate.md | 7 + .../pages/functions/filepath-routing.test.ts | 149 ++++++++++++------ .../pages/functions/filepath-routing.ts | 4 +- 3 files changed, 113 insertions(+), 47 deletions(-) create mode 100644 .changeset/hungry-pugs-relate.md diff --git a/.changeset/hungry-pugs-relate.md b/.changeset/hungry-pugs-relate.md new file mode 100644 index 000000000000..873f2e422b9f --- /dev/null +++ b/.changeset/hungry-pugs-relate.md @@ -0,0 +1,7 @@ +--- +"wrangler": patch +--- + +fix: do not merge routes with different methods when computing pages routes + +Fixes #92 diff --git a/packages/wrangler/pages/functions/filepath-routing.test.ts b/packages/wrangler/pages/functions/filepath-routing.test.ts index 52d0a6d2966c..9ba7165840cc 100644 --- a/packages/wrangler/pages/functions/filepath-routing.test.ts +++ b/packages/wrangler/pages/functions/filepath-routing.test.ts @@ -1,60 +1,117 @@ +import { writeFileSync } from "fs"; +import { runInTempDir } from "../../src/__tests__/helpers/run-in-tmp"; import { toUrlPath } from "../../src/paths"; -import { compareRoutes } from "./filepath-routing"; +import { compareRoutes, generateConfigFromFileTree } from "./filepath-routing"; +import type { UrlPath } from "../../src/paths"; import type { HTTPMethod, RouteConfig } from "./routes"; -describe("compareRoutes()", () => { - test("routes / last", () => { - expect( - compareRoutes(routeConfig("/"), routeConfig("/foo")) - ).toBeGreaterThanOrEqual(1); - expect( - compareRoutes(routeConfig("/"), routeConfig("/:foo")) - ).toBeGreaterThanOrEqual(1); - expect( - compareRoutes(routeConfig("/"), routeConfig("/:foo*")) - ).toBeGreaterThanOrEqual(1); - }); +describe("filepath-routing", () => { + describe("compareRoutes()", () => { + test("routes / last", () => { + expect( + compareRoutes(routeConfig("/"), routeConfig("/foo")) + ).toBeGreaterThanOrEqual(1); + expect( + compareRoutes(routeConfig("/"), routeConfig("/:foo")) + ).toBeGreaterThanOrEqual(1); + expect( + compareRoutes(routeConfig("/"), routeConfig("/:foo*")) + ).toBeGreaterThanOrEqual(1); + }); - test("routes with fewer segments come after those with more segments", () => { - expect( - compareRoutes(routeConfig("/foo"), routeConfig("/foo/bar")) - ).toBeGreaterThanOrEqual(1); - expect( - compareRoutes(routeConfig("/foo"), routeConfig("/foo/bar/cat")) - ).toBeGreaterThanOrEqual(1); - }); + test("routes with fewer segments come after those with more segments", () => { + expect( + compareRoutes(routeConfig("/foo"), routeConfig("/foo/bar")) + ).toBeGreaterThanOrEqual(1); + expect( + compareRoutes(routeConfig("/foo"), routeConfig("/foo/bar/cat")) + ).toBeGreaterThanOrEqual(1); + }); - test("routes with wildcard segments come after those without", () => { - expect(compareRoutes(routeConfig("/:foo*"), routeConfig("/foo"))).toBe(1); - expect(compareRoutes(routeConfig("/:foo*"), routeConfig("/:foo"))).toBe(1); - }); + test("routes with wildcard segments come after those without", () => { + expect(compareRoutes(routeConfig("/:foo*"), routeConfig("/foo"))).toBe(1); + expect(compareRoutes(routeConfig("/:foo*"), routeConfig("/:foo"))).toBe( + 1 + ); + }); - test("routes with dynamic segments come after those without", () => { - expect(compareRoutes(routeConfig("/:foo"), routeConfig("/foo"))).toBe(1); - }); + test("routes with dynamic segments come after those without", () => { + expect(compareRoutes(routeConfig("/:foo"), routeConfig("/foo"))).toBe(1); + }); - test("routes with dynamic segments occurring earlier come after those with dynamic segments in later positions", () => { - expect( - compareRoutes(routeConfig("/foo/:id/bar"), routeConfig("/foo/bar/:id")) - ).toBe(1); - }); + test("routes with dynamic segments occurring earlier come after those with dynamic segments in later positions", () => { + expect( + compareRoutes(routeConfig("/foo/:id/bar"), routeConfig("/foo/bar/:id")) + ).toBe(1); + }); - test("routes with no HTTP method come after those specifying a method", () => { - expect(compareRoutes(routeConfig("/foo"), routeConfig("/foo", "GET"))).toBe( - 1 - ); - }); + test("routes with no HTTP method come after those specifying a method", () => { + expect( + compareRoutes(routeConfig("/foo"), routeConfig("/foo", "GET")) + ).toBe(1); + }); + + test("two equal routes are sorted according to their original position in the list", () => { + expect( + compareRoutes(routeConfig("/foo", "GET"), routeConfig("/foo", "GET")) + ).toBe(0); + }); - test("two equal routes are sorted according to their original position in the list", () => { - expect( - compareRoutes(routeConfig("/foo", "GET"), routeConfig("/foo", "GET")) - ).toBe(0); + test("it returns -1 if the first argument should appear first in the list", () => { + expect( + compareRoutes(routeConfig("/foo", "GET"), routeConfig("/foo")) + ).toBe(-1); + }); }); - test("it returns -1 if the first argument should appear first in the list", () => { - expect(compareRoutes(routeConfig("/foo", "GET"), routeConfig("/foo"))).toBe( - -1 - ); + describe("generateConfigFromFileTree", () => { + runInTempDir(); + + it("should generate a route entry for each file in the tree", async () => { + writeFileSync( + "foo.ts", + ` + export function onRequestGet() {} + export function onRequestPost() {} + ` + ); + writeFileSync( + "bar.ts", + ` + export function onRequestPut() {} + export function onRequestDelete() {} + ` + ); + + const entries = await generateConfigFromFileTree({ + baseDir: ".", + baseURL: "/base" as UrlPath, + }); + expect(entries).toEqual({ + routes: [ + { + method: "PUT", + module: ["bar.ts:onRequestPut"], + routePath: "/base/bar", + }, + { + method: "DELETE", + module: ["bar.ts:onRequestDelete"], + routePath: "/base/bar", + }, + { + method: "GET", + module: ["foo.ts:onRequestGet"], + routePath: "/base/foo", + }, + { + method: "POST", + module: ["foo.ts:onRequestPost"], + routePath: "/base/foo", + }, + ], + }); + }); }); }); diff --git a/packages/wrangler/pages/functions/filepath-routing.ts b/packages/wrangler/pages/functions/filepath-routing.ts index 6beea7f762f3..42ce13f7efa3 100644 --- a/packages/wrangler/pages/functions/filepath-routing.ts +++ b/packages/wrangler/pages/functions/filepath-routing.ts @@ -127,7 +127,9 @@ export async function generateConfigFromFileTree({ routeEntries = routeEntries.reduce( (acc: typeof routeEntries, { routePath, ...rest }) => { const existingRouteEntry = acc.find( - (routeEntry) => routeEntry.routePath === routePath + (routeEntry) => + routeEntry.routePath === routePath && + routeEntry.method === rest.method ); if (existingRouteEntry !== undefined) { Object.assign(existingRouteEntry, rest);