diff --git a/.changeset/fifty-kiwis-sniff.md b/.changeset/fifty-kiwis-sniff.md new file mode 100644 index 0000000000..4f18a301c1 --- /dev/null +++ b/.changeset/fifty-kiwis-sniff.md @@ -0,0 +1,5 @@ +--- +"@remix-run/router": patch +--- + +Include submission info in `shouldRevalidate` on action redirects diff --git a/package.json b/package.json index 48f42de25b..53ba67abbc 100644 --- a/package.json +++ b/package.json @@ -107,7 +107,7 @@ }, "filesize": { "packages/router/dist/router.umd.min.js": { - "none": "37.5 kB" + "none": "38 kB" }, "packages/react-router/dist/react-router.production.min.js": { "none": "12.5 kB" diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index 1e2a1d0a0c..b74e5fa17c 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -1671,6 +1671,112 @@ describe("a router", () => { router.dispose(); }); + it("includes submission on actions that return data", async () => { + let shouldRevalidate = jest.fn(() => true); + + let history = createMemoryHistory({ initialEntries: ["/child"] }); + let router = createRouter({ + history, + routes: [ + { + path: "/", + id: "root", + loader: () => "ROOT", + shouldRevalidate, + children: [ + { + path: "child", + id: "child", + loader: () => "CHILD", + action: () => "ACTION", + }, + ], + }, + ], + }); + router.initialize(); + + // Initial load - no existing data, should always call loader and should + // not give use ability to opt-out + await tick(); + router.navigate("/child", { + formMethod: "post", + formData: createFormData({ key: "value" }), + }); + await tick(); + expect(shouldRevalidate.mock.calls.length).toBe(1); + // @ts-expect-error + let arg = shouldRevalidate.mock.calls[0][0]; + expect(arg).toMatchObject({ + currentParams: {}, + currentUrl: new URL("http://localhost/child"), + nextParams: {}, + nextUrl: new URL("http://localhost/child"), + defaultShouldRevalidate: true, + formMethod: "post", + formAction: "/child", + formEncType: "application/x-www-form-urlencoded", + actionResult: "ACTION", + }); + // @ts-expect-error + expect(Object.fromEntries(arg.formData)).toEqual({ key: "value" }); + + router.dispose(); + }); + + it("includes submission on actions that return redirects", async () => { + let shouldRevalidate = jest.fn(() => true); + + let history = createMemoryHistory({ initialEntries: ["/child"] }); + let router = createRouter({ + history, + routes: [ + { + path: "/", + id: "root", + loader: () => "ROOT", + shouldRevalidate, + children: [ + { + path: "child", + id: "child", + loader: () => "CHILD", + action: () => redirect("/"), + }, + ], + }, + ], + }); + router.initialize(); + + // Initial load - no existing data, should always call loader and should + // not give use ability to opt-out + await tick(); + router.navigate("/child", { + formMethod: "post", + formData: createFormData({ key: "value" }), + }); + await tick(); + expect(shouldRevalidate.mock.calls.length).toBe(1); + // @ts-expect-error + let arg = shouldRevalidate.mock.calls[0][0]; + expect(arg).toMatchObject({ + currentParams: {}, + currentUrl: new URL("http://localhost/child"), + nextParams: {}, + nextUrl: new URL("http://localhost/"), + defaultShouldRevalidate: true, + formMethod: "post", + formAction: "/child", + formEncType: "application/x-www-form-urlencoded", + actionResult: undefined, + }); + // @ts-expect-error + expect(Object.fromEntries(arg.formData)).toEqual({ key: "value" }); + + router.dispose(); + }); + it("provides the default implementation to the route function", async () => { let rootLoader = jest.fn((args) => "ROOT"); @@ -1894,7 +2000,8 @@ describe("a router", () => { data: "FETCH", }); - expect(shouldRevalidate.mock.calls[0][0]).toMatchInlineSnapshot(` + let arg = shouldRevalidate.mock.calls[0][0]; + expect(arg).toMatchInlineSnapshot(` Object { "actionResult": "FETCH", "currentParams": Object {}, @@ -1908,6 +2015,68 @@ describe("a router", () => { "nextUrl": "http://localhost/", } `); + expect(Object.fromEntries(arg.formData)).toEqual({ key: "value" }); + + router.dispose(); + }); + + it("applies to fetcher submissions when action redirects", async () => { + let shouldRevalidate = jest.fn((args) => true); + + let history = createMemoryHistory(); + let router = createRouter({ + history, + routes: [ + { + path: "", + id: "root", + + children: [ + { + path: "/", + id: "index", + loader: () => "INDEX", + shouldRevalidate, + }, + { + path: "/fetch", + id: "fetch", + action: () => redirect("/"), + }, + ], + }, + ], + }); + router.initialize(); + await tick(); + + let key = "key"; + router.fetch(key, "root", "/fetch", { + formMethod: "post", + formData: createFormData({ key: "value" }), + }); + await tick(); + expect(router.state.fetchers.get(key)).toMatchObject({ + state: "idle", + data: undefined, + }); + + let arg = shouldRevalidate.mock.calls[0][0]; + expect(arg).toMatchInlineSnapshot(` + Object { + "actionResult": undefined, + "currentParams": Object {}, + "currentUrl": "http://localhost/", + "defaultShouldRevalidate": true, + "formAction": "/fetch", + "formData": FormData {}, + "formEncType": "application/x-www-form-urlencoded", + "formMethod": "post", + "nextParams": Object {}, + "nextUrl": "http://localhost/", + } + `); + expect(Object.fromEntries(arg.formData)).toEqual({ key: "value" }); router.dispose(); }); diff --git a/packages/router/router.ts b/packages/router/router.ts index 8b05e6521d..60c78f1593 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -1092,7 +1092,7 @@ export function createRouter(init: RouterInit): Router { replace = result.location === state.location.pathname + state.location.search; } - await startRedirectNavigation(state, result, replace); + await startRedirectNavigation(state, result, { submission, replace }); return { shortCircuited: true }; } @@ -1152,10 +1152,26 @@ export function createRouter(init: RouterInit): Router { loadingNavigation = navigation; } + // If this was a redirect from an action we don't have a "submission" but + // we have it on the loading navigation so use that if available + let activeSubmission = submission + ? submission + : loadingNavigation.formMethod && + loadingNavigation.formAction && + loadingNavigation.formData && + loadingNavigation.formEncType + ? { + formMethod: loadingNavigation.formMethod, + formAction: loadingNavigation.formAction, + formData: loadingNavigation.formData, + formEncType: loadingNavigation.formEncType, + } + : undefined; + let [matchesToLoad, revalidatingFetchers] = getMatchesToLoad( state, matches, - submission, + activeSubmission, location, isRevalidationRequired, cancelledDeferredRoutes, @@ -1244,7 +1260,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) { - await startRedirectNavigation(state, redirect, replace); + await startRedirectNavigation(state, redirect, { replace }); return { shortCircuited: true }; } @@ -1401,7 +1417,10 @@ export function createRouter(init: RouterInit): Router { state.fetchers.set(key, loadingFetcher); updateState({ fetchers: new Map(state.fetchers) }); - return startRedirectNavigation(state, actionResult, false, true); + return startRedirectNavigation(state, actionResult, { + submission, + isFetchActionRedirect: true, + }); } // Process any non-redirect errors thrown @@ -1495,7 +1514,7 @@ export function createRouter(init: RouterInit): Router { let redirect = findRedirect(results); if (redirect) { - return startRedirectNavigation(state, redirect); + return startRedirectNavigation(state, redirect, { submission }); } // Process and commit output from loaders @@ -1673,8 +1692,15 @@ export function createRouter(init: RouterInit): Router { async function startRedirectNavigation( state: RouterState, redirect: RedirectResult, - replace?: boolean, - isFetchActionRedirect?: boolean + { + submission, + replace, + isFetchActionRedirect, + }: { + submission?: Submission; + replace?: boolean; + isFetchActionRedirect?: boolean; + } = {} ) { if (redirect.revalidate) { isRevalidationRequired = true; @@ -1714,24 +1740,30 @@ export function createRouter(init: RouterInit): Router { let redirectHistoryAction = replace === true ? HistoryAction.Replace : HistoryAction.Push; + // Use the incoming submission if provided, fallback on the active one in + // state.navigation let { formMethod, formAction, formEncType, formData } = state.navigation; + if (!submission && formMethod && formAction && formData && formEncType) { + submission = { + formMethod, + formAction, + formEncType, + formData, + }; + } // If this was a 307/308 submission we want to preserve the HTTP method and // re-submit the GET/POST/PUT/PATCH/DELETE as a submission navigation to the // redirected location if ( redirectPreserveMethodStatusCodes.has(redirect.status) && - formMethod && - isMutationMethod(formMethod) && - formEncType && - formData + submission && + isMutationMethod(submission.formMethod) ) { await startNavigation(redirectHistoryAction, redirectLocation, { submission: { - formMethod, + ...submission, formAction: redirect.location, - formEncType, - formData, }, }); } else { @@ -1741,10 +1773,10 @@ export function createRouter(init: RouterInit): Router { overrideNavigation: { state: "loading", location: redirectLocation, - formMethod: formMethod || undefined, - formAction: formAction || undefined, - formEncType: formEncType || undefined, - formData: formData || undefined, + formMethod: submission ? submission.formMethod : undefined, + formAction: submission ? submission.formAction : undefined, + formEncType: submission ? submission.formEncType : undefined, + formData: submission ? submission.formData : undefined, }, }); }