Send submission to shouldRevalidate on action redirects (#9777)

Author: brophdawg11

.changeset/fifty-kiwis-sniff.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/router": patch
+---
+
+Include submission info in `shouldRevalidate` on action redirects

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"

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();
     });

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,
         },
       });
     }