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

Allow revalidations to complete if submitting fetcher is deleted #10535

Merged
merged 1 commit into from Jun 1, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/delete-submit-fetcher.md
@@ -0,0 +1,5 @@
---
"@remix-run/router": patch
---

Allow fetcher revalidations to complete if submitting fetcher is deleted
60 changes: 60 additions & 0 deletions packages/router/__tests__/router-test.ts
Expand Up @@ -10317,6 +10317,66 @@ describe("a router", () => {
data: "TASKS ACTION",
});
});

it("handles revalidating fetcher when the triggering fetcher is deleted", async () => {
let key = "key";
let actionKey = "actionKey";
let t = setup({
routes: [
{
id: "root",
path: "/",
children: [
{
id: "home",
index: true,
loader: true,
},
{
id: "action",
path: "action",
action: true,
},
{
id: "fetch",
path: "fetch",
loader: true,
},
],
},
],
hydrationData: { loaderData: { home: "HOME" } },
});

// Load a fetcher
let A = await t.fetch("/fetch", key);
await A.loaders.fetch.resolve("FETCH");

// Submit a different fetcher, which will trigger revalidation
let B = await t.fetch("/action", actionKey, {
formMethod: "post",
formData: createFormData({}),
});
t.shimHelper(B.loaders, "fetch", "loader", "fetch");

// After action resolves, both fetchers go into a loading state
await B.actions.action.resolve("ACTION");
expect(t.router.state.fetchers.get(key)?.state).toBe("loading");
expect(t.router.state.fetchers.get(actionKey)?.state).toBe("loading");

// Remove the submitting fetcher (assume it's component unmounts)
t.router.deleteFetcher(actionKey);

await B.loaders.home.resolve("HOME*");
await B.loaders.fetch.resolve("FETCH*");

expect(t.router.state.loaderData).toEqual({ home: "HOME*" });
expect(t.router.state.fetchers.get(key)).toMatchObject({
state: "idle",
data: "FETCH*",
});
expect(t.router.state.fetchers.get(actionKey)).toBeUndefined();
});
});

describe("fetcher ?index params", () => {
Expand Down
40 changes: 27 additions & 13 deletions packages/router/router.ts
Expand Up @@ -1783,7 +1783,6 @@ export function createRouter(init: RouterInit): Router {
let nextLocation = state.navigation.location || state.location;
let revalidationRequest = createClientSideRequest(
init.history,

nextLocation,
abortController.signal
);
Expand Down Expand Up @@ -1894,16 +1893,20 @@ export function createRouter(init: RouterInit): Router {
activeDeferreds
);

let doneFetcher: FetcherStates["Idle"] = {
state: "idle",
data: actionResult.data,
formMethod: undefined,
formAction: undefined,
formEncType: undefined,
formData: undefined,
" _hasFetcherDoneAnything ": true,
};
state.fetchers.set(key, doneFetcher);
// Since we let revalidations complete even if the submitting fetcher was
// deleted, only put it back to idle if it hasn't been deleted
if (state.fetchers.has(key)) {
let doneFetcher: FetcherStates["Idle"] = {
state: "idle",
data: actionResult.data,
formMethod: undefined,
formAction: undefined,
formEncType: undefined,
formData: undefined,
" _hasFetcherDoneAnything ": true,
};
state.fetchers.set(key, doneFetcher);
}

let didAbortFetchLoads = abortStaleFetchLoads(loadId);

Expand Down Expand Up @@ -1935,7 +1938,9 @@ export function createRouter(init: RouterInit): Router {
matches,
errors
),
...(didAbortFetchLoads ? { fetchers: new Map(state.fetchers) } : {}),
...(didAbortFetchLoads || revalidatingFetchers.length > 0
? { fetchers: new Map(state.fetchers) }
: {}),
});
isRevalidationRequired = false;
}
Expand Down Expand Up @@ -2271,7 +2276,16 @@ export function createRouter(init: RouterInit): Router {
}

function deleteFetcher(key: string): void {
if (fetchControllers.has(key)) abortFetcher(key);
let fetcher = state.fetchers.get(key);
// Don't abort the controller if this is a deletion of a fetcher.submit()
// in it's loading phase since - we don't want to abort the corresponding
// revalidation and want them to complete and land
if (
fetchControllers.has(key) &&
!(fetcher && fetcher.state === "loading" && fetchReloadIds.has(key))
) {
abortFetcher(key);
}
fetchLoadMatches.delete(key);
fetchReloadIds.delete(key);
fetchRedirectIds.delete(key);
Expand Down