diff --git a/packages/react-router/.changes/patch.fix-fetcher-formdata-flicker.md b/packages/react-router/.changes/patch.fix-fetcher-formdata-flicker.md new file mode 100644 index 0000000000..47115b18d8 --- /dev/null +++ b/packages/react-router/.changes/patch.fix-fetcher-formdata-flicker.md @@ -0,0 +1 @@ +Update router to operate on fetcher Maps in an immutable manner to avoid delayed React renders from potentially reading an updated but not yet committed Map. This could result in brief flickers in some fetcher-driven optimistic UI scenarios. diff --git a/packages/react-router/.changes/patch.memoize-usefetchers.md b/packages/react-router/.changes/patch.memoize-usefetchers.md new file mode 100644 index 0000000000..bab5e57f17 --- /dev/null +++ b/packages/react-router/.changes/patch.memoize-usefetchers.md @@ -0,0 +1 @@ +Memoize `useFetchers` to return a stable identity and only change if fetchers changed diff --git a/packages/react-router/__tests__/dom/data-browser-router-test.tsx b/packages/react-router/__tests__/dom/data-browser-router-test.tsx index b1dbfcee4c..06bd9adb5d 100644 --- a/packages/react-router/__tests__/dom/data-browser-router-test.tsx +++ b/packages/react-router/__tests__/dom/data-browser-router-test.tsx @@ -5738,6 +5738,111 @@ function testDomRouter( `); }); + it("useFetchers returns stable array reference when fetchers are unchanged", async () => { + let fetchDfd = createDeferred(); + let fetchersArrays: (Fetcher & { key: string })[][] = []; + let setCountRef = { current: null as React.Dispatch> | null }; + + function Parent() { + let fetchers = useFetchers(); + let fetcher = useFetcher(); + let [, setCount] = React.useState(0); + setCountRef.current = setCount; + fetchersArrays.push(fetchers); + return ( + + ); + } + + let router = createTestRouter( + [ + { + path: "/", + Component: Parent, + children: [{ path: "/fetch", loader: () => fetchDfd.promise }], + }, + ], + { + window: getWindow("/"), + hydrationData: { loaderData: { "0": null } }, + }, + ); + render(); + + // Wait for initial mount + await waitFor(() => screen.getByText("load")); + expect(fetchersArrays.at(-1)).toEqual([]); + + // Trigger a fetch — fetchers array should have a loading fetcher + fireEvent.click(screen.getByText("load")); + await waitFor(() => + expect(fetchersArrays.at(-1)).toEqual( + expect.arrayContaining([ + expect.objectContaining({ state: "loading" }), + ]), + ), + ); + let arrayWhileLoading = fetchersArrays.at(-1)!; + + // Unrelated state update re-renders the component — array ref should be stable + act(() => setCountRef.current!((c) => c + 1)); + expect(fetchersArrays.at(-1)).toBe(arrayWhileLoading); + + // Resolve the fetch — fetchers go idle and are removed from useFetchers + fetchDfd.resolve("DATA"); + await waitFor(() => expect(fetchersArrays.at(-1)).toEqual([])); + + // The final empty array is a new reference (fetchers changed) + expect(fetchersArrays.at(-1)).not.toBe(arrayWhileLoading); + }); + + it("useFetchers updates when a fetcher transitions state", async () => { + let fetchDfd = createDeferred(); + let states: string[] = []; + + function Parent() { + let fetchers = useFetchers(); + let fetcher = useFetcher(); + states.push(fetchers.map((f) => f.state).join(",") || "empty"); + return ( + + ); + } + + let router = createTestRouter( + [ + { + path: "/", + Component: Parent, + children: [{ path: "/fetch", loader: () => fetchDfd.promise }], + }, + ], + { + window: getWindow("/"), + hydrationData: { loaderData: { "0": null } }, + }, + ); + render(); + + // Wait for initial mount + await waitFor(() => screen.getByText("load")); + expect(states.at(-1)).toBe("empty"); + + // Fetch starts — useFetchers should see "loading" + fireEvent.click(screen.getByText("load")); + await waitFor(() => expect(states).toContain("loading")); + + // Fetch completes — useFetchers should go back to empty (idle fetchers excluded) + fetchDfd.resolve("DATA"); + await waitFor(() => expect(states.at(-1)).toBe("empty")); + + // States should have progressed: …empty → loading → empty + expect(states).toContain("loading"); + let loadingIdx = states.lastIndexOf("loading"); + let lastEmptyIdx = states.lastIndexOf("empty"); + expect(lastEmptyIdx).toBeGreaterThan(loadingIdx); + }); + it("handles revalidating fetchers", async () => { let count = 0; let fetchCount = 0; @@ -6501,10 +6606,10 @@ function testDomRouter( expect(container.querySelector("pre")?.innerHTML).toBe(""); fireEvent.click(screen.getByText("Load fetchers")); await waitFor(() => - // React `useId()` results in something such as `_r_2k_` or `_r_u_` - // depending on `DataBrowserRouter`/`DataHashRouter` + // React `useId()` results in something such as `_r_2k_`, `_r_u_`, + // or `_r_11_` depending on React version and component tree depth expect(container.querySelector("pre")?.innerHTML).toMatch( - /^_r_[0-9]?[a-z]_,my-key$/, + /^_r_[0-9a-z]+_,my-key$/, ), ); }); diff --git a/packages/react-router/__tests__/router/fetchers-test.ts b/packages/react-router/__tests__/router/fetchers-test.ts index 4f42d4a36c..fea1fc24b4 100644 --- a/packages/react-router/__tests__/router/fetchers-test.ts +++ b/packages/react-router/__tests__/router/fetchers-test.ts @@ -1,5 +1,5 @@ /* eslint-disable jest/valid-title */ -import type { HydrationState } from "../../lib/router/router"; +import type { Fetcher, HydrationState } from "../../lib/router/router"; import { createMemoryHistory } from "../../lib/router/history"; import { createRouter, @@ -3703,4 +3703,259 @@ describe("fetchers", () => { expect(A.loaders.fetch.signal.reason).toBe("BECAUSE I SAID SO"); }); }); + + describe("fetcher Map mutation", () => { + // The root cause of the bug: after updateState({ fetchers: new Map(...) }) + // hands a Map (MapA) to React, a subsequent direct mutation of + // state.fetchers (e.g. state.fetchers.set(key, getDoneFetcher())) mutates + // that same MapA because state.fetchers === MapA after the updateState. + // React's concurrent renderer may still hold MapA and render it post- + // mutation, seeing an idle fetcher (formData gone) alongside stale + // loaderData — the "flicker". + // + // The subscriber-based approach does NOT catch this: the subscriber is + // called synchronously after each updateState, so it only ever sees + // fully-settled state. Instead, the test captures the Map reference handed + // to the subscriber during the "loading" phase and, after the fetch + // completes, asserts that reference was never mutated in place. + it("does not mutate the Map reference handed to subscribers (fetcher.submit)", async () => { + let fetcherMaps: Map[] = []; + let itemStatus = false; + + let router = createRouter({ + history: createMemoryHistory({ initialEntries: ["/item"] }), + routes: [ + { + id: "item", + path: "/item", + loader: () => ({ status: itemStatus }), + action: async ({ request }) => { + let formData = await request.formData(); + itemStatus = formData.get("status") === "true"; + return { ok: true }; + }, + }, + ], + hydrationData: { + loaderData: { item: { status: false } }, + }, + }); + + router.initialize(); + + // Mount the fetcher so it stays in state + router.getFetcher("toggle"); + + router.subscribe((state) => { + fetcherMaps.push(state.fetchers); + }); + + let formData = new FormData(); + formData.append("status", "true"); + + await router.fetch("toggle", "item", "/item", { + formMethod: "POST", + formData, + }); + + router.dispose(); + + // After the fetch fully completes, the Maps we captured during the + // submitting/loading phases must still reflect submitting/loading — it must not have been + // mutated to "idle" in place + expect(fetcherMaps.length).toBe(3); + expect(fetcherMaps[0].get("toggle")?.state).toBe("submitting"); + expect(fetcherMaps[0].get("toggle")?.formData).toBeDefined(); + expect(fetcherMaps[1].get("toggle")?.state).toBe("loading"); + expect(fetcherMaps[1].get("toggle")?.formData).toBeDefined(); + expect(fetcherMaps[2].get("toggle")).toBeUndefined(); + }); + + it("does not mutate the Map reference handed to subscribers (fetcher.load)", async () => { + // updateFetcherState() does: state.fetchers.set(key, fetcher); updateState({fetchers: new Map(state.fetchers)}). + // After the first call (loading state), state.fetchers === MapA which React holds. + // The second call (done state) mutates MapA via state.fetchers.set before creating MapB. + let fetcherMaps: Map[] = []; + + let router = createRouter({ + history: createMemoryHistory({ initialEntries: ["/"] }), + routes: [ + { id: "root", path: "/", loader: () => ({ data: "root" }) }, + { id: "item", path: "/item", loader: () => ({ data: "item" }) }, + ], + hydrationData: { loaderData: { root: { data: "root" } } }, + }); + + router.initialize(); + router.getFetcher("load"); + + router.subscribe((state) => { + fetcherMaps.push(state.fetchers); + }); + + await router.fetch("load", "root", "/item"); + + router.dispose(); + + // After the fetch fully completes, the Maps we captured during the + // loading phase must still reflect loading — they must not have been + // mutated to "idle" in place. + expect(fetcherMaps.length).toBe(2); + expect(fetcherMaps[0].get("load")?.state).toBe("loading"); + expect(fetcherMaps[0].get("load")?.data).toBeUndefined(); + expect(fetcherMaps[1].get("load")).toBeUndefined(); + }); + + it("does not mutate the Map reference handed to subscribers (fetcher revalidation during navigation)", async () => { + // getUpdatedRevalidatingFetchers() (dev branch) calls state.fetchers.set() + // on the current Map before returning a copy. This mutates MapPrev. + // Later, processLoaderData mutates the Map that subscribers received for + // the "loading" revalidation state. Test that the subscriber's loading + // Map is not mutated to idle by processLoaderData after loaders complete. + let fetcherMaps: Map[] = []; + + let router = createRouter({ + history: createMemoryHistory({ initialEntries: ["/"] }), + routes: [ + { + id: "root", + path: "/", + loader: () => ({ data: "root" }), + action: () => ({ ok: true }), + }, + ], + hydrationData: { loaderData: { root: { data: "root" } } }, + }); + + router.initialize(); + router.getFetcher("f"); + + router.subscribe((state) => { + fetcherMaps.push(state.fetchers); + }); + + // GET load to register the fetcher in fetchLoadMatches (eligible for + // revalidation when a subsequent navigation action fires). + await router.fetch("f", "root", "/"); + + // POST to the same route — sets isRevalidationRequired=true and causes + // fetcher "f" to revalidate alongside the route loaders. + let formData = new FormData(); + await router.navigate("/", { formMethod: "POST", formData }); + + router.dispose(); + + // After the navigation fully completes, the Maps we captured during the + // loading phases (initial fetch + revalidation) must still reflect + // loading — they must not have been mutated to "idle"/done in place by + // getUpdatedRevalidatingFetchers/processLoaderData. + expect(fetcherMaps.length).toBe(5); + expect(fetcherMaps[0].get("f")?.state).toBe("loading"); + expect(fetcherMaps[1].get("f")).toBeUndefined(); + expect(fetcherMaps[2].get("f")).toBeUndefined(); + expect(fetcherMaps[3].get("f")?.state).toBe("loading"); + expect(fetcherMaps[4].get("f")).toBeUndefined(); + }); + + it("does not mutate the Map reference handed to subscribers (fetcher loader redirect)", async () => { + // handleFetcherLoader hands MapA (loading) to React via updateFetcherState(). + // When the loader returns a redirect, markFetchRedirectsDone() calls + // state.fetchers.set(key, doneFetcher) which mutates MapA before the + // final completeNavigation updateState creates MapB. + let fetcherMaps: Map[] = []; + + let router = createRouter({ + history: createMemoryHistory({ initialEntries: ["/"] }), + routes: [ + { id: "root", path: "/", loader: () => ({ data: "root" }) }, + { + id: "redirect-source", + path: "/redirect", + loader: () => + new Response(null, { + status: 302, + headers: { Location: "/" }, + }), + }, + ], + hydrationData: { loaderData: { root: { data: "root" } } }, + }); + + router.initialize(); + router.getFetcher("redir"); + + router.subscribe((state) => { + fetcherMaps.push(state.fetchers); + }); + + await router.fetch("redir", "root", "/redirect"); + + router.dispose(); + + // After the redirect navigation fully completes, the Maps captured + // during the loading phases must still reflect loading — they must not + // have been mutated to "idle"/done in place by markFetchRedirectsDone(). + expect(fetcherMaps.length).toBe(3); + expect(fetcherMaps[0].get("redir")?.state).toBe("loading"); + expect(fetcherMaps[1].get("redir")?.state).toBe("loading"); + expect(fetcherMaps[2].get("redir")).toBeUndefined(); + }); + + it("does not mutate the Map reference handed to subscribers (fetcher action redirect)", async () => { + // When a fetcher action returns a redirect, handleFetcherAction calls + // updateFetcherState(key, getLoadingFetcher(submission)) which first + // mutates the "submitting" MapA via state.fetchers.set, creates MapB + // (loading), then kicks off startRedirectNavigation. Inside + // completeNavigation, markFetchRedirectsDone() mutates MapB + // (state.fetchers.set(key, doneFetcher)) before the final updateState + // creates MapC. Test that MapB is not mutated. + let fetcherMaps: Map[] = []; + + let router = createRouter({ + history: createMemoryHistory({ initialEntries: ["/"] }), + routes: [ + { id: "root", path: "/", loader: () => ({ data: "root" }) }, + { + id: "action-route", + path: "/action", + action: () => + new Response(null, { + status: 302, + headers: { Location: "/" }, + }), + }, + ], + hydrationData: { loaderData: { root: { data: "root" } } }, + }); + + router.initialize(); + router.getFetcher("act"); + + router.subscribe((state) => { + fetcherMaps.push(state.fetchers); + }); + + let formData = new FormData(); + await router.fetch("act", "root", "/action", { + formMethod: "POST", + formData, + }); + + router.dispose(); + + // After the redirect navigation fully completes, the Maps captured + // during the submitting/loading phases must still reflect those + // states (with formData intact) — they must not have been mutated to + // "idle"/done in place by markFetchRedirectsDone() before + // completeNavigation finalizes. + expect(fetcherMaps.length).toBe(4); + expect(fetcherMaps[0].get("act")?.state).toBe("submitting"); + expect(fetcherMaps[0].get("act")?.formData).toBeDefined(); + expect(fetcherMaps[1].get("act")?.state).toBe("loading"); + expect(fetcherMaps[1].get("act")?.formData).toBeDefined(); + expect(fetcherMaps[2].get("act")?.state).toBe("loading"); + expect(fetcherMaps[2].get("act")?.formData).toBeDefined(); + expect(fetcherMaps[3].get("act")).toBeUndefined(); + }); + }); }); diff --git a/packages/react-router/lib/dom/lib.tsx b/packages/react-router/lib/dom/lib.tsx index 9f6dde80bf..7ace344b1c 100644 --- a/packages/react-router/lib/dom/lib.tsx +++ b/packages/react-router/lib/dom/lib.tsx @@ -3000,10 +3000,14 @@ export function useFetcher({ */ export function useFetchers(): (Fetcher & { key: string })[] { let state = useDataRouterState(DataRouterStateHook.UseFetchers); - return Array.from(state.fetchers.entries()).map(([key, fetcher]) => ({ - ...fetcher, - key, - })); + return React.useMemo( + () => + Array.from(state.fetchers.entries()).map(([key, fetcher]) => ({ + ...fetcher, + key, + })), + [state.fetchers], + ); } const SCROLL_RESTORATION_STORAGE_KEY = "react-router-scroll-positions"; diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index c0ce6887e8..89cf9380c4 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -841,6 +841,10 @@ interface HandleLoadersResult extends ShortCircuitable { * errors thrown from the current set of loaders */ errors?: RouterState["errors"]; + /** + * Updated fetchers map (when stale fetch loads were aborted or redirects completed) + */ + workingFetchers?: RouterState["fetchers"]; } /** @@ -1377,7 +1381,7 @@ export function createRouter(init: RouterInit): Router { } subscribers.clear(); pendingNavigationController && pendingNavigationController.abort(); - state.fetchers.forEach((_, key) => deleteFetcher(key)); + state.fetchers.forEach((_, key) => deleteFetcher(state.fetchers, key)); state.blockers.forEach((_, key) => deleteBlocker(key)); } @@ -1460,8 +1464,10 @@ export function createRouter(init: RouterInit): Router { }), ); - // Cleanup internally now that we've called our subscribers/updated state - unmountedFetchers.forEach((key) => deleteFetcher(key)); + // Cleanup after subscribers have been called. Unmounted fetchers are fully + // removed; mounted idle fetchers are removed from state.fetchers only (they + // stay in fetchLoadMatches etc. in case they're re-used). + unmountedFetchers.forEach((key) => deleteFetcher(state.fetchers, key)); mountedFetchers.forEach((key) => state.fetchers.delete(key)); } @@ -2022,6 +2028,7 @@ export function createRouter(init: RouterInit): Router { matches: updatedMatches, loaderData, errors, + workingFetchers, } = await handleLoaders( request, location, @@ -2053,6 +2060,7 @@ export function createRouter(init: RouterInit): Router { ...getActionDataForCommit(pendingActionResult), loaderData, errors, + ...(workingFetchers ? { fetchers: workingFetchers } : {}), }); } @@ -2382,7 +2390,8 @@ export function createRouter(init: RouterInit): Router { ) && revalidatingFetchers.length === 0 ) { - let updatedFetchers = markFetchRedirectsDone(); + let workingFetchers = new Map(state.fetchers); + let didUpdateFetcherRedirects = markFetchRedirectsDone(workingFetchers); completeNavigation( location, { @@ -2394,7 +2403,7 @@ export function createRouter(init: RouterInit): Router { ? { [pendingActionResult[0]]: pendingActionResult[1].error } : null, ...getActionDataForCommit(pendingActionResult), - ...(updatedFetchers ? { fetchers: new Map(state.fetchers) } : {}), + ...(didUpdateFetcherRedirects ? { fetchers: workingFetchers } : {}), }, { flushSync }, ); @@ -2484,6 +2493,7 @@ export function createRouter(init: RouterInit): Router { } // Process and commit output from loaders + let workingFetchers = new Map(state.fetchers); let { loaderData, errors } = processLoaderData( state, matches, @@ -2491,6 +2501,7 @@ export function createRouter(init: RouterInit): Router { pendingActionResult, revalidatingFetchers, fetcherResults, + workingFetchers, ); // Preserve SSR errors during partial hydration @@ -2498,16 +2509,21 @@ export function createRouter(init: RouterInit): Router { errors = { ...state.errors, ...errors }; } - let updatedFetchers = markFetchRedirectsDone(); - let didAbortFetchLoads = abortStaleFetchLoads(pendingNavigationLoadId); + let didUpdateFetcherRedirects = markFetchRedirectsDone(workingFetchers); + let didAbortFetchLoads = abortStaleFetchLoads( + pendingNavigationLoadId, + workingFetchers, + ); let shouldUpdateFetchers = - updatedFetchers || didAbortFetchLoads || revalidatingFetchers.length > 0; + didUpdateFetcherRedirects || + didAbortFetchLoads || + revalidatingFetchers.length > 0; return { matches, loaderData, errors, - ...(shouldUpdateFetchers ? { fetchers: new Map(state.fetchers) } : {}), + ...(shouldUpdateFetchers ? { workingFetchers } : {}), }; } @@ -2533,15 +2549,16 @@ export function createRouter(init: RouterInit): Router { function getUpdatedRevalidatingFetchers( revalidatingFetchers: RevalidatingFetcher[], ) { + let workingFetchers = new Map(state.fetchers); revalidatingFetchers.forEach((rf) => { - let fetcher = state.fetchers.get(rf.key); + let fetcher = workingFetchers.get(rf.key); let revalidatingFetcher = getLoadingFetcher( undefined, fetcher ? fetcher.data : undefined, ); - state.fetchers.set(rf.key, revalidatingFetcher); + workingFetchers.set(rf.key, revalidatingFetcher); }); - return new Map(state.fetchers); + return workingFetchers; } // Trigger a fetcher load/submit for the given fetcher key @@ -2808,9 +2825,6 @@ export function createRouter(init: RouterInit): Router { let loadId = ++incrementingLoadId; fetchReloadIds.set(key, loadId); - let loadFetcher = getLoadingFetcher(submission, actionResult.data); - state.fetchers.set(key, loadFetcher); - let { dsMatches, revalidatingFetchers } = getMatchesToLoad( revalidationRequest, scopedContext, @@ -2836,6 +2850,14 @@ export function createRouter(init: RouterInit): Router { callSiteDefaultShouldRevalidate, ); + // Build an updated fetchers map for the updateState call below without + // mutating state.fetchers. Set the submitting fetcher into loading state + // and put all revalidating fetchers (except the current one) into loading + // state as well. + let loadFetcher = getLoadingFetcher(submission, actionResult.data); + let workingFetchers = new Map(state.fetchers); + workingFetchers.set(key, loadFetcher); + // Put all revalidating fetchers into the loading state, except for the // current fetcher which we want to keep in it's current loading state which // contains it's action submission info + action data @@ -2843,19 +2865,19 @@ export function createRouter(init: RouterInit): Router { .filter((rf) => rf.key !== key) .forEach((rf) => { let staleKey = rf.key; - let existingFetcher = state.fetchers.get(staleKey); + let existingFetcher = workingFetchers.get(staleKey); let revalidatingFetcher = getLoadingFetcher( undefined, existingFetcher ? existingFetcher.data : undefined, ); - state.fetchers.set(staleKey, revalidatingFetcher); + workingFetchers.set(staleKey, revalidatingFetcher); abortFetcher(staleKey); if (rf.controller) { fetchControllers.set(staleKey, rf.controller); } }); - updateState({ fetchers: new Map(state.fetchers) }); + updateState({ fetchers: workingFetchers }); let abortPendingFetchRevalidations = () => revalidatingFetchers.forEach((rf) => abortFetcher(rf.key)); @@ -2887,15 +2909,25 @@ export function createRouter(init: RouterInit): Router { fetchControllers.delete(key); revalidatingFetchers.forEach((r) => fetchControllers.delete(r.key)); - // 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 = getDoneFetcher(actionResult.data); - state.fetchers.set(key, doneFetcher); - } + let fetcherIsMounted = state.fetchers.has(key); + + // Generate a new local copy of `state` for the redirect navigation to read + // from and eventually commit. This avoids mutating the existing state.fetchers + // map Map which React already holds a reference to from updateState() above + let getRedirectStateWithDoneFetcher = (s: RouterState) => { + // 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 (!fetcherIsMounted) return s; + let workingFetchers = new Map(s.fetchers); + workingFetchers.set(key, getDoneFetcher(actionResult.data)); + return { ...s, fetchers: workingFetchers }; + }; let redirect = findRedirect(loaderResults); if (redirect) { + // Advance state.fetchers to include the done fetcher before handing off + // to the redirect navigation so that completeNavigation() sees it as idle. + state = getRedirectStateWithDoneFetcher(state); return startRedirectNavigation( revalidationRequest, redirect.result, @@ -2910,6 +2942,7 @@ export function createRouter(init: RouterInit): Router { // fetchRedirectIds so it doesn't get revalidated on the next set of // loader executions fetchRedirectIds.add(redirect.key); + state = getRedirectStateWithDoneFetcher(state); return startRedirectNavigation( revalidationRequest, redirect.result, @@ -2918,6 +2951,16 @@ export function createRouter(init: RouterInit): Router { ); } + // Build finalFetchers before processing so that processLoaderData and + // abortStaleFetchLoads can write revalidating-fetcher results into it + // alongside the done-fetcher for this action. Using a fresh Map ensures + // we never mutate the Map that was handed to React via the earlier + // updateState() call (see #14506). + let finalFetchers = new Map(state.fetchers); + if (fetcherIsMounted) { + finalFetchers.set(key, getDoneFetcher(actionResult.data)); + } + // Process and commit output from loaders let { loaderData, errors } = processLoaderData( state, @@ -2926,9 +2969,10 @@ export function createRouter(init: RouterInit): Router { undefined, revalidatingFetchers, fetcherResults, + finalFetchers, ); - abortStaleFetchLoads(loadId); + abortStaleFetchLoads(loadId, finalFetchers); // If we are currently in a navigation loading state and this fetcher is // more recent than the navigation, we want the newer data so abort the @@ -2944,7 +2988,7 @@ export function createRouter(init: RouterInit): Router { matches, loaderData, errors, - fetchers: new Map(state.fetchers), + fetchers: finalFetchers, }); } else { // otherwise just update with the fetcher data, preserving any existing @@ -2958,7 +3002,7 @@ export function createRouter(init: RouterInit): Router { matches, errors, ), - fetchers: new Map(state.fetchers), + fetchers: finalFetchers, }); isRevalidationRequired = false; } @@ -3408,9 +3452,10 @@ export function createRouter(init: RouterInit): Router { fetcher: Fetcher, opts: { flushSync?: boolean } = {}, ) { - state.fetchers.set(key, fetcher); + let workingFetchers = new Map(state.fetchers); + workingFetchers.set(key, fetcher); updateState( - { fetchers: new Map(state.fetchers) }, + { fetchers: workingFetchers }, { flushSync: (opts && opts.flushSync) === true }, ); } @@ -3422,13 +3467,17 @@ export function createRouter(init: RouterInit): Router { opts: { flushSync?: boolean } = {}, ) { let boundaryMatch = findNearestBoundary(state.matches, routeId); - deleteFetcher(key); + // Build the new Map first and delete from there, we don't want to mutate the map React + // already has a reference to. It will be removed from `state.fetchers` on a + // subsequent updateState() call + let workingFetchers = new Map(state.fetchers); + deleteFetcher(workingFetchers, key); updateState( { errors: { [boundaryMatch.route.id]: error, }, - fetchers: new Map(state.fetchers), + fetchers: workingFetchers, }, { flushSync: (opts && opts.flushSync) === true }, ); @@ -3449,7 +3498,7 @@ export function createRouter(init: RouterInit): Router { updateFetcherState(key, getDoneFetcher(null)); } - function deleteFetcher(key: string): void { + function deleteFetcher(fetchers: Map, key: string): void { 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 @@ -3465,7 +3514,7 @@ export function createRouter(init: RouterInit): Router { fetchRedirectIds.delete(key); fetchersQueuedForDeletion.delete(key); cancelledFetcherLoads.delete(key); - state.fetchers.delete(key); + fetchers.delete(key); } function queueFetcherForDeletion(key: string): void { @@ -3487,35 +3536,39 @@ export function createRouter(init: RouterInit): Router { } } - function markFetchersDone(keys: string[]) { + function markFetchersDone(keys: string[], fetchers: Map) { for (let key of keys) { - let fetcher = getFetcher(key); + let fetcher = fetchers.get(key); + invariant(fetcher, `Expected fetcher: ${key}`); let doneFetcher = getDoneFetcher(fetcher.data); - state.fetchers.set(key, doneFetcher); + fetchers.set(key, doneFetcher); } } - function markFetchRedirectsDone(): boolean { + function markFetchRedirectsDone(fetchers: Map): boolean { let doneKeys = []; - let updatedFetchers = false; + let didUpdateFetchers = false; for (let key of fetchRedirectIds) { - let fetcher = state.fetchers.get(key); + let fetcher = fetchers.get(key); invariant(fetcher, `Expected fetcher: ${key}`); if (fetcher.state === "loading") { fetchRedirectIds.delete(key); doneKeys.push(key); - updatedFetchers = true; + didUpdateFetchers = true; } } - markFetchersDone(doneKeys); - return updatedFetchers; + markFetchersDone(doneKeys, fetchers); + return didUpdateFetchers; } - function abortStaleFetchLoads(landedId: number): boolean { + function abortStaleFetchLoads( + landedId: number, + fetchers: Map, + ): boolean { let yeetedKeys = []; for (let [key, id] of fetchReloadIds) { if (id < landedId) { - let fetcher = state.fetchers.get(key); + let fetcher = fetchers.get(key); invariant(fetcher, `Expected fetcher: ${key}`); if (fetcher.state === "loading") { abortFetcher(key); @@ -3524,7 +3577,7 @@ export function createRouter(init: RouterInit): Router { } } } - markFetchersDone(yeetedKeys); + markFetchersDone(yeetedKeys, fetchers); return yeetedKeys.length > 0; } @@ -6981,6 +7034,7 @@ function processLoaderData( pendingActionResult: PendingActionResult | undefined, revalidatingFetchers: RevalidatingFetcher[], fetcherResults: Record, + workingFetchers: Map, ): { loaderData: RouterState["loaderData"]; errors?: RouterState["errors"]; @@ -7015,14 +7069,14 @@ function processLoaderData( [boundaryMatch.route.id]: result.error, }; } - state.fetchers.delete(key); + workingFetchers.delete(key); } else if (isRedirectResult(result)) { // Should never get here, redirects should get processed above, but we // keep this to type narrow to a success result in the else invariant(false, "Unhandled fetcher revalidation redirect"); } else { let doneFetcher = getDoneFetcher(result.data); - state.fetchers.set(key, doneFetcher); + workingFetchers.set(key, doneFetcher); } });