From 8940d3b7f5cfe9b63be308b4ff568cda4bada2e3 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 15 Nov 2022 11:34:04 -0500 Subject: [PATCH 1/7] fix: preserve method on 307/308 redirects --- packages/router/__tests__/router-test.ts | 172 +++++++++++++++++++++++ packages/router/router.ts | 93 ++++++------ packages/router/utils.ts | 4 +- 3 files changed, 225 insertions(+), 44 deletions(-) diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index 0b19d09eaf..541c3702d6 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -365,6 +365,11 @@ function setup({ let redirectNavigationId = ++guid; activeLoaderType = "navigation"; activeLoaderNavigationId = redirectNavigationId; + if (status === 307 || status === 308) { + activeActionType = "navigation"; + activeActionNavigationId = redirectNavigationId; + } + let helpers = getNavigationHelpers(href, redirectNavigationId); // Since a redirect kicks off and awaits a new navigation we can't shim @@ -5484,6 +5489,173 @@ describe("a router", () => { errors: null, }); }); + + describe("redirect status code handling", () => { + it("should not treat 300 as a redirect", async () => { + let t = setup({ routes: REDIRECT_ROUTES }); + + let A = await t.navigate("/parent"); + await A.loaders.parent.redirectReturn("/idk", 300); + expect(t.router.state).toMatchObject({ + loaderData: {}, + location: { + pathname: "/parent", + }, + navigation: { + state: "idle", + }, + }); + }); + + it("should not preserve the method on 301 redirects", async () => { + let t = setup({ routes: REDIRECT_ROUTES }); + + let A = await t.navigate("/parent/child", { + formMethod: "post", + formData: createFormData({ key: "value" }), + }); + // Triggers a GET redirect + let B = await A.actions.child.redirectReturn("/parent", 301); + await B.loaders.parent.resolve("PARENT"); + expect(t.router.state).toMatchObject({ + loaderData: { + parent: "PARENT", + }, + location: { + pathname: "/parent", + }, + navigation: { + state: "idle", + }, + }); + }); + + it("should not preserve the method on 302 redirects", async () => { + let t = setup({ routes: REDIRECT_ROUTES }); + + let A = await t.navigate("/parent/child", { + formMethod: "post", + formData: createFormData({ key: "value" }), + }); + // Triggers a GET redirect + let B = await A.actions.child.redirectReturn("/parent", 302); + await B.loaders.parent.resolve("PARENT"); + expect(t.router.state).toMatchObject({ + loaderData: { + parent: "PARENT", + }, + location: { + pathname: "/parent", + }, + navigation: { + state: "idle", + }, + }); + }); + + it("should not preserve the method on 303 redirects", async () => { + let t = setup({ routes: REDIRECT_ROUTES }); + + let A = await t.navigate("/parent/child", { + formMethod: "post", + formData: createFormData({ key: "value" }), + }); + // Triggers a GET redirect + let B = await A.actions.child.redirectReturn("/parent", 303); + await B.loaders.parent.resolve("PARENT"); + expect(t.router.state).toMatchObject({ + loaderData: { + parent: "PARENT", + }, + location: { + pathname: "/parent", + }, + navigation: { + state: "idle", + }, + }); + }); + + it("should not treat 304 as a redirect", async () => { + let t = setup({ routes: REDIRECT_ROUTES }); + + let A = await t.navigate("/parent"); + await A.loaders.parent.resolve(new Response(null, { status: 304 })); + expect(t.router.state).toMatchObject({ + loaderData: {}, + location: { + pathname: "/parent", + }, + navigation: { + state: "idle", + }, + }); + }); + + it("should preserve the method on 307 redirects", async () => { + let t = setup({ routes: REDIRECT_ROUTES }); + + let A = await t.navigate("/parent/child", { + formMethod: "post", + formData: createFormData({ key: "value" }), + }); + // Triggers a POST redirect + let B = await A.actions.child.redirectReturn("/parent", 307); + await B.actions.parent.resolve("PARENT ACTION"); + await B.loaders.parent.resolve("PARENT"); + expect(t.router.state).toMatchObject({ + actionData: { + parent: "PARENT ACTION", + }, + loaderData: { + parent: "PARENT", + }, + location: { + pathname: "/parent", + }, + navigation: { + state: "idle", + }, + }); + + let request = B.actions.parent.stub.mock.calls[0][0].request; + expect(request.method).toBe("POST"); + let fd = await request.formData(); + expect(Array.from(fd.entries())).toEqual([["key", "value"]]); + }); + + it("should preserve the method on 308 redirects", async () => { + let t = setup({ routes: REDIRECT_ROUTES }); + + let A = await t.navigate("/parent/child", { + formMethod: "post", + formData: createFormData({ key: "value" }), + }); + // Triggers a POST redirect + let B = await A.actions.child.redirectReturn("/parent", 308); + await B.actions.parent.resolve("PARENT ACTION"); + await B.loaders.parent.resolve("PARENT"); + expect(t.router.state).toMatchObject({ + actionData: { + parent: "PARENT ACTION", + }, + loaderData: { + parent: "PARENT", + }, + location: { + pathname: "/parent", + }, + navigation: { + state: "idle", + }, + }); + + let request = B.actions.parent.stub.mock.calls[0][0].request; + expect(request.method).toBe("POST"); + let fd = await request.formData(); + expect(Array.from(fd.entries())).toEqual([["key", "value"]]); + }); + }); }); describe("scroll restoration", () => { diff --git a/packages/router/router.ts b/packages/router/router.ts index 51e02187de..a6d9cc4b9f 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -1000,15 +1000,10 @@ export function createRouter(init: RouterInit): Router { } if (isRedirectResult(result)) { - let redirectNavigation: NavigationStates["Loading"] = { - state: "loading", - location: createLocation(state.location, result.location), - ...submission, - }; await startRedirectNavigation( + state, result, - redirectNavigation, - opts && opts.replace + opts && opts.replace === true ); return { shortCircuited: true }; } @@ -1152,8 +1147,7 @@ export function createRouter(init: RouterInit): Router { // If any loaders returned a redirect Response, start a new REPLACE navigation let redirect = findRedirect(results); if (redirect) { - let redirectNavigation = getLoaderRedirect(state, redirect); - await startRedirectNavigation(redirect, redirectNavigation, replace); + await startRedirectNavigation(state, redirect, replace); return { shortCircuited: true }; } @@ -1304,12 +1298,7 @@ export function createRouter(init: RouterInit): Router { state.fetchers.set(key, loadingFetcher); updateState({ fetchers: new Map(state.fetchers) }); - let redirectNavigation: NavigationStates["Loading"] = { - state: "loading", - location: createLocation(state.location, actionResult.location), - ...submission, - }; - await startRedirectNavigation(actionResult, redirectNavigation); + await startRedirectNavigation(state, actionResult); return; } @@ -1402,8 +1391,7 @@ export function createRouter(init: RouterInit): Router { let redirect = findRedirect(results); if (redirect) { - let redirectNavigation = getLoaderRedirect(state, redirect); - await startRedirectNavigation(redirect, redirectNavigation); + await startRedirectNavigation(state, redirect); return; } @@ -1515,8 +1503,7 @@ export function createRouter(init: RouterInit): Router { // If the loader threw a redirect Response, start a new REPLACE navigation if (isRedirectResult(result)) { - let redirectNavigation = getLoaderRedirect(state, result); - await startRedirectNavigation(result, redirectNavigation); + await startRedirectNavigation(state, result); return; } @@ -1571,15 +1558,17 @@ export function createRouter(init: RouterInit): Router { * the history action from the original navigation (PUSH or REPLACE). */ async function startRedirectNavigation( + state: RouterState, redirect: RedirectResult, - navigation: Navigation, replace?: boolean ) { if (redirect.revalidate) { isRevalidationRequired = true; } + + let redirectLocation = createLocation(state.location, redirect.location); invariant( - navigation.location, + redirectLocation, "Expected a location on the redirect navigation" ); // There's no need to abort on redirects, since we don't detect the @@ -1589,9 +1578,41 @@ export function createRouter(init: RouterInit): Router { let redirectHistoryAction = replace === true ? HistoryAction.Replace : HistoryAction.Push; - await startNavigation(redirectHistoryAction, navigation.location, { - overrideNavigation: navigation, - }); + let { formMethod, formAction, formEncType, formData } = state.navigation; + + // If this was a 307/308 submission we want to preserve the HTTP method and + // re-submit the POST/PUT/PATCH/DELETE as a submission navigation to the + // redirected location + if ( + [307, 308].includes(redirect.status) && + formMethod && + formMethod !== "get" && + formMethod !== "head" && + formEncType && + formData + ) { + await startNavigation(redirectHistoryAction, redirectLocation, { + submission: { + formMethod, + formAction: redirect.location, + formEncType, + formData, + }, + }); + } else { + // Otherwise, we kick off a new loading navigation, preserving the + // submission info for the duration of this navigation + await startNavigation(redirectHistoryAction, redirectLocation, { + overrideNavigation: { + state: "loading", + location: redirectLocation, + formMethod: formMethod || undefined, + formAction: formAction || undefined, + formEncType: formEncType || undefined, + formData: formData || undefined, + }, + }); + } } async function callLoadersAndMaybeResolveData( @@ -2279,7 +2300,11 @@ function normalizeNavigateOptions( } // Create a Submission on non-GET navigations - if (opts.formMethod != null && opts.formMethod !== "get") { + if ( + opts.formMethod != null && + opts.formMethod !== "get" && + opts.formMethod !== "head" + ) { return { path, submission: { @@ -2322,22 +2347,6 @@ function normalizeNavigateOptions( return { path: createPath(parsedPath) }; } -function getLoaderRedirect( - state: RouterState, - redirect: RedirectResult -): Navigation { - let { formMethod, formAction, formEncType, formData } = state.navigation; - let navigation: NavigationStates["Loading"] = { - state: "loading", - location: createLocation(state.location, redirect.location), - formMethod: formMethod || undefined, - formAction: formAction || undefined, - formEncType: formEncType || undefined, - formData: formData || undefined, - }; - return navigation; -} - // Filter out all routes below any caught error as they aren't going to // render so we don't need to load them function getLoaderMatchesUntilBoundary( @@ -2547,7 +2556,7 @@ async function callLoaderOrAction( let status = result.status; // Process redirects - if (status >= 300 && status <= 399) { + if ([301, 302, 303, 307, 308].includes(status)) { let location = result.headers.get("Location"); invariant( location, diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 5628fd7f45..900adf61cb 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -61,7 +61,7 @@ export type DataResult = | RedirectResult | ErrorResult; -export type FormMethod = "get" | "post" | "put" | "patch" | "delete"; +export type FormMethod = "get" | "head" | "post" | "put" | "patch" | "delete"; export type FormEncType = | "application/x-www-form-urlencoded" | "multipart/form-data"; @@ -72,7 +72,7 @@ export type FormEncType = * external consumption */ export interface Submission { - formMethod: Exclude; + formMethod: Exclude; formAction: string; formEncType: FormEncType; formData: FormData; From fac69cba70d57ebb0426e2bf9ecb8373c65afbc9 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 15 Nov 2022 11:35:44 -0500 Subject: [PATCH 2/7] Add changeset --- .changeset/stale-coats-smoke.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/stale-coats-smoke.md diff --git a/.changeset/stale-coats-smoke.md b/.changeset/stale-coats-smoke.md new file mode 100644 index 0000000000..4e5c3fc770 --- /dev/null +++ b/.changeset/stale-coats-smoke.md @@ -0,0 +1,5 @@ +--- +"@remix-run/router": patch +--- + +Preserve the HTTP method on 307/208 redirects From e34b07a8f86e64b4a3e9189a60a2b4085c364f31 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 17 Nov 2022 09:34:55 -0500 Subject: [PATCH 3/7] Fix changeset Co-authored-by: Mehdi Achour --- .changeset/stale-coats-smoke.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/stale-coats-smoke.md b/.changeset/stale-coats-smoke.md index 4e5c3fc770..68cd78074e 100644 --- a/.changeset/stale-coats-smoke.md +++ b/.changeset/stale-coats-smoke.md @@ -2,4 +2,4 @@ "@remix-run/router": patch --- -Preserve the HTTP method on 307/208 redirects +Preserve the HTTP method on 307/308 redirects From 87a44c4514c0af0cd5c20d0996e4ee7f03da3694 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 17 Nov 2022 10:49:49 -0500 Subject: [PATCH 4/7] Strengthen HTTP method validation --- packages/router/__tests__/router-test.ts | 82 +++++++++++++++++++++- packages/router/router.ts | 87 +++++++++++++++--------- packages/router/utils.ts | 6 +- 3 files changed, 141 insertions(+), 34 deletions(-) diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index 541c3702d6..2b74a7d4a1 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -3971,6 +3971,86 @@ describe("a router", () => { }); }); + it("returns a 405 error if attempting to submit with method=HEAD", async () => { + let t = setup({ + routes: TASK_ROUTES, + initialEntries: ["/"], + hydrationData: { + loaderData: { + root: "ROOT DATA", + index: "INDEX DATA", + }, + }, + }); + + let formData = new FormData(); + formData.append( + "blob", + new Blob(["

Some html file contents

"], { + type: "text/html", + }) + ); + + await t.navigate("/tasks", { + // @ts-expect-error + formMethod: "head", + formData: formData, + }); + expect(t.router.state.navigation.state).toBe("idle"); + expect(t.router.state.location).toMatchObject({ + pathname: "/tasks", + search: "", + }); + expect(t.router.state.errors).toEqual({ + tasks: new ErrorResponse( + 405, + "Method Not Allowed", + new Error('Invalid request method "HEAD"'), + true + ), + }); + }); + + it("returns a 405 error if attempting to submit with method=OPTIONS", async () => { + let t = setup({ + routes: TASK_ROUTES, + initialEntries: ["/"], + hydrationData: { + loaderData: { + root: "ROOT DATA", + index: "INDEX DATA", + }, + }, + }); + + let formData = new FormData(); + formData.append( + "blob", + new Blob(["

Some html file contents

"], { + type: "text/html", + }) + ); + + await t.navigate("/tasks", { + // @ts-expect-error + formMethod: "options", + formData: formData, + }); + expect(t.router.state.navigation.state).toBe("idle"); + expect(t.router.state.location).toMatchObject({ + pathname: "/tasks", + search: "", + }); + expect(t.router.state.errors).toEqual({ + tasks: new ErrorResponse( + 405, + "Method Not Allowed", + new Error('Invalid request method "OPTIONS"'), + true + ), + }); + }); + it("runs loaders above the boundary for 400 errors if binary data is attempted to be submitted using formMethod=GET", async () => { let t = setup({ routes: [ @@ -6994,7 +7074,7 @@ describe("a router", () => { 405, "Method Not Allowed", new Error( - 'You made a post request to "/" but did not provide a `loader` ' + + 'You made a POST request to "/" but did not provide an `action` ' + 'for route "root", so there is no way to handle the request.' ), true diff --git a/packages/router/router.ts b/packages/router/router.ts index a6d9cc4b9f..f9be740ec8 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -20,6 +20,7 @@ import type { Submission, SuccessResult, AgnosticRouteMatch, + SubmissionFormMethod, } from "./utils"; import { DeferredData, @@ -503,6 +504,17 @@ interface QueryRouteResponse { response: Response; } +const validActionMethodsArr: SubmissionFormMethod[] = [ + "post", + "put", + "patch", + "delete", +]; +const validActionMethods = new Set(validActionMethodsArr); + +const validRequestMethodsArr: FormMethod[] = ["get", ...validActionMethodsArr]; +const validRequestMethods = new Set(validRequestMethodsArr); + export const IDLE_NAVIGATION: NavigationStates["Idle"] = { state: "idle", location: undefined, @@ -1586,8 +1598,7 @@ export function createRouter(init: RouterInit): Router { if ( [307, 308].includes(redirect.status) && formMethod && - formMethod !== "get" && - formMethod !== "head" && + isSubmissionMethod(formMethod) && formEncType && formData ) { @@ -1861,9 +1872,6 @@ 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 { @@ -1897,11 +1905,13 @@ export function unstable_createStaticHandler( request: Request ): Promise { let url = new URL(request.url); + let method = request.method.toLowerCase(); let location = createLocation("", createPath(url), null, "default"); let matches = matchRoutes(dataRoutes, location); - if (!validRequestMethods.has(request.method)) { - let error = getInternalRouterError(405, { method: request.method }); + // SSR supports HEAD requests while SPA doesn't + if (!isValidMethod(method) && method !== "head") { + let error = getInternalRouterError(405, { method }); let { matches: methodNotAllowedMatches, route } = getShortCircuitMatches(dataRoutes); return { @@ -1967,11 +1977,13 @@ export function unstable_createStaticHandler( */ async function queryRoute(request: Request, routeId?: string): Promise { let url = new URL(request.url); + let method = request.method.toLowerCase(); let location = createLocation("", createPath(url), null, "default"); let matches = matchRoutes(dataRoutes, location); - if (!validRequestMethods.has(request.method)) { - throw getInternalRouterError(405, { method: request.method }); + // SSR supports HEAD requests while SPA doesn't + if (!isValidMethod(method) && method !== "head") { + throw getInternalRouterError(405, { method }); } else if (!matches) { throw getInternalRouterError(404, { pathname: location.pathname }); } @@ -2021,7 +2033,7 @@ export function unstable_createStaticHandler( ); try { - if (validActionMethods.has(request.method)) { + if (isSubmissionMethod(request.method.toLowerCase())) { let result = await submit( request, matches, @@ -2281,6 +2293,12 @@ export function getStaticContextFromError( return newContext; } +function isSubmissionNavigation( + opts: RouterNavigateOptions +): opts is SubmissionNavigateOptions { + return opts != null && "formData" in opts; +} + // Normalize navigation options by converting formMethod=GET formData objects to // URLSearchParams so they behave identically to links with query params function normalizeNavigateOptions( @@ -2295,16 +2313,19 @@ function normalizeNavigateOptions( let path = typeof to === "string" ? to : createPath(to); // Return location verbatim on non-submission navigations - if (!opts || (!("formMethod" in opts) && !("formData" in opts))) { + if (!opts || !isSubmissionNavigation(opts)) { return { path }; } + if (opts.formMethod && !isValidMethod(opts.formMethod)) { + return { + path, + error: getInternalRouterError(405, { method: opts.formMethod }), + }; + } + // Create a Submission on non-GET navigations - if ( - opts.formMethod != null && - opts.formMethod !== "get" && - opts.formMethod !== "head" - ) { + if (isSubmissionMethod(opts.formMethod)) { return { path, submission: { @@ -2317,11 +2338,6 @@ function normalizeNavigateOptions( }; } - // No formData to flatten for GET submission - if (!opts.formData) { - return { path }; - } - // Flatten submission onto URLSearchParams for GET submissions let parsedPath = parsePath(path); try { @@ -2901,8 +2917,8 @@ function getInternalRouterError( message?: string; } = {} ) { - let statusText: string; - let errorMessage = message; + let statusText = "Unknown Server Error"; + let errorMessage = "Unknown @remix-run/router error"; if (status === 400) { statusText = "Bad Request"; @@ -2916,23 +2932,20 @@ function getInternalRouterError( } else if (status === 405) { statusText = "Method Not Allowed"; if (method && pathname && routeId) { - if (validActionMethods.has(method)) { + if (isSubmissionMethod(method.toLowerCase())) { errorMessage = - `You made a ${method} request to "${pathname}" but ` + + `You made a ${method.toUpperCase()} request to "${pathname}" but ` + `did not provide an \`action\` for route "${routeId}", ` + `so there is no way to handle the request.`; } else { errorMessage = - `You made a ${method} request to "${pathname}" but ` + + `You made a ${method.toUpperCase()} request to "${pathname}" but ` + `did not provide a \`loader\` for route "${routeId}", ` + `so there is no way to handle the request.`; } - } else { - errorMessage = `Invalid request method "${method}"`; + } else if (method) { + errorMessage = `Invalid request method "${method.toUpperCase()}"`; } - } else { - statusText = "Unknown Server Error"; - errorMessage = "Unknown @remix-run/router error"; } return new ErrorResponse( @@ -2994,6 +3007,18 @@ function isQueryRouteResponse(obj: any): obj is QueryRouteResponse { ); } +function isValidMethod(method: string | undefined): method is FormMethod { + if (!method) return false; + return validRequestMethods.has(method as FormMethod); +} + +function isSubmissionMethod( + method: string | undefined +): method is SubmissionFormMethod { + if (!method) return false; + return validActionMethods.has(method as SubmissionFormMethod); +} + async function resolveDeferredResults( currentMatches: AgnosticDataRouteMatch[], matchesToLoad: AgnosticDataRouteMatch[], diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 900adf61cb..3fe7d0c504 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -61,7 +61,9 @@ export type DataResult = | RedirectResult | ErrorResult; -export type FormMethod = "get" | "head" | "post" | "put" | "patch" | "delete"; +export type SubmissionFormMethod = "post" | "put" | "patch" | "delete"; +export type FormMethod = "get" | SubmissionFormMethod; + export type FormEncType = | "application/x-www-form-urlencoded" | "multipart/form-data"; @@ -72,7 +74,7 @@ export type FormEncType = * external consumption */ export interface Submission { - formMethod: Exclude; + formMethod: SubmissionFormMethod; formAction: string; formEncType: FormEncType; formData: FormData; From c069c2e8e1356836e8f74b61e364b4d59891afff Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 17 Nov 2022 12:59:59 -0500 Subject: [PATCH 5/7] Move redirect status codes to a constant --- packages/router/router.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/router/router.ts b/packages/router/router.ts index f9be740ec8..d3500399de 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -515,6 +515,9 @@ const validActionMethods = new Set(validActionMethodsArr); const validRequestMethodsArr: FormMethod[] = ["get", ...validActionMethodsArr]; const validRequestMethods = new Set(validRequestMethodsArr); +const redirectStatusCodes = new Set([301, 302, 303, 307, 308]); +const redirectPreserveMethodStatusCodes = new Set([307, 308]); + export const IDLE_NAVIGATION: NavigationStates["Idle"] = { state: "idle", location: undefined, @@ -1596,7 +1599,7 @@ export function createRouter(init: RouterInit): Router { // re-submit the POST/PUT/PATCH/DELETE as a submission navigation to the // redirected location if ( - [307, 308].includes(redirect.status) && + redirectPreserveMethodStatusCodes.has(redirect.status) && formMethod && isSubmissionMethod(formMethod) && formEncType && @@ -2572,7 +2575,7 @@ async function callLoaderOrAction( let status = result.status; // Process redirects - if ([301, 302, 303, 307, 308].includes(status)) { + if (redirectStatusCodes.has(status)) { let location = result.headers.get("Location"); invariant( location, From 263c1a8f8f80720a8ee8a38ab4282bcf30826728 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Fri, 18 Nov 2022 09:35:40 -0500 Subject: [PATCH 6/7] PR feedback --- packages/router/router.ts | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/packages/router/router.ts b/packages/router/router.ts index d3500399de..d9241996e4 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -1313,8 +1313,7 @@ export function createRouter(init: RouterInit): Router { state.fetchers.set(key, loadingFetcher); updateState({ fetchers: new Map(state.fetchers) }); - await startRedirectNavigation(state, actionResult); - return; + return startRedirectNavigation(state, actionResult); } // Process any non-redirect errors thrown @@ -1406,8 +1405,7 @@ export function createRouter(init: RouterInit): Router { let redirect = findRedirect(results); if (redirect) { - await startRedirectNavigation(state, redirect); - return; + return startRedirectNavigation(state, redirect); } // Process and commit output from loaders @@ -2328,7 +2326,7 @@ function normalizeNavigateOptions( } // Create a Submission on non-GET navigations - if (isSubmissionMethod(opts.formMethod)) { + if (opts.formMethod && isSubmissionMethod(opts.formMethod)) { return { path, submission: { @@ -3010,15 +3008,11 @@ function isQueryRouteResponse(obj: any): obj is QueryRouteResponse { ); } -function isValidMethod(method: string | undefined): method is FormMethod { - if (!method) return false; +function isValidMethod(method: string): method is FormMethod { return validRequestMethods.has(method as FormMethod); } -function isSubmissionMethod( - method: string | undefined -): method is SubmissionFormMethod { - if (!method) return false; +function isSubmissionMethod(method: string): method is SubmissionFormMethod { return validActionMethods.has(method as SubmissionFormMethod); } From acc70662891ada575e934f9dd8b0b7aa39cf0009 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 23 Nov 2022 16:19:27 -0500 Subject: [PATCH 7/7] bundle bump --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1f29d2697a..d359a2a285 100644 --- a/package.json +++ b/package.json @@ -119,7 +119,7 @@ "none": "10.5 kB" }, "packages/react-router-dom/dist/umd/react-router-dom.production.min.js": { - "none": "16 kB" + "none": "16.5 kB" } } }