Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Dec 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/new-news-remember.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/router": patch
---

Reset `actionData` on action redirect to current location
51 changes: 50 additions & 1 deletion packages/router/__tests__/router-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2844,13 +2844,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",
Expand Down Expand Up @@ -2882,8 +2886,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");
Expand All @@ -2894,6 +2901,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",
Expand Down Expand Up @@ -3069,6 +3079,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: [
Expand Down
10 changes: 4 additions & 6 deletions packages/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a naive way to look for "not a redirect" before, but now we have state._isRedirect since we did the Remix back compat layer which avoids false negatives on redirecting to the current location


let actionData: RouteData | null;
if (newState.actionData) {
Expand Down