From 3d0d28c9870304b13b39217976bc1c14b31c4f49 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Fri, 21 Oct 2022 16:46:39 -0400 Subject: [PATCH] Updates to createStaticHandler for Remix consumption (#9482) * Updates to createStaticHandler for Remix consumption * unit tests * add changeset * one more test for HEAD in queryRoute * add tests --- .changeset/funny-shirts-admire.md | 5 + packages/router/__tests__/router-test.ts | 204 ++++++++++++++++++----- packages/router/router.ts | 70 +++++--- 3 files changed, 213 insertions(+), 66 deletions(-) create mode 100644 .changeset/funny-shirts-admire.md diff --git a/.changeset/funny-shirts-admire.md b/.changeset/funny-shirts-admire.md new file mode 100644 index 0000000000..9c3575ce71 --- /dev/null +++ b/.changeset/funny-shirts-admire.md @@ -0,0 +1,5 @@ +--- +"@remix-run/router": patch +--- + +Changes to statis handler for incorporating into Remix" diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index 144a23f68e..9f5e8ee2c5 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -2144,6 +2144,31 @@ describe("a router", () => { ]); }); + it("matches root pathless route", () => { + let t = setup({ + routes: [{ id: "root", children: [{ path: "foo" }] }], + }); + + t.navigate("/not-found"); + expect(t.router.state.errors).toEqual({ + root: { + status: 404, + statusText: "Not Found", + data: null, + }, + }); + expect(t.router.state.matches).toMatchObject([ + { + params: {}, + pathname: "", + route: { + id: "root", + children: expect.any(Array), + }, + }, + ]); + }); + it("clears prior loader/action data", async () => { let t = initializeTmTest(); expect(t.router.state.loaderData).toEqual({ @@ -3128,11 +3153,7 @@ describe("a router", () => { formData: createFormData({ gosh: "dang" }), }); expect(t.router.state.errors).toEqual({ - child: new ErrorResponse( - 405, - "Method Not Allowed", - "No action found for [/child]" - ), + child: new ErrorResponse(405, "Method Not Allowed", ""), }); expect(console.warn).toHaveBeenCalled(); spy.mockReset(); @@ -3185,11 +3206,7 @@ describe("a router", () => { }); expect(t.router.state.actionData).toBe(null); expect(t.router.state.errors).toEqual({ - grandchild: new ErrorResponse( - 405, - "Method Not Allowed", - "No action found for [/child/grandchild]" - ), + grandchild: new ErrorResponse(405, "Method Not Allowed", ""), }); }); }); @@ -6667,11 +6684,7 @@ describe("a router", () => { }); expect(A.fetcher).toBe(IDLE_FETCHER); expect(t.router.state.errors).toEqual({ - root: new ErrorResponse( - 405, - "Method Not Allowed", - "No action found for [/]" - ), + root: new ErrorResponse(405, "Method Not Allowed", ""), }); }); @@ -9906,6 +9919,23 @@ describe("a router", () => { }); }); + it("should support document load navigations with HEAD requests", async () => { + let { query } = createStaticHandler(SSR_ROUTES); + let context = await query( + createRequest("/parent/child", { method: "HEAD" }) + ); + expect(context).toMatchObject({ + actionData: null, + loaderData: { + parent: "PARENT LOADER", + child: "CHILD LOADER", + }, + errors: null, + location: { pathname: "/parent/child" }, + matches: [{ route: { id: "parent" } }, { route: { id: "child" } }], + }); + }); + it("should support document load navigations returning responses", async () => { let { query } = createStaticHandler(SSR_ROUTES); let context = await query(createRequest("/parent/json")); @@ -9962,6 +9992,39 @@ describe("a router", () => { }); }); + it("should support alternative submission methods", async () => { + let { query } = createStaticHandler(SSR_ROUTES); + let context; + + let expected = { + actionData: { + child: "CHILD ACTION", + }, + loaderData: { + parent: "PARENT LOADER", + child: "CHILD LOADER", + }, + errors: null, + location: { pathname: "/parent/child" }, + matches: [{ route: { id: "parent" } }, { route: { id: "child" } }], + }; + + context = await query( + createSubmitRequest("/parent/child", { method: "PUT" }) + ); + expect(context).toMatchObject(expected); + + context = await query( + createSubmitRequest("/parent/child", { method: "PATCH" }) + ); + expect(context).toMatchObject(expected); + + context = await query( + createSubmitRequest("/parent/child", { method: "DELETE" }) + ); + expect(context).toMatchObject(expected); + }); + it("should support document submit navigations returning responses", async () => { let { query } = createStaticHandler(SSR_ROUTES); let context = await query(createSubmitRequest("/parent/json")); @@ -10202,20 +10265,6 @@ describe("a router", () => { expect(e).toMatchInlineSnapshot(`[Error: query() call aborted]`); }); - it("should not support HEAD requests", async () => { - let { query } = createStaticHandler(SSR_ROUTES); - let request = createRequest("/", { method: "head" }); - let e; - try { - await query(request); - } catch (_e) { - e = _e; - } - expect(e).toMatchInlineSnapshot( - `[Error: query()/queryRoute() do not support HEAD requests]` - ); - }); - it("should require a signal on the request", async () => { let { query } = createStaticHandler(SSR_ROUTES); let request = createRequest("/", { signal: undefined }); @@ -10246,7 +10295,30 @@ describe("a router", () => { root: { status: 405, statusText: "Method Not Allowed", - data: "No action found for [/]", + data: "", + }, + }, + matches: [{ route: { id: "root" } }], + }); + }); + + it("should handle unsupported methods with a 405 error", async () => { + let { query } = createStaticHandler([ + { + id: "root", + path: "/", + }, + ]); + let request = createRequest("/", { method: "OPTIONS" }); + let context = await query(request); + expect(context).toMatchObject({ + actionData: null, + loaderData: {}, + errors: { + root: { + status: 405, + statusText: "Method Not Allowed", + data: null, }, }, matches: [{ route: { id: "root" } }], @@ -10632,6 +10704,14 @@ describe("a router", () => { expect(data).toBe("CHILD LOADER"); }); + it("should support HEAD requests", async () => { + let { queryRoute } = createStaticHandler(SSR_ROUTES); + let data = await queryRoute( + createRequest("/parent", { method: "HEAD" }) + ); + expect(data).toBe("PARENT LOADER"); + }); + it("should support singular route load navigations (primitives)", async () => { let { queryRoute } = createStaticHandler(SSR_ROUTES); let data; @@ -10800,6 +10880,29 @@ describe("a router", () => { expect(data).toBe(""); }); + it("should support alternative submission methods", async () => { + let { queryRoute } = createStaticHandler(SSR_ROUTES); + let data; + + data = await queryRoute( + createSubmitRequest("/parent", { method: "PUT" }), + "parent" + ); + expect(data).toBe("PARENT ACTION"); + + data = await queryRoute( + createSubmitRequest("/parent", { method: "PATCH" }), + "parent" + ); + expect(data).toBe("PARENT ACTION"); + + data = await queryRoute( + createSubmitRequest("/parent", { method: "DELETE" }), + "parent" + ); + expect(data).toBe("PARENT ACTION"); + }); + it("should support singular route submit navigations (Responses)", async () => { /* eslint-disable jest/no-conditional-expect */ let T = setupFlexRouteTest(); @@ -11042,20 +11145,6 @@ describe("a router", () => { expect(e).toMatchInlineSnapshot(`[Error: queryRoute() call aborted]`); }); - it("should not support HEAD requests", async () => { - let { queryRoute } = createStaticHandler(SSR_ROUTES); - let request = createRequest("/", { method: "head" }); - let e; - try { - await queryRoute(request, "index"); - } catch (_e) { - e = _e; - } - expect(e).toMatchInlineSnapshot( - `[Error: query()/queryRoute() do not support HEAD requests]` - ); - }); - it("should require a signal on the request", async () => { let { queryRoute } = createStaticHandler(SSR_ROUTES); let request = createRequest("/", { signal: undefined }); @@ -11139,7 +11228,32 @@ describe("a router", () => { expect(data.status).toBe(405); expect(data.statusText).toBe("Method Not Allowed"); expect(data.headers.get("X-Remix-Router-Error")).toBe("yes"); - expect(await data.text()).toBe("No action found for [/]"); + expect(await data.text()).toBe(""); + } + /* eslint-enable jest/no-conditional-expect */ + }); + + it("should handle unsupported methods with a 405 Response", async () => { + /* eslint-disable jest/no-conditional-expect */ + let { queryRoute } = createStaticHandler([ + { + id: "root", + path: "/", + }, + ]); + + try { + await queryRoute( + createSubmitRequest("/", { method: "OPTIONS" }), + "root" + ); + expect(false).toBe(true); + } catch (data) { + expect(data instanceof Response).toBe(true); + expect(data.status).toBe(405); + expect(data.statusText).toBe("Method Not Allowed"); + expect(data.headers.get("X-Remix-Router-Error")).toBe("yes"); + expect(await data.text()).toBe(""); } /* eslint-enable jest/no-conditional-expect */ }); diff --git a/packages/router/router.ts b/packages/router/router.ts index 0e00c308d0..e83f6aba52 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -1103,7 +1103,7 @@ export function createRouter(init: RouterInit): Router { // a revalidation interrupting an actionReload) if (!isUninterruptedRevalidation) { revalidatingFetchers.forEach(([key]) => { - const fetcher = state.fetchers.get(key); + let fetcher = state.fetchers.get(key); let revalidatingFetcher: FetcherStates["Loading"] = { state: "loading", data: fetcher && fetcher.data, @@ -1829,6 +1829,9 @@ export function createRouter(init: RouterInit): Router { //#region createStaticHandler //////////////////////////////////////////////////////////////////////////////// +const validActionMethods = new Set(["POST", "PUT", "PATCH", "DELETE"]); +const validRequestMethods = new Set(["GET", "HEAD", ...validActionMethods]); + export function unstable_createStaticHandler( routes: AgnosticRouteObject[] ): StaticHandler { @@ -1865,7 +1868,25 @@ export function unstable_createStaticHandler( let location = createLocation("", createPath(url), null, "default"); let matches = matchRoutes(dataRoutes, location); - if (!matches) { + if (!validRequestMethods.has(request.method)) { + let { + matches: methodNotAllowedMatches, + route, + error, + } = getMethodNotAllowedMatches(dataRoutes); + return { + location, + matches: methodNotAllowedMatches, + loaderData: {}, + actionData: null, + errors: { + [route.id]: error, + }, + statusCode: error.status, + loaderHeaders: {}, + actionHeaders: {}, + }; + } else if (!matches) { let { matches: notFoundMatches, route, @@ -1879,7 +1900,7 @@ export function unstable_createStaticHandler( errors: { [route.id]: error, }, - statusCode: 404, + statusCode: error.status, loaderHeaders: {}, actionHeaders: {}, }; @@ -1918,7 +1939,12 @@ export function unstable_createStaticHandler( let location = createLocation("", createPath(url), null, "default"); let matches = matchRoutes(dataRoutes, location); - if (!matches) { + if (!validRequestMethods.has(request.method)) { + throw createRouterErrorResponse(null, { + status: 405, + statusText: "Method Not Allowed", + }); + } else if (!matches) { throw createRouterErrorResponse(null, { status: 404, statusText: "Not Found", @@ -1961,17 +1987,13 @@ export function unstable_createStaticHandler( matches: AgnosticDataRouteMatch[], routeMatch?: AgnosticDataRouteMatch ): Promise | Response> { - invariant( - request.method !== "HEAD", - "query()/queryRoute() do not support HEAD requests" - ); invariant( request.signal, "query()/queryRoute() requests must contain an AbortController signal" ); try { - if (request.method !== "GET") { + if (validActionMethods.has(request.method)) { let result = await submit( request, matches, @@ -2018,7 +2040,7 @@ export function unstable_createStaticHandler( if (!actionMatch.route.action) { let href = createServerHref(new URL(request.url)); if (isRouteRequest) { - throw createRouterErrorResponse(`No action found for [${href}]`, { + throw createRouterErrorResponse(null, { status: 405, statusText: "Method Not Allowed", }); @@ -2832,16 +2854,18 @@ function findNearestBoundary( ); } -function getNotFoundMatches(routes: AgnosticDataRouteObject[]): { +function getShortCircuitMatches( + routes: AgnosticDataRouteObject[], + status: number, + statusText: string +): { matches: AgnosticDataRouteMatch[]; route: AgnosticDataRouteObject; error: ErrorResponse; } { // Prefer a root layout route if present, otherwise shim in a route object - let route = routes.find( - (r) => r.index || r.path === "" || r.path === "/" - ) || { - id: "__shim-404-route__", + let route = routes.find((r) => r.index || !r.path || r.path === "/") || { + id: `__shim-${status}-route__`, }; return { @@ -2854,10 +2878,18 @@ function getNotFoundMatches(routes: AgnosticDataRouteObject[]): { }, ], route, - error: new ErrorResponse(404, "Not Found", null), + error: new ErrorResponse(status, statusText, null), }; } +function getNotFoundMatches(routes: AgnosticDataRouteObject[]) { + return getShortCircuitMatches(routes, 404, "Not Found"); +} + +function getMethodNotAllowedMatches(routes: AgnosticDataRouteObject[]) { + return getShortCircuitMatches(routes, 405, "Method Not Allowed"); +} + function getMethodNotAllowedResult(path: Location | string): ErrorResult { let href = typeof path === "string" ? path : createServerHref(path); console.warn( @@ -2867,11 +2899,7 @@ function getMethodNotAllowedResult(path: Location | string): ErrorResult { ); return { type: ResultType.error, - error: new ErrorResponse( - 405, - "Method Not Allowed", - `No action found for [${href}]` - ), + error: new ErrorResponse(405, "Method Not Allowed", ""), }; }