From cbcba0bf2ab722f15712adbedfee1e80285e8db2 Mon Sep 17 00:00:00 2001 From: Johann Rakotoharisoa Date: Sat, 24 Sep 2022 13:56:21 +0200 Subject: [PATCH 1/4] fix: reset actionData state on redirection from an action --- .changeset/short-hats-peel.md | 5 +++ contributors.yml | 1 + packages/router/__tests__/router-test.ts | 45 ++++++++++++++++++++++++ packages/router/router.ts | 4 ++- 4 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 .changeset/short-hats-peel.md diff --git a/.changeset/short-hats-peel.md b/.changeset/short-hats-peel.md new file mode 100644 index 0000000000..2081648025 --- /dev/null +++ b/.changeset/short-hats-peel.md @@ -0,0 +1,5 @@ +--- +"@remix-run/router": patch +--- + +reset action data after redirection trigger by action diff --git a/contributors.yml b/contributors.yml index d7423fe37d..7a18c3b8d0 100644 --- a/contributors.yml +++ b/contributors.yml @@ -113,3 +113,4 @@ - xavier-lc - xcsnowcity - yuleicul +- jrakotoharisoa diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index 036a1a3042..ad40ae4ad5 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -2706,6 +2706,51 @@ describe("a router", () => { expect(t.router.state.actionData).toBeNull(); }); + it("removes action data after action redirect (return)", async () => { + let t = setup({ + routes: [ + { + path: "/", + id: "root", + loader: true, + hasErrorBoundary: true, + children: [ + { + index: true, + id: "child", + action: true, + }, + { + path: "/other", + id: "other", + loader: true, + action: true, + }, + ], + }, + ], + }); + let A = await t.navigate("/?index", { + formMethod: "post", + formData: createFormData({ gosh: "" }), + }); + await A.actions.child.resolve({ error: "invalid" }); + await A.loaders.root.resolve("ROOT LOADER"); + expect(t.router.state.actionData).toEqual({ + child: { error: "invalid" }, + }); + + let B = await t.navigate("/?index", { + formMethod: "post", + formData: createFormData({ gosh: "dang" }), + }); + let C = await B.actions.child.redirectReturn("/other"); + await C.loaders.root.resolve("ROOT LOADER"); + await C.loaders.other.resolve("OTHER LOADER"); + + expect(t.router.state.actionData).toBeNull(); + }); + 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 index 6af50bef5b..699c07b0a9 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -667,7 +667,9 @@ export function createRouter(init: RouterInit): Router { let isActionReload = state.actionData != null && state.navigation.formMethod != null && - state.navigation.state === "loading"; + state.navigation.state === "loading" && + state.navigation.formAction?.split("?")[0] === + state.navigation.location.pathname; // Always preserve any existing loaderData from re-used routes let newLoaderData = newState.loaderData From bf1c68bfd892907e7fc223905e0c95dca6692461 Mon Sep 17 00:00:00 2001 From: Joha2n Date: Sat, 24 Sep 2022 14:54:17 +0200 Subject: [PATCH 2/4] Update contributors.yml --- contributors.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contributors.yml b/contributors.yml index 7a18c3b8d0..044bdc5f48 100644 --- a/contributors.yml +++ b/contributors.yml @@ -54,6 +54,7 @@ - jmargeta - johnpangalos - jonkoops +- jrakotoharisoa - kantuni - kddnewton - kentcdodds @@ -113,4 +114,3 @@ - xavier-lc - xcsnowcity - yuleicul -- jrakotoharisoa From 5b71b95a9709e2c7560dc8d8afbd48d6930ad7ba Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 27 Sep 2022 09:51:19 -0400 Subject: [PATCH 3/4] use destination location and add unit test --- packages/router/__tests__/router-test.ts | 80 ++++++++++++++++-------- packages/router/router.ts | 11 ++-- 2 files changed, 60 insertions(+), 31 deletions(-) diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index ad40ae4ad5..f66b7f621b 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -2706,49 +2706,79 @@ describe("a router", () => { expect(t.router.state.actionData).toBeNull(); }); - it("removes action data after action redirect (return)", async () => { + it("removes action data after action redirect (w/o loaders to run)", async () => { let t = setup({ routes: [ { - path: "/", - id: "root", + index: true, + id: "index", + action: true, + }, + { + path: "/other", + id: "other", + }, + ], + }); + 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" }), + }); + await B.actions.index.redirectReturn("/other"); + + expect(t.router.state.actionData).toBeNull(); + }); + + it("removes action data after action redirect (w/ loaders to run)", async () => { + let t = setup({ + routes: [ + { + index: true, + id: "index", + action: true, + }, + { + path: "/other", + id: "other", loader: true, - hasErrorBoundary: true, - children: [ - { - index: true, - id: "child", - action: true, - }, - { - path: "/other", - id: "other", - loader: true, - action: true, - }, - ], }, ], }); - let A = await t.navigate("/?index", { + let A = await t.navigate("/", { formMethod: "post", formData: createFormData({ gosh: "" }), }); - await A.actions.child.resolve({ error: "invalid" }); - await A.loaders.root.resolve("ROOT LOADER"); + await A.actions.index.resolve({ error: "invalid" }); expect(t.router.state.actionData).toEqual({ - child: { error: "invalid" }, + index: { error: "invalid" }, }); - let B = await t.navigate("/?index", { + let B = await t.navigate("/", { formMethod: "post", formData: createFormData({ gosh: "dang" }), }); - let C = await B.actions.child.redirectReturn("/other"); - await C.loaders.root.resolve("ROOT LOADER"); - await C.loaders.other.resolve("OTHER LOADER"); + + let C = await B.actions.index.redirectReturn("/other"); + expect(t.router.state.actionData).toEqual({ + index: { error: "invalid" }, + }); + expect(t.router.state.loaderData).toEqual({}); + + await C.loaders.other.resolve("OTHER"); expect(t.router.state.actionData).toBeNull(); + expect(t.router.state.loaderData).toEqual({ + other: "OTHER", + }); }); it("uses the proper action for index routes", async () => { diff --git a/packages/router/router.ts b/packages/router/router.ts index 699c07b0a9..d670a6ae43 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -660,16 +660,15 @@ export function createRouter(init: RouterInit): Router { // - We have committed actionData in the store // - The current navigation was a submission // - We're past the submitting state and into the loading state - // - This should not be susceptible to false positives for - // loading/submissionRedirect since there would not be actionData in the - // state since the prior action would have returned a redirect response - // and short circuited + // - The location we're landing at is different from the submission + // location, indicating we redirected from the action (avoids false + // positives for loading/submissionRedirect when actionData returned + // on a prior submiission) let isActionReload = state.actionData != null && state.navigation.formMethod != null && state.navigation.state === "loading" && - state.navigation.formAction?.split("?")[0] === - state.navigation.location.pathname; + state.navigation.formAction?.split("?")[0] === location.pathname; // Always preserve any existing loaderData from re-used routes let newLoaderData = newState.loaderData From 53d58dc6d1dcf2e3f147e10dc395fdfc2978db4a Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 27 Sep 2022 09:57:25 -0400 Subject: [PATCH 4/4] Update changelog + comments --- .changeset/short-hats-peel.md | 2 +- packages/router/router.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.changeset/short-hats-peel.md b/.changeset/short-hats-peel.md index 2081648025..c11a20b037 100644 --- a/.changeset/short-hats-peel.md +++ b/.changeset/short-hats-peel.md @@ -2,4 +2,4 @@ "@remix-run/router": patch --- -reset action data after redirection trigger by action +fix: reset `actionData` after successful action redirect (#9334) diff --git a/packages/router/router.ts b/packages/router/router.ts index d670a6ae43..f9a863f172 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -660,10 +660,10 @@ export function createRouter(init: RouterInit): Router { // - We have committed actionData in the store // - The current navigation was a submission // - We're past the submitting state and into the loading state - // - The location we're landing at is different from the submission + // - 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 submiission) + // on a prior submission) let isActionReload = state.actionData != null && state.navigation.formMethod != null &&