fix: prevent fetcher formData flicker and eliminate state.fetchers mutations

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)

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<React.SetStateAction<number>> | null };
+
+        function Parent() {
+          let fetchers = useFetchers();
+          let fetcher = useFetcher();
+          let [, setCount] = React.useState(0);
+          setCountRef.current = setCount;
+          fetchersArrays.push(fetchers);
+          return (
+            <button onClick={() => fetcher.load("/fetch")}>load</button>
+          );
+        }
+
+        let router = createTestRouter(
+          [
+            {
+              path: "/",
+              Component: Parent,
+              children: [{ path: "/fetch", loader: () => fetchDfd.promise }],
+            },
+          ],
+          {
+            window: getWindow("/"),
+            hydrationData: { loaderData: { "0": null } },
+          },
+        );
+        render(<RouterProvider router={router} />);
+
+        // 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 (
+            <button onClick={() => fetcher.load("/fetch")}>load</button>
+          );
+        }
+
+        let router = createTestRouter(
+          [
+            {
+              path: "/",
+              Component: Parent,
+              children: [{ path: "/fetch", loader: () => fetchDfd.promise }],
+            },
+          ],
+          {
+            window: getWindow("/"),
+            hydrationData: { loaderData: { "0": null } },
+          },
+        );
+        render(<RouterProvider router={router} />);
+
+        // 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$/,
             ),
           );
         });

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,264 @@ 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 (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<string, Fetcher> | null = null;
+
+      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");
+        // 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();
+      formData.append("status", "true");
+
+      await router.fetch("toggle", "item", "/item", {
+        formMethod: "POST",
+        formData,
+      });
+
+      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();
+    });
+
+    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<string, Fetcher> | 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<string, Fetcher> | 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<string, Fetcher> | 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<string, Fetcher> | 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");
+    });
+  });
 });