From 3260bfa272ab510340fb00dd4ab80fc32116cfc9 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Fri, 8 May 2026 14:51:28 -0400 Subject: [PATCH 01/10] fix: prevent fetcher formData flicker when loaders revalidate after action (#14506) When a fetcher action completes and revalidation loaders run, the router was calling state.fetchers.set(key, doneFetcher) BEFORE checking for redirects and before the final updateState/completeNavigation call. This mutated the same Map object that had already been handed to React via a prior updateState() call. Because React renders startTransition updates asynchronously, if React rendered between the mutation and the next updateState(), it would see: - fetcher.formData === undefined (already idle) - loaderData === old value (not yet updated) This caused a brief flicker in optimistic UI patterns like: fetcher.formData?.get('status') ?? item.status The fix defers the fetcher idle-state assignment to the final state update (non-redirect path) so it's committed atomically alongside the new loaderData. For redirect paths the mutation is preserved (needed so the redirect navigation's completeNavigation sees the idle fetcher). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../patch.fix-fetcher-formdata-flicker.md | 1 + .../__tests__/router/fetchers-test.ts | 80 +++++++++++++++++++ packages/react-router/lib/router/router.ts | 32 ++++++-- 3 files changed, 106 insertions(+), 7 deletions(-) create mode 100644 packages/react-router/.changes/patch.fix-fetcher-formdata-flicker.md 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..56b22bf352 --- /dev/null +++ b/packages/react-router/.changes/patch.fix-fetcher-formdata-flicker.md @@ -0,0 +1 @@ +Fix a race condition in fetcher state machine where `formData` briefly became `undefined` before new `loaderData` was available, causing a UI flicker in optimistic update patterns (#14506) diff --git a/packages/react-router/__tests__/router/fetchers-test.ts b/packages/react-router/__tests__/router/fetchers-test.ts index 4f42d4a36c..a85642de41 100644 --- a/packages/react-router/__tests__/router/fetchers-test.ts +++ b/packages/react-router/__tests__/router/fetchers-test.ts @@ -3703,4 +3703,84 @@ describe("fetchers", () => { expect(A.loaders.fetch.signal.reason).toBe("BECAUSE I SAID SO"); }); }); + + describe("fetcher optimistic UI flicker (#14506)", () => { + // The root cause of the bug: after updateState({ fetchers: new Map(...) }) + // hands a Map to React, subsequent direct mutations of state.fetchers (e.g. + // state.fetchers.set(key, getDoneFetcher())) mutate that same Map before + // React renders it. If React renders the earlier state snapshot after the + // mutation, it sees idle formData alongside stale loaderData — a flicker. + it("does not expose idle fetcher before new loaderData is committed", async () => { + // Capture every state the subscriber receives so we can verify that the + // subscriber never sees formData disappear while loaderData is still stale. + let subscriberStates: Array<{ + fetcherState: string; + hasFormData: boolean; + loaderStatus: boolean | undefined; + }> = []; + + 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) => { + let fetcher = state.fetchers.get("toggle"); + if (!fetcher) return; + let loaderData = state.loaderData["item"] as + | { status: boolean } + | undefined; + subscriberStates.push({ + fetcherState: fetcher.state, + hasFormData: fetcher.formData !== undefined, + loaderStatus: loaderData?.status, + }); + }); + + let formData = new FormData(); + formData.append("status", "true"); + + await router.fetch("toggle", "item", "/item", { + formMethod: "POST", + formData, + }); + + router.dispose(); + + // The subscriber must never see a state where formData is gone (idle/no + // submission info) while loaderData still has the old value (false). + // That combination is the "flicker" state described in #14506. + let flickerStates = subscriberStates.filter( + (s) => !s.hasFormData && s.loaderStatus === false, + ); + + expect(flickerStates).toEqual([]); + + // Sanity checks: we should have seen submitting/loading states with + // formData, and the final state should have the updated loaderData. + let lastState = subscriberStates[subscriberStates.length - 1]; + expect(lastState.loaderStatus).toBe(true); + }); + }); }); diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index c0ce6887e8..c0c083e1aa 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -2888,14 +2888,18 @@ export function createRouter(init: RouterInit): Router { 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); - } + // deleted, only put it back to idle if it hasn't been deleted. + // We track this now for use later, but defer the actual mutation. + let fetcherHasKey = state.fetchers.has(key); let redirect = findRedirect(loaderResults); if (redirect) { + // For redirect paths we still need to mutate state.fetchers so that the + // subsequent redirect navigation's completeNavigation() sees the fetcher + // as idle when it reads state.fetchers. + if (fetcherHasKey) { + state.fetchers.set(key, getDoneFetcher(actionResult.data)); + } return startRedirectNavigation( revalidationRequest, redirect.result, @@ -2909,6 +2913,9 @@ export function createRouter(init: RouterInit): Router { // If this redirect came from a fetcher make sure we mark it in // fetchRedirectIds so it doesn't get revalidated on the next set of // loader executions + if (fetcherHasKey) { + state.fetchers.set(key, getDoneFetcher(actionResult.data)); + } fetchRedirectIds.add(redirect.key); return startRedirectNavigation( revalidationRequest, @@ -2930,6 +2937,17 @@ export function createRouter(init: RouterInit): Router { abortStaleFetchLoads(loadId); + // Build the final fetchers Map atomically so that the done-fetcher + // transition and the new loaderData are committed in the same React state + // update. Do NOT mutate state.fetchers here: that Map was already handed + // to React via the updateState() call above (line ~2824) and mutating it + // before React renders would expose idle formData alongside stale + // loaderData — the flicker described in #14506. + let finalFetchers = new Map(state.fetchers); + if (fetcherHasKey) { + finalFetchers.set(key, getDoneFetcher(actionResult.data)); + } + // 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 // navigation and complete it with the fetcher data @@ -2944,7 +2962,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 +2976,7 @@ export function createRouter(init: RouterInit): Router { matches, errors, ), - fetchers: new Map(state.fetchers), + fetchers: finalFetchers, }); isRevalidationRequired = false; } From cc1bffd9387586f313a1234f7e2a8bbd2cc7a83c Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Fri, 8 May 2026 16:11:24 -0400 Subject: [PATCH 02/10] refactor: never mutate state.fetchers directly in router.ts Build a new Map whenever fetcher state needs to change instead of mutating the existing map in-place. The existing map may already have setState) and React renders asynchronously, so mutating it after the fact is unsafe. Key changes: - updateState: delete idle mountedFetchers only AFTER subscribers are notified (so subscribers can capture idle fetcher data), not before - HandleLoadersResult now carries an optional fetchers map so that abortStaleFetchLoads / markFetchRedirectsDone results are committed completeNavigation - getUpdatedRevalidatingFetchers: build a copy, don't mutate state.fetchers - handleFetcherAction: build updatedFetchers / finalFetchers maps instead of mutating state.fetchers; redirect paths advance state.fetchers via state = { ...state, fetchers: withDoneFetcher() } - updateFetcherState: build new Map before calling updateState - setFetcherError: build new Map before calling updateState - deleteFetcher: no longer deletes from state.fetchers; callers that need a React-facing snapshot must build their own Map and exclude the key - markFetchersDone / markFetchRedirectsDone / abortStaleFetchLoads: accept a fetchers Map parameter and operate on that instead of state.fetchers - processLoaderData: accepts workingFetchers Map parameter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- packages/react-router/lib/router/router.ts | 149 ++++++++++++++------- 1 file changed, 97 insertions(+), 52 deletions(-) diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index c0c083e1aa..b8326fbd5d 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) + */ + fetchers?: RouterState["fetchers"]; } /** @@ -1377,7 +1381,10 @@ export function createRouter(init: RouterInit): Router { } subscribers.clear(); pendingNavigationController && pendingNavigationController.abort(); - state.fetchers.forEach((_, key) => deleteFetcher(key)); + state.fetchers.forEach((_, key) => { + deleteFetcher(key); + state.fetchers.delete(key); + }); state.blockers.forEach((_, key) => deleteBlocker(key)); } @@ -1460,8 +1467,13 @@ 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(key); + state.fetchers.delete(key); + }); mountedFetchers.forEach((key) => state.fetchers.delete(key)); } @@ -2022,6 +2034,7 @@ export function createRouter(init: RouterInit): Router { matches: updatedMatches, loaderData, errors, + fetchers: updatedFetcherMap, } = await handleLoaders( request, location, @@ -2053,6 +2066,7 @@ export function createRouter(init: RouterInit): Router { ...getActionDataForCommit(pendingActionResult), loaderData, errors, + ...(updatedFetcherMap ? { fetchers: updatedFetcherMap } : {}), }); } @@ -2382,7 +2396,8 @@ export function createRouter(init: RouterInit): Router { ) && revalidatingFetchers.length === 0 ) { - let updatedFetchers = markFetchRedirectsDone(); + let fetcherMap = new Map(state.fetchers); + let updatedFetchers = markFetchRedirectsDone(fetcherMap); completeNavigation( location, { @@ -2394,7 +2409,7 @@ export function createRouter(init: RouterInit): Router { ? { [pendingActionResult[0]]: pendingActionResult[1].error } : null, ...getActionDataForCommit(pendingActionResult), - ...(updatedFetchers ? { fetchers: new Map(state.fetchers) } : {}), + ...(updatedFetchers ? { fetchers: fetcherMap } : {}), }, { flushSync }, ); @@ -2484,6 +2499,7 @@ export function createRouter(init: RouterInit): Router { } // Process and commit output from loaders + let fetcherMap = new Map(state.fetchers); let { loaderData, errors } = processLoaderData( state, matches, @@ -2491,6 +2507,7 @@ export function createRouter(init: RouterInit): Router { pendingActionResult, revalidatingFetchers, fetcherResults, + fetcherMap, ); // Preserve SSR errors during partial hydration @@ -2498,8 +2515,11 @@ export function createRouter(init: RouterInit): Router { errors = { ...state.errors, ...errors }; } - let updatedFetchers = markFetchRedirectsDone(); - let didAbortFetchLoads = abortStaleFetchLoads(pendingNavigationLoadId); + let updatedFetchers = markFetchRedirectsDone(fetcherMap); + let didAbortFetchLoads = abortStaleFetchLoads( + pendingNavigationLoadId, + fetcherMap, + ); let shouldUpdateFetchers = updatedFetchers || didAbortFetchLoads || revalidatingFetchers.length > 0; @@ -2507,7 +2527,7 @@ export function createRouter(init: RouterInit): Router { matches, loaderData, errors, - ...(shouldUpdateFetchers ? { fetchers: new Map(state.fetchers) } : {}), + ...(shouldUpdateFetchers ? { fetchers: fetcherMap } : {}), }; } @@ -2533,15 +2553,16 @@ export function createRouter(init: RouterInit): Router { function getUpdatedRevalidatingFetchers( revalidatingFetchers: RevalidatingFetcher[], ) { + let updatedFetchers = new Map(state.fetchers); revalidatingFetchers.forEach((rf) => { - let fetcher = state.fetchers.get(rf.key); + let fetcher = updatedFetchers.get(rf.key); let revalidatingFetcher = getLoadingFetcher( undefined, fetcher ? fetcher.data : undefined, ); - state.fetchers.set(rf.key, revalidatingFetcher); + updatedFetchers.set(rf.key, revalidatingFetcher); }); - return new Map(state.fetchers); + return updatedFetchers; } // Trigger a fetcher load/submit for the given fetcher key @@ -2809,7 +2830,6 @@ export function createRouter(init: RouterInit): Router { fetchReloadIds.set(key, loadId); let loadFetcher = getLoadingFetcher(submission, actionResult.data); - state.fetchers.set(key, loadFetcher); let { dsMatches, revalidatingFetchers } = getMatchesToLoad( revalidationRequest, @@ -2836,6 +2856,13 @@ 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 updatedFetchers = new Map(state.fetchers); + updatedFetchers.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 +2870,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 = updatedFetchers.get(staleKey); let revalidatingFetcher = getLoadingFetcher( undefined, existingFetcher ? existingFetcher.data : undefined, ); - state.fetchers.set(staleKey, revalidatingFetcher); + updatedFetchers.set(staleKey, revalidatingFetcher); abortFetcher(staleKey); if (rf.controller) { fetchControllers.set(staleKey, rf.controller); } }); - updateState({ fetchers: new Map(state.fetchers) }); + updateState({ fetchers: updatedFetchers }); let abortPendingFetchRevalidations = () => revalidatingFetchers.forEach((rf) => abortFetcher(rf.key)); @@ -2889,17 +2916,23 @@ export function createRouter(init: RouterInit): Router { // Since we let revalidations complete even if the submitting fetcher was // deleted, only put it back to idle if it hasn't been deleted. - // We track this now for use later, but defer the actual mutation. let fetcherHasKey = state.fetchers.has(key); + // Helper to produce a new state.fetchers Map with the done fetcher included + // without mutating the existing Map (which React may already hold a + // reference to from the updateState() call above). + let withDoneFetcher = () => { + if (!fetcherHasKey) return state.fetchers; + let fetchers = new Map(state.fetchers); + fetchers.set(key, getDoneFetcher(actionResult.data)); + return fetchers; + }; + let redirect = findRedirect(loaderResults); if (redirect) { - // For redirect paths we still need to mutate state.fetchers so that the - // subsequent redirect navigation's completeNavigation() sees the fetcher - // as idle when it reads state.fetchers. - if (fetcherHasKey) { - state.fetchers.set(key, getDoneFetcher(actionResult.data)); - } + // Advance state.fetchers to include the done fetcher before handing off + // to the redirect navigation so that completeNavigation() sees it as idle. + state = { ...state, fetchers: withDoneFetcher() }; return startRedirectNavigation( revalidationRequest, redirect.result, @@ -2913,10 +2946,8 @@ export function createRouter(init: RouterInit): Router { // If this redirect came from a fetcher make sure we mark it in // fetchRedirectIds so it doesn't get revalidated on the next set of // loader executions - if (fetcherHasKey) { - state.fetchers.set(key, getDoneFetcher(actionResult.data)); - } fetchRedirectIds.add(redirect.key); + state = { ...state, fetchers: withDoneFetcher() }; return startRedirectNavigation( revalidationRequest, redirect.result, @@ -2925,6 +2956,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 (fetcherHasKey) { + finalFetchers.set(key, getDoneFetcher(actionResult.data)); + } + // Process and commit output from loaders let { loaderData, errors } = processLoaderData( state, @@ -2933,20 +2974,10 @@ export function createRouter(init: RouterInit): Router { undefined, revalidatingFetchers, fetcherResults, + finalFetchers, ); - abortStaleFetchLoads(loadId); - - // Build the final fetchers Map atomically so that the done-fetcher - // transition and the new loaderData are committed in the same React state - // update. Do NOT mutate state.fetchers here: that Map was already handed - // to React via the updateState() call above (line ~2824) and mutating it - // before React renders would expose idle formData alongside stale - // loaderData — the flicker described in #14506. - let finalFetchers = new Map(state.fetchers); - if (fetcherHasKey) { - finalFetchers.set(key, getDoneFetcher(actionResult.data)); - } + 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 @@ -3426,9 +3457,10 @@ export function createRouter(init: RouterInit): Router { fetcher: Fetcher, opts: { flushSync?: boolean } = {}, ) { - state.fetchers.set(key, fetcher); + let fetchers = new Map(state.fetchers); + fetchers.set(key, fetcher); updateState( - { fetchers: new Map(state.fetchers) }, + { fetchers }, { flushSync: (opts && opts.flushSync) === true }, ); } @@ -3440,13 +3472,17 @@ export function createRouter(init: RouterInit): Router { opts: { flushSync?: boolean } = {}, ) { let boundaryMatch = findNearestBoundary(state.matches, routeId); + // Build the new Map first, then call deleteFetcher so we don't + // accidentally include a stale entry. + let fetchers = new Map(state.fetchers); + fetchers.delete(key); deleteFetcher(key); updateState( { errors: { [boundaryMatch.route.id]: error, }, - fetchers: new Map(state.fetchers), + fetchers, }, { flushSync: (opts && opts.flushSync) === true }, ); @@ -3483,7 +3519,11 @@ export function createRouter(init: RouterInit): Router { fetchRedirectIds.delete(key); fetchersQueuedForDeletion.delete(key); cancelledFetcherLoads.delete(key); - state.fetchers.delete(key); + // Note: we intentionally do NOT delete from state.fetchers here. Callers + // that need a snapshot to pass to React must build their own new Map and + // exclude this key (or call state.fetchers.delete after obtaining their + // snapshot). Direct mutation of state.fetchers risks handing React a Map + // that gets mutated after it has been captured. } function queueFetcherForDeletion(key: string): void { @@ -3505,19 +3545,20 @@ 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; 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); @@ -3525,15 +3566,18 @@ export function createRouter(init: RouterInit): Router { updatedFetchers = true; } } - markFetchersDone(doneKeys); + markFetchersDone(doneKeys, fetchers); return updatedFetchers; } - 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); @@ -3542,7 +3586,7 @@ export function createRouter(init: RouterInit): Router { } } } - markFetchersDone(yeetedKeys); + markFetchersDone(yeetedKeys, fetchers); return yeetedKeys.length > 0; } @@ -6999,6 +7043,7 @@ function processLoaderData( pendingActionResult: PendingActionResult | undefined, revalidatingFetchers: RevalidatingFetcher[], fetcherResults: Record, + workingFetchers: Map, ): { loaderData: RouterState["loaderData"]; errors?: RouterState["errors"]; @@ -7033,14 +7078,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); } }); From 3c980bb05e9df3dd1d56a08ca22f693040a5f29f Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 11 May 2026 12:46:56 -0400 Subject: [PATCH 03/10] Uopdates --- packages/react-router/lib/router/router.ts | 116 ++++++++++----------- 1 file changed, 54 insertions(+), 62 deletions(-) diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index b8326fbd5d..c68adf2b5e 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -844,7 +844,7 @@ interface HandleLoadersResult extends ShortCircuitable { /** * Updated fetchers map (when stale fetch loads were aborted or redirects completed) */ - fetchers?: RouterState["fetchers"]; + workingFetchers?: RouterState["fetchers"]; } /** @@ -1381,10 +1381,7 @@ export function createRouter(init: RouterInit): Router { } subscribers.clear(); pendingNavigationController && pendingNavigationController.abort(); - state.fetchers.forEach((_, key) => { - deleteFetcher(key); - state.fetchers.delete(key); - }); + state.fetchers.forEach((_, key) => deleteFetcher(state.fetchers, key)); state.blockers.forEach((_, key) => deleteBlocker(key)); } @@ -1470,10 +1467,7 @@ export function createRouter(init: RouterInit): Router { // 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(key); - state.fetchers.delete(key); - }); + unmountedFetchers.forEach((key) => deleteFetcher(state.fetchers, key)); mountedFetchers.forEach((key) => state.fetchers.delete(key)); } @@ -2034,7 +2028,7 @@ export function createRouter(init: RouterInit): Router { matches: updatedMatches, loaderData, errors, - fetchers: updatedFetcherMap, + workingFetchers, } = await handleLoaders( request, location, @@ -2066,7 +2060,7 @@ export function createRouter(init: RouterInit): Router { ...getActionDataForCommit(pendingActionResult), loaderData, errors, - ...(updatedFetcherMap ? { fetchers: updatedFetcherMap } : {}), + ...(workingFetchers ? { fetchers: workingFetchers } : {}), }); } @@ -2396,8 +2390,8 @@ export function createRouter(init: RouterInit): Router { ) && revalidatingFetchers.length === 0 ) { - let fetcherMap = new Map(state.fetchers); - let updatedFetchers = markFetchRedirectsDone(fetcherMap); + let workingFetchers = new Map(state.fetchers); + let fetchersUpdatedViaRedirects = markFetchRedirectsDone(workingFetchers); completeNavigation( location, { @@ -2409,7 +2403,7 @@ export function createRouter(init: RouterInit): Router { ? { [pendingActionResult[0]]: pendingActionResult[1].error } : null, ...getActionDataForCommit(pendingActionResult), - ...(updatedFetchers ? { fetchers: fetcherMap } : {}), + ...(fetchersUpdatedViaRedirects ? { fetchers: workingFetchers } : {}), }, { flushSync }, ); @@ -2499,7 +2493,7 @@ export function createRouter(init: RouterInit): Router { } // Process and commit output from loaders - let fetcherMap = new Map(state.fetchers); + let workingFetchers = new Map(state.fetchers); let { loaderData, errors } = processLoaderData( state, matches, @@ -2507,7 +2501,7 @@ export function createRouter(init: RouterInit): Router { pendingActionResult, revalidatingFetchers, fetcherResults, - fetcherMap, + workingFetchers, ); // Preserve SSR errors during partial hydration @@ -2515,19 +2509,21 @@ export function createRouter(init: RouterInit): Router { errors = { ...state.errors, ...errors }; } - let updatedFetchers = markFetchRedirectsDone(fetcherMap); + let fetchersUpdatedViaRedirects = markFetchRedirectsDone(workingFetchers); let didAbortFetchLoads = abortStaleFetchLoads( pendingNavigationLoadId, - fetcherMap, + workingFetchers, ); let shouldUpdateFetchers = - updatedFetchers || didAbortFetchLoads || revalidatingFetchers.length > 0; + fetchersUpdatedViaRedirects || + didAbortFetchLoads || + revalidatingFetchers.length > 0; return { matches, loaderData, errors, - ...(shouldUpdateFetchers ? { fetchers: fetcherMap } : {}), + ...(shouldUpdateFetchers ? { workingFetchers } : {}), }; } @@ -2553,16 +2549,16 @@ export function createRouter(init: RouterInit): Router { function getUpdatedRevalidatingFetchers( revalidatingFetchers: RevalidatingFetcher[], ) { - let updatedFetchers = new Map(state.fetchers); + let workingFetchers = new Map(state.fetchers); revalidatingFetchers.forEach((rf) => { - let fetcher = updatedFetchers.get(rf.key); + let fetcher = workingFetchers.get(rf.key); let revalidatingFetcher = getLoadingFetcher( undefined, fetcher ? fetcher.data : undefined, ); - updatedFetchers.set(rf.key, revalidatingFetcher); + workingFetchers.set(rf.key, revalidatingFetcher); }); - return updatedFetchers; + return workingFetchers; } // Trigger a fetcher load/submit for the given fetcher key @@ -2860,8 +2856,8 @@ export function createRouter(init: RouterInit): Router { // 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 updatedFetchers = new Map(state.fetchers); - updatedFetchers.set(key, loadFetcher); + 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 @@ -2870,19 +2866,19 @@ export function createRouter(init: RouterInit): Router { .filter((rf) => rf.key !== key) .forEach((rf) => { let staleKey = rf.key; - let existingFetcher = updatedFetchers.get(staleKey); + let existingFetcher = workingFetchers.get(staleKey); let revalidatingFetcher = getLoadingFetcher( undefined, existingFetcher ? existingFetcher.data : undefined, ); - updatedFetchers.set(staleKey, revalidatingFetcher); + workingFetchers.set(staleKey, revalidatingFetcher); abortFetcher(staleKey); if (rf.controller) { fetchControllers.set(staleKey, rf.controller); } }); - updateState({ fetchers: updatedFetchers }); + updateState({ fetchers: workingFetchers }); let abortPendingFetchRevalidations = () => revalidatingFetchers.forEach((rf) => abortFetcher(rf.key)); @@ -2914,25 +2910,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. - let fetcherHasKey = state.fetchers.has(key); - - // Helper to produce a new state.fetchers Map with the done fetcher included - // without mutating the existing Map (which React may already hold a - // reference to from the updateState() call above). - let withDoneFetcher = () => { - if (!fetcherHasKey) return state.fetchers; - let fetchers = new Map(state.fetchers); - fetchers.set(key, getDoneFetcher(actionResult.data)); - return fetchers; + 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 = { ...state, fetchers: withDoneFetcher() }; + state = getRedirectStateWithDoneFetcher(state); return startRedirectNavigation( revalidationRequest, redirect.result, @@ -2947,7 +2943,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 = { ...state, fetchers: withDoneFetcher() }; + state = getRedirectStateWithDoneFetcher(state); return startRedirectNavigation( revalidationRequest, redirect.result, @@ -2962,7 +2958,7 @@ export function createRouter(init: RouterInit): Router { // we never mutate the Map that was handed to React via the earlier // updateState() call (see #14506). let finalFetchers = new Map(state.fetchers); - if (fetcherHasKey) { + if (fetcherIsMounted) { finalFetchers.set(key, getDoneFetcher(actionResult.data)); } @@ -3457,10 +3453,10 @@ export function createRouter(init: RouterInit): Router { fetcher: Fetcher, opts: { flushSync?: boolean } = {}, ) { - let fetchers = new Map(state.fetchers); - fetchers.set(key, fetcher); + let workingFetchers = new Map(state.fetchers); + workingFetchers.set(key, fetcher); updateState( - { fetchers }, + { fetchers: workingFetchers }, { flushSync: (opts && opts.flushSync) === true }, ); } @@ -3472,17 +3468,17 @@ export function createRouter(init: RouterInit): Router { opts: { flushSync?: boolean } = {}, ) { let boundaryMatch = findNearestBoundary(state.matches, routeId); - // Build the new Map first, then call deleteFetcher so we don't - // accidentally include a stale entry. - let fetchers = new Map(state.fetchers); - fetchers.delete(key); - 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, + fetchers: workingFetchers, }, { flushSync: (opts && opts.flushSync) === true }, ); @@ -3503,7 +3499,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 @@ -3519,11 +3515,7 @@ export function createRouter(init: RouterInit): Router { fetchRedirectIds.delete(key); fetchersQueuedForDeletion.delete(key); cancelledFetcherLoads.delete(key); - // Note: we intentionally do NOT delete from state.fetchers here. Callers - // that need a snapshot to pass to React must build their own new Map and - // exclude this key (or call state.fetchers.delete after obtaining their - // snapshot). Direct mutation of state.fetchers risks handing React a Map - // that gets mutated after it has been captured. + fetchers.delete(key); } function queueFetcherForDeletion(key: string): void { @@ -3556,18 +3548,18 @@ export function createRouter(init: RouterInit): Router { function markFetchRedirectsDone(fetchers: Map): boolean { let doneKeys = []; - let updatedFetchers = false; + let didUpdateFetchers = false; for (let key of fetchRedirectIds) { 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, fetchers); - return updatedFetchers; + return didUpdateFetchers; } function abortStaleFetchLoads( From 073f56f6cd5159a6479d4d8afc52c044472e38e1 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 11 May 2026 12:51:55 -0400 Subject: [PATCH 04/10] Uopdates --- packages/react-router/lib/router/router.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index c68adf2b5e..89cf9380c4 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -2391,7 +2391,7 @@ export function createRouter(init: RouterInit): Router { revalidatingFetchers.length === 0 ) { let workingFetchers = new Map(state.fetchers); - let fetchersUpdatedViaRedirects = markFetchRedirectsDone(workingFetchers); + let didUpdateFetcherRedirects = markFetchRedirectsDone(workingFetchers); completeNavigation( location, { @@ -2403,7 +2403,7 @@ export function createRouter(init: RouterInit): Router { ? { [pendingActionResult[0]]: pendingActionResult[1].error } : null, ...getActionDataForCommit(pendingActionResult), - ...(fetchersUpdatedViaRedirects ? { fetchers: workingFetchers } : {}), + ...(didUpdateFetcherRedirects ? { fetchers: workingFetchers } : {}), }, { flushSync }, ); @@ -2509,13 +2509,13 @@ export function createRouter(init: RouterInit): Router { errors = { ...state.errors, ...errors }; } - let fetchersUpdatedViaRedirects = markFetchRedirectsDone(workingFetchers); + let didUpdateFetcherRedirects = markFetchRedirectsDone(workingFetchers); let didAbortFetchLoads = abortStaleFetchLoads( pendingNavigationLoadId, workingFetchers, ); let shouldUpdateFetchers = - fetchersUpdatedViaRedirects || + didUpdateFetcherRedirects || didAbortFetchLoads || revalidatingFetchers.length > 0; @@ -2825,8 +2825,6 @@ export function createRouter(init: RouterInit): Router { let loadId = ++incrementingLoadId; fetchReloadIds.set(key, loadId); - let loadFetcher = getLoadingFetcher(submission, actionResult.data); - let { dsMatches, revalidatingFetchers } = getMatchesToLoad( revalidationRequest, scopedContext, @@ -2856,6 +2854,7 @@ export function createRouter(init: RouterInit): Router { // 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); From 7a4521f40e39a6e6be580a5add0f0c46359ce4aa Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 11 May 2026 15:39:18 -0400 Subject: [PATCH 05/10] perf: memoize useFetchers result on state.fetchers reference Since state.fetchers is now always replaced with a new Map (never mutated in place), we can key useMemo on that reference. The expensive Array.from + spread only runs when fetchers actually change, not on every unrelated navigation or state update. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../dom/data-browser-router-test.tsx | 105 ++++++++++++++++++ packages/react-router/lib/dom/lib.tsx | 12 +- 2 files changed, 113 insertions(+), 4 deletions(-) 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..9f85ea6b2c 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; 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"; From 74a8e3aa867247f98e755efa46194013ebc87823 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 11 May 2026 17:02:24 -0400 Subject: [PATCH 06/10] test: fix #14506 regression test to reliably catch the mutation bug The original test checked subscriber states and could never thefail subscriber sees state after each updateState call, not the intermediate mutation. The real bug is that state.fetchers.set(key, doneFetcher) mutates the Map reference already handed to React's concurrent renderer. The new test captures the Map reference from the subscriber when the fetcher enters the 'loading' state, then asserts that reference was NOT mutated to 'idle' after the fetch completes. This fails on the dev branch (before the fix) with 'Expected: loading, Received: idle' and passes with the immutability refactor in place. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../__tests__/router/fetchers-test.ts | 67 +++++++++---------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/packages/react-router/__tests__/router/fetchers-test.ts b/packages/react-router/__tests__/router/fetchers-test.ts index a85642de41..b0eac1a7dc 100644 --- a/packages/react-router/__tests__/router/fetchers-test.ts +++ b/packages/react-router/__tests__/router/fetchers-test.ts @@ -3706,18 +3706,24 @@ describe("fetchers", () => { describe("fetcher optimistic UI flicker (#14506)", () => { // The root cause of the bug: after updateState({ fetchers: new Map(...) }) - // hands a Map to React, subsequent direct mutations of state.fetchers (e.g. - // state.fetchers.set(key, getDoneFetcher())) mutate that same Map before - // React renders it. If React renders the earlier state snapshot after the - // mutation, it sees idle formData alongside stale loaderData — a flicker. - it("does not expose idle fetcher before new loaderData is committed", async () => { - // Capture every state the subscriber receives so we can verify that the - // subscriber never sees formData disappear while loaderData is still stale. - let subscriberStates: Array<{ - fetcherState: string; - hasFormData: boolean; - loaderStatus: boolean | undefined; - }> = []; + // 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", async () => { + // The Map we receive from the subscriber when the fetcher is "loading" + // (post-action, pre-loader-completion). On a buggy build this Map gets + // mutated to idle before the final updateState; with the fix it is + // immutable after handoff. + let capturedLoadingMap: Map | null = null; let itemStatus = false; @@ -3747,15 +3753,11 @@ describe("fetchers", () => { router.subscribe((state) => { let fetcher = state.fetchers.get("toggle"); - if (!fetcher) return; - let loaderData = state.loaderData["item"] as - | { status: boolean } - | undefined; - subscriberStates.push({ - fetcherState: fetcher.state, - hasFormData: fetcher.formData !== undefined, - loaderStatus: loaderData?.status, - }); + // Capture the Map reference the first time the fetcher enters the + // "loading" state (action done, loaders revalidating). + if (fetcher?.state === "loading" && capturedLoadingMap === null) { + capturedLoadingMap = state.fetchers; + } }); let formData = new FormData(); @@ -3768,19 +3770,16 @@ describe("fetchers", () => { router.dispose(); - // The subscriber must never see a state where formData is gone (idle/no - // submission info) while loaderData still has the old value (false). - // That combination is the "flicker" state described in #14506. - let flickerStates = subscriberStates.filter( - (s) => !s.hasFormData && s.loaderStatus === false, - ); - - expect(flickerStates).toEqual([]); - - // Sanity checks: we should have seen submitting/loading states with - // formData, and the final state should have the updated loaderData. - let lastState = subscriberStates[subscriberStates.length - 1]; - expect(lastState.loaderStatus).toBe(true); + // After the fetch fully completes, the Map we captured during the + // "loading" phase must still reflect "loading" — it must not have been + // mutated to "idle" in place. On the dev branch (before the fix) this + // assertion fails because state.fetchers.set(key, doneFetcher) mutated + // the Map that was already handed to React. + expect(capturedLoadingMap).not.toBeNull(); + expect((capturedLoadingMap! as Map).get("toggle")?.state).toBe("loading"); + expect( + (capturedLoadingMap! as Map).get("toggle")?.formData, + ).toBeDefined(); }); }); }); From 331dcc7136a912568e92c82b0b6b4dd90a467e99 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 11 May 2026 17:14:05 -0400 Subject: [PATCH 07/10] test: add Map immutability tests for all fetcher mutation paths Adds four additional regression tests alongside the existing action+loader test, covering every code path that previously mutated state.fetchers in place after handing the Map reference to React: - Fetcher GET load: updateFetcherState() called twice (loading then done) mutated the loading Map via the second state.fetchers.set() call. - Fetcher revalidation during navigation: processLoaderData() mutated the loading Map (handed to subscribers during getUpdatedRevalidatingFetchers) when marking revalidating fetchers done after loaders completed. - Fetcher loader redirect: markFetchersDone() inside completeNavigation mutated the loading Map handed to subscribers before the redirect finished. - Fetcher action redirect: markFetchersDone() inside completeNavigation mutated the loading Map (post-action-redirect) handed to subscribers before the redirect navigation completed. Each test captures the Map reference from the subscriber at the relevant intermediate state, then asserts after completion that the Map still reflects that intermediate state (not the final idle state). All five tests fail on the dev branch and pass with the immutability refactor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../__tests__/router/fetchers-test.ts | 191 +++++++++++++++++- 1 file changed, 186 insertions(+), 5 deletions(-) diff --git a/packages/react-router/__tests__/router/fetchers-test.ts b/packages/react-router/__tests__/router/fetchers-test.ts index b0eac1a7dc..f0e573c9f1 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, @@ -3776,10 +3776,191 @@ describe("fetchers", () => { // assertion fails because state.fetchers.set(key, doneFetcher) mutated // the Map that was already handed to React. expect(capturedLoadingMap).not.toBeNull(); - expect((capturedLoadingMap! as Map).get("toggle")?.state).toBe("loading"); - expect( - (capturedLoadingMap! as Map).get("toggle")?.formData, - ).toBeDefined(); + expect(capturedLoadingMap!.get("toggle")?.state).toBe("loading"); + expect(capturedLoadingMap!.get("toggle")?.formData).toBeDefined(); + }); + + it("does not mutate the Map reference handed to subscribers (fetcher GET 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 capturedLoadingMap: Map | null = null; + + 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) => { + let fetcher = state.fetchers.get("load"); + if (fetcher?.state === "loading" && capturedLoadingMap === null) { + capturedLoadingMap = state.fetchers; + } + }); + + await router.fetch("load", "root", "/item"); + + router.dispose(); + + // The "loading" Map must not have been mutated to "idle" in place. + expect(capturedLoadingMap).not.toBeNull(); + expect(capturedLoadingMap!.get("load")?.state).toBe("loading"); + expect(capturedLoadingMap!.get("load")?.data).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 capturedRevalidatingMap: Map | null = null; + + 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) => { + let fetcher = state.fetchers.get("f"); + // Capture the first time the fetcher enters "loading" during + // revalidation (triggered by the navigation POST action below). + if (fetcher?.state === "loading" && capturedRevalidatingMap === null) { + capturedRevalidatingMap = 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(); + + // The Map handed to subscribers during the revalidation "loading" phase + // must not have been mutated to "idle" in place by processLoaderData. + expect(capturedRevalidatingMap).not.toBeNull(); + expect(capturedRevalidatingMap!.get("f")?.state).toBe("loading"); + }); + + 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 capturedLoadingMap: Map | null = null; + + 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) => { + let fetcher = state.fetchers.get("redir"); + if (fetcher?.state === "loading" && capturedLoadingMap === null) { + capturedLoadingMap = state.fetchers; + } + }); + + await router.fetch("redir", "root", "/redirect"); + + router.dispose(); + + // The "loading" Map must still show the fetcher loading — not mutated + // to "idle" by markFetchersDone() during the redirect navigation. + expect(capturedLoadingMap).not.toBeNull(); + expect(capturedLoadingMap!.get("redir")?.state).toBe("loading"); + }); + + 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 capturedLoadingMap: Map | null = null; + + 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) => { + let fetcher = state.fetchers.get("act"); + // Capture the Map when the fetcher enters "loading" state (action + // redirect accepted — fetcher transitions from submitting → loading + // while the redirect navigation runs). + if (fetcher?.state === "loading" && capturedLoadingMap === null) { + capturedLoadingMap = state.fetchers; + } + }); + + let formData = new FormData(); + await router.fetch("act", "root", "/action", { + formMethod: "POST", + formData, + }); + + router.dispose(); + + // MapB (fetcher loading during redirect) must not have been mutated to + // "idle" by markFetchersDone() before completeNavigation creates MapC. + expect(capturedLoadingMap).not.toBeNull(); + expect(capturedLoadingMap!.get("act")?.state).toBe("loading"); }); }); }); From e3dac3d276ad0d03e8f40b380d20a6e664c0f53a Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 12 May 2026 10:34:51 -0400 Subject: [PATCH 08/10] test: broaden useId regex to handle multi-digit React IDs React's useId() can produce IDs like _r_11_ (digits only, no trailing letter), but the regex was /^_r_[0-9]?[a-z]_/ which required exactly one alpha character. Broaden to /^_r_[0-9a-z]+_/ to match any alphanumeric suffix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../react-router/__tests__/dom/data-browser-router-test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 9f85ea6b2c..06bd9adb5d 100644 --- a/packages/react-router/__tests__/dom/data-browser-router-test.tsx +++ b/packages/react-router/__tests__/dom/data-browser-router-test.tsx @@ -6606,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$/, ), ); }); From cb07b4a8a2e481cb2058a15137f84139ae5a33da Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 12 May 2026 17:12:09 -0400 Subject: [PATCH 09/10] Updates --- .../.changes/patch.memoize-usefetchers.md | 1 + .../__tests__/router/fetchers-test.ts | 123 +++++++++--------- 2 files changed, 60 insertions(+), 64 deletions(-) create mode 100644 packages/react-router/.changes/patch.memoize-usefetchers.md 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__/router/fetchers-test.ts b/packages/react-router/__tests__/router/fetchers-test.ts index f0e573c9f1..fea1fc24b4 100644 --- a/packages/react-router/__tests__/router/fetchers-test.ts +++ b/packages/react-router/__tests__/router/fetchers-test.ts @@ -3704,7 +3704,7 @@ describe("fetchers", () => { }); }); - describe("fetcher optimistic UI flicker (#14506)", () => { + 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 @@ -3718,13 +3718,8 @@ describe("fetchers", () => { // 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", async () => { - // The Map we receive from the subscriber when the fetcher is "loading" - // (post-action, pre-loader-completion). On a buggy build this Map gets - // mutated to idle before the final updateState; with the fix it is - // immutable after handoff. - let capturedLoadingMap: Map | null = null; - + it("does not mutate the Map reference handed to subscribers (fetcher.submit)", async () => { + let fetcherMaps: Map[] = []; let itemStatus = false; let router = createRouter({ @@ -3752,12 +3747,7 @@ describe("fetchers", () => { router.getFetcher("toggle"); router.subscribe((state) => { - let fetcher = state.fetchers.get("toggle"); - // Capture the Map reference the first time the fetcher enters the - // "loading" state (action done, loaders revalidating). - if (fetcher?.state === "loading" && capturedLoadingMap === null) { - capturedLoadingMap = state.fetchers; - } + fetcherMaps.push(state.fetchers); }); let formData = new FormData(); @@ -3770,21 +3760,22 @@ describe("fetchers", () => { router.dispose(); - // After the fetch fully completes, the Map we captured during the - // "loading" phase must still reflect "loading" — it must not have been - // mutated to "idle" in place. On the dev branch (before the fix) this - // assertion fails because state.fetchers.set(key, doneFetcher) mutated - // the Map that was already handed to React. - expect(capturedLoadingMap).not.toBeNull(); - expect(capturedLoadingMap!.get("toggle")?.state).toBe("loading"); - expect(capturedLoadingMap!.get("toggle")?.formData).toBeDefined(); + // 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 GET load)", async () => { + 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 capturedLoadingMap: Map | null = null; + let fetcherMaps: Map[] = []; let router = createRouter({ history: createMemoryHistory({ initialEntries: ["/"] }), @@ -3799,20 +3790,20 @@ describe("fetchers", () => { router.getFetcher("load"); router.subscribe((state) => { - let fetcher = state.fetchers.get("load"); - if (fetcher?.state === "loading" && capturedLoadingMap === null) { - capturedLoadingMap = state.fetchers; - } + fetcherMaps.push(state.fetchers); }); await router.fetch("load", "root", "/item"); router.dispose(); - // The "loading" Map must not have been mutated to "idle" in place. - expect(capturedLoadingMap).not.toBeNull(); - expect(capturedLoadingMap!.get("load")?.state).toBe("loading"); - expect(capturedLoadingMap!.get("load")?.data).toBeUndefined(); + // 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 () => { @@ -3821,7 +3812,7 @@ describe("fetchers", () => { // 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 capturedRevalidatingMap: Map | null = null; + let fetcherMaps: Map[] = []; let router = createRouter({ history: createMemoryHistory({ initialEntries: ["/"] }), @@ -3840,12 +3831,7 @@ describe("fetchers", () => { router.getFetcher("f"); router.subscribe((state) => { - let fetcher = state.fetchers.get("f"); - // Capture the first time the fetcher enters "loading" during - // revalidation (triggered by the navigation POST action below). - if (fetcher?.state === "loading" && capturedRevalidatingMap === null) { - capturedRevalidatingMap = state.fetchers; - } + fetcherMaps.push(state.fetchers); }); // GET load to register the fetcher in fetchLoadMatches (eligible for @@ -3859,10 +3845,16 @@ describe("fetchers", () => { router.dispose(); - // The Map handed to subscribers during the revalidation "loading" phase - // must not have been mutated to "idle" in place by processLoaderData. - expect(capturedRevalidatingMap).not.toBeNull(); - expect(capturedRevalidatingMap!.get("f")?.state).toBe("loading"); + // 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 () => { @@ -3870,7 +3862,7 @@ describe("fetchers", () => { // When the loader returns a redirect, markFetchRedirectsDone() calls // state.fetchers.set(key, doneFetcher) which mutates MapA before the // final completeNavigation updateState creates MapB. - let capturedLoadingMap: Map | null = null; + let fetcherMaps: Map[] = []; let router = createRouter({ history: createMemoryHistory({ initialEntries: ["/"] }), @@ -3893,20 +3885,20 @@ describe("fetchers", () => { router.getFetcher("redir"); router.subscribe((state) => { - let fetcher = state.fetchers.get("redir"); - if (fetcher?.state === "loading" && capturedLoadingMap === null) { - capturedLoadingMap = state.fetchers; - } + fetcherMaps.push(state.fetchers); }); await router.fetch("redir", "root", "/redirect"); router.dispose(); - // The "loading" Map must still show the fetcher loading — not mutated - // to "idle" by markFetchersDone() during the redirect navigation. - expect(capturedLoadingMap).not.toBeNull(); - expect(capturedLoadingMap!.get("redir")?.state).toBe("loading"); + // 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 () => { @@ -3917,7 +3909,7 @@ describe("fetchers", () => { // completeNavigation, markFetchRedirectsDone() mutates MapB // (state.fetchers.set(key, doneFetcher)) before the final updateState // creates MapC. Test that MapB is not mutated. - let capturedLoadingMap: Map | null = null; + let fetcherMaps: Map[] = []; let router = createRouter({ history: createMemoryHistory({ initialEntries: ["/"] }), @@ -3940,13 +3932,7 @@ describe("fetchers", () => { router.getFetcher("act"); router.subscribe((state) => { - let fetcher = state.fetchers.get("act"); - // Capture the Map when the fetcher enters "loading" state (action - // redirect accepted — fetcher transitions from submitting → loading - // while the redirect navigation runs). - if (fetcher?.state === "loading" && capturedLoadingMap === null) { - capturedLoadingMap = state.fetchers; - } + fetcherMaps.push(state.fetchers); }); let formData = new FormData(); @@ -3957,10 +3943,19 @@ describe("fetchers", () => { router.dispose(); - // MapB (fetcher loading during redirect) must not have been mutated to - // "idle" by markFetchersDone() before completeNavigation creates MapC. - expect(capturedLoadingMap).not.toBeNull(); - expect(capturedLoadingMap!.get("act")?.state).toBe("loading"); + // 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(); }); }); }); From 61bce6476a2200a83a8fa2250a9f6346b6f15693 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 12 May 2026 17:15:20 -0400 Subject: [PATCH 10/10] Apply suggestion from @brophdawg11 --- .../react-router/.changes/patch.fix-fetcher-formdata-flicker.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-router/.changes/patch.fix-fetcher-formdata-flicker.md b/packages/react-router/.changes/patch.fix-fetcher-formdata-flicker.md index 56b22bf352..47115b18d8 100644 --- a/packages/react-router/.changes/patch.fix-fetcher-formdata-flicker.md +++ b/packages/react-router/.changes/patch.fix-fetcher-formdata-flicker.md @@ -1 +1 @@ -Fix a race condition in fetcher state machine where `formData` briefly became `undefined` before new `loaderData` was available, causing a UI flicker in optimistic update patterns (#14506) +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.