Fix: reset actionData on action redirect to current location (#9772)

* Clear actionData on redirect to current location * Add changeset
remix-run · Dec 22, 2022 · 159b988 · 159b988
1 parent 554aa84
commit 159b988
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 7 deletions.
diff --git a/.changeset/new-news-remember.md b/.changeset/new-news-remember.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/router": patch
+---
+
+Reset `actionData` on action redirect to current location
diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts
@@ -3013,13 +3013,17 @@ describe("a router", () => {
 
       await A.actions.foo.resolve("FOO ACTION");
       expect(A.loaders.root.stub.mock.calls.length).toBe(1);
+      expect(t.router.state.actionData).toEqual({
+        foo: "FOO ACTION",
+      });
 
       let B = await A.loaders.foo.redirect("/bar");
       await A.loaders.root.reject("ROOT ERROR");
       await B.loaders.root.resolve("ROOT LOADER 2");
       await B.loaders.bar.resolve("BAR LOADER");
       expect(B.loaders.root.stub.mock.calls.length).toBe(1);
       expect(t.router.state).toMatchObject({
+        actionData: null,
         loaderData: {
           root: "ROOT LOADER 2",
           bar: "BAR LOADER",
@@ -3051,8 +3055,11 @@ describe("a router", () => {
       });
       expect(A.loaders.root.stub.mock.calls.length).toBe(0);
 
-      await A.actions.foo.resolve(null);
+      await A.actions.foo.resolve("FOO ACTION");
       expect(A.loaders.root.stub.mock.calls.length).toBe(1);
+      expect(t.router.state.actionData).toEqual({
+        foo: "FOO ACTION",
+      });
 
       await A.loaders.foo.resolve("A LOADER");
       expect(t.router.state.navigation.state).toBe("loading");
@@ -3063,6 +3070,9 @@ describe("a router", () => {
 
       await A.loaders.root.resolve("ROOT LOADER");
       expect(t.router.state.navigation.state).toBe("idle");
+      expect(t.router.state.actionData).toEqual({
+        foo: "FOO ACTION", // kept around on action reload
+      });
       expect(t.router.state.loaderData).toEqual({
         foo: "A LOADER",
         root: "ROOT LOADER",
@@ -3238,6 +3248,45 @@ describe("a router", () => {
       });
     });
 
+    it("removes action data after action redirect to current location", async () => {
+      let t = setup({
+        routes: [
+          {
+            path: "/",
+            id: "index",
+            action: true,
+            loader: true,
+          },
+        ],
+      });
+      let A = await t.navigate("/", {
+        formMethod: "post",
+        formData: createFormData({ gosh: "" }),
+      });
+      await A.actions.index.resolve({ error: "invalid" });
+      expect(t.router.state.actionData).toEqual({
+        index: { error: "invalid" },
+      });
+
+      let B = await t.navigate("/", {
+        formMethod: "post",
+        formData: createFormData({ gosh: "dang" }),
+      });
+
+      let C = await B.actions.index.redirectReturn("/");
+      expect(t.router.state.actionData).toEqual({
+        index: { error: "invalid" },
+      });
+      expect(t.router.state.loaderData).toEqual({});
+
+      await C.loaders.index.resolve("NEW");
+
+      expect(t.router.state.actionData).toBeNull();
+      expect(t.router.state.loaderData).toEqual({
+        index: "NEW",
+      });
+    });
+
     it("uses the proper action for index routes", async () => {
       let t = setup({
         routes: [

diff --git a/packages/router/router.ts b/packages/router/router.ts
@@ -735,17 +735,15 @@ export function createRouter(init: RouterInit): Router {
   ): void {
     // Deduce if we're in a loading/actionReload state:
     // - We have committed actionData in the store
-    // - The current navigation was a submission
+    // - The current navigation was a mutation submission
     // - We're past the submitting state and into the loading state
-    // - The location we've finished loading is different from the submission
-    //   location, indicating we redirected from the action (avoids false
-    //   positives for loading/submissionRedirect when actionData returned
-    //   on a prior submission)
+    // - The location being loaded is not the result of a redirect
     let isActionReload =
       state.actionData != null &&
       state.navigation.formMethod != null &&
+      isMutationMethod(state.navigation.formMethod) &&
       state.navigation.state === "loading" &&
-      state.navigation.formAction?.split("?")[0] === location.pathname;
+      location.state?._isRedirect !== true;
 
     let actionData: RouteData | null;
     if (newState.actionData) {