From 19af0cf7652f309a6a68c3c698f8eb36e733ce72 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 26 Oct 2023 15:25:04 -0400 Subject: [PATCH] Add future.v7_fetcherPersist flag (#10962) --- .changeset/fetcher-cleanup.md | 11 + .changeset/fix-router-future-prop.md | 6 + docs/guides/api-development-strategy.md | 1 + examples/data-router/src/app.tsx | 395 ++----------- .../__tests__/data-browser-router-test.tsx | 520 +++++++++++++++++- packages/react-router-dom/index.tsx | 20 +- packages/react-router/lib/components.tsx | 2 +- packages/router/router.ts | 42 +- 8 files changed, 608 insertions(+), 389 deletions(-) create mode 100644 .changeset/fetcher-cleanup.md create mode 100644 .changeset/fix-router-future-prop.md diff --git a/.changeset/fetcher-cleanup.md b/.changeset/fetcher-cleanup.md new file mode 100644 index 0000000000..029329149e --- /dev/null +++ b/.changeset/fetcher-cleanup.md @@ -0,0 +1,11 @@ +--- +"@remix-run/router": minor +--- + +Add a new `future.v7_fetcherPersist` flag to the `@remix-run/router` to change the persistence behavior of fetchers when `router.deleteFetcher` is called. Instead of being immediately cleaned up, fetchers will persist until they return to an `idle` state([RFC](https://github.com/remix-run/remix/discussions/7698)) + +- This is sort of a long-standing bug fix as the `useFetchers()` API was always supposed to only reflect **in-flight** fetcher information for pending/optimistic UI -- it was not intended to reflect fetcher data or hang onto fetchers after they returned to an `idle` state +- With `v7_fetcherPersist`, the `router` only knows about in-flight fetchers - they do not exist in `state.fetchers` until a `fetch()` call is made, and they are removed as soon as it returns to `idle` (and the data is handed off to the React layer) +- Keep an eye out for the following specific behavioral changes when opting into this flag and check your app for compatibility: + - Fetchers that complete _while still mounted_ will no longer appear in `useFetchers()`. They served effectively no purpose in there since you can access the data via `useFetcher().data`). + - Fetchers that previously unmounted _while in-flight_ will not be immediately aborted and will instead be cleaned up once they return to an `idle` state. They will remain exposed via `useFetchers` while in-flight so you can still access pending/optimistic data after unmount. diff --git a/.changeset/fix-router-future-prop.md b/.changeset/fix-router-future-prop.md new file mode 100644 index 0000000000..cb564b5a69 --- /dev/null +++ b/.changeset/fix-router-future-prop.md @@ -0,0 +1,6 @@ +--- +"react-router": patch +"react-router-dom": patch +--- + +Fix the `future`prop on `BrowserRouter`, `HashRouter` and `MemoryRouter` so that it accepts a `Partial` instead of requiring all flags to be included. diff --git a/docs/guides/api-development-strategy.md b/docs/guides/api-development-strategy.md index a750c8be67..2930bba6fe 100644 --- a/docs/guides/api-development-strategy.md +++ b/docs/guides/api-development-strategy.md @@ -66,6 +66,7 @@ const router = createBrowserRouter(routes, { | Flag | Description | | ------------------------ | --------------------------------------------------------------------- | | `v7_normalizeFormMethod` | Normalize `useNavigation().formMethod` to be an uppercase HTTP Method | +| `v7_fetcherPersist` | Delay active fetcher cleanup until they return to an `idle` state | ### React Router Future Flags diff --git a/examples/data-router/src/app.tsx b/examples/data-router/src/app.tsx index 4cf6396a7e..c0e115f95d 100644 --- a/examples/data-router/src/app.tsx +++ b/examples/data-router/src/app.tsx @@ -24,38 +24,48 @@ import { addTodo, deleteTodo, getTodos } from "./todos"; import "./index.css"; -let router = createBrowserRouter([ - { - path: "/", - Component: Layout, - children: [ - { - index: true, - loader: homeLoader, - Component: Home, +let router = createBrowserRouter( + [ + { + path: "/", + Component() { + let fetchers = useFetchers(); + return ( + <> + 
{JSON.stringify(fetchers)}
+ + + ); }, - { - path: "todos", - action: todosAction, - loader: todosLoader, - Component: TodosList, - ErrorBoundary: TodosBoundary, - children: [ - { - path: ":id", - loader: todoLoader, - Component: Todo, + children: [ + { + index: true, + Component() { + let fetcher = useFetcher({ key: "a" }); + let fetcher2 = useFetcher({ key: "a" }); + return ( +
+ + + 
{JSON.stringify(fetcher)}
+ 
{JSON.stringify(fetcher2)}
+
+ ); }, - ], - }, - { - path: "deferred", - loader: deferredLoader, - Component: DeferredPage, - }, - ], - }, -]); + }, + ], + }, + { + path: "/fetch", + loader: () => new Promise((r) => setTimeout(() => r("FETCH"), 1000)), + }, + ], + { + future: { + v7_fetcherPersist: true, + }, + } +); if (import.meta.hot) { import.meta.hot.dispose(() => router.dispose()); @@ -72,328 +82,3 @@ export function sleep(n: number = 500) { export function Fallback() { return

Performing initial data load

; } - -// Layout -export function Layout() { - let navigation = useNavigation(); - let revalidator = useRevalidator(); - let fetchers = useFetchers(); - let fetcherInProgress = fetchers.some((f) => - ["loading", "submitting"].includes(f.state) - ); - - return ( - <> -

Data Router Example

- -

- This example demonstrates some of the core features of React Router - including nested <Route>s, <Outlet>s, <Link>s, and - using a "*" route (aka "splat route") to render a "not found" page when - someone visits an unrecognized URL. -

- - -
- {navigation.state !== "idle" &&

Navigation in progress...

} - {revalidator.state !== "idle" &&

Revalidation in progress...

} - {fetcherInProgress &&

Fetcher in progress...

} -
-

- Click on over to /todos and check out these - data loading APIs! -

-

- Or, checkout /deferred to see how to - separate critical and lazily loaded data in your loaders. -

-

- We've introduced some fake async-aspects of routing here, so Keep an eye - on the top-right hand corner to see when we're actively navigating. -

-
- - - ); -} - -// Home -interface HomeLoaderData { - date: string; -} - -export async function homeLoader(): Promise { - await sleep(); - return { - date: new Date().toISOString(), - }; -} - -export function Home() { - let data = useLoaderData() as HomeLoaderData; - return ( - <> -

Home

-

Date from loader: {data.date}

- - ); -} - -// Todos -export async function todosAction({ request }: ActionFunctionArgs) { - await sleep(); - - let formData = await request.formData(); - - // Deletion via fetcher - if (formData.get("action") === "delete") { - let id = formData.get("todoId"); - if (typeof id === "string") { - deleteTodo(id); - return { ok: true }; - } - } - - // Addition via
- let todo = formData.get("todo"); - if (typeof todo === "string") { - addTodo(todo); - } - - return new Response(null, { - status: 302, - headers: { Location: "/todos" }, - }); -} - -export async function todosLoader(): Promise { - await sleep(); - return getTodos(); -} - -export function TodosList() { - let todos = useLoaderData() as Todos; - let navigation = useNavigation(); - let formRef = React.useRef(null); - - // If we add and then we delete - this will keep isAdding=true until the - // fetcher completes it's revalidation - let [isAdding, setIsAdding] = React.useState(false); - React.useEffect(() => { - if (navigation.formData?.get("action") === "add") { - setIsAdding(true); - } else if (navigation.state === "idle") { - setIsAdding(false); - formRef.current?.reset(); - } - }, [navigation]); - - return ( - <> -

Todos

-

- This todo app uses a <Form> to submit new todos and a - <fetcher.form> to delete todos. Click on a todo item to navigate - to the /todos/:id route. -

-
    -
  • - - Click this link to force an error in the loader - -
    • - {Object.entries(todos).map(([id, todo]) => ( -
  • - -
    • - ))} -
- - - - - - - - ); -} - -export function TodosBoundary() { - let error = useRouteError() as Error; - return ( - <> -

Error 💥

-

{error.message}

- - ); -} - -interface TodoItemProps { - id: string; - todo: string; -} - -export function TodoItem({ id, todo }: TodoItemProps) { - let fetcher = useFetcher(); - - let isDeleting = fetcher.formData != null; - return ( - <> - {todo} -   - - - - - - ); -} - -// Todo -export async function todoLoader({ - params, -}: LoaderFunctionArgs): Promise { - await sleep(); - let todos = getTodos(); - if (!params.id) { - throw new Error("Expected params.id"); - } - let todo = todos[params.id]; - if (!todo) { - throw new Error(`Uh oh, I couldn't find a todo with id "${params.id}"`); - } - return todo; -} - -export function Todo() { - let params = useParams(); - let todo = useLoaderData() as string; - return ( - <> -

Nested Todo Route:

-

id: {params.id}

-

todo: {todo}

- - ); -} - -// Deferred Data -interface DeferredRouteLoaderData { - critical1: string; - critical2: string; - lazyResolved: Promise; - lazy1: Promise; - lazy2: Promise; - lazy3: Promise; - lazyError: Promise; -} - -const rand = () => Math.round(Math.random() * 100); -const resolve = (d: string, ms: number) => - new Promise((r) => setTimeout(() => r(`${d} - ${rand()}`), ms)); -const reject = (d: Error | string, ms: number) => - new Promise((_, r) => - setTimeout(() => { - if (d instanceof Error) { - d.message += ` - ${rand()}`; - } else { - d += ` - ${rand()}`; - } - r(d); - }, ms) - ); - -export async function deferredLoader() { - return defer({ - critical1: await resolve("Critical 1", 250), - critical2: await resolve("Critical 2", 500), - lazyResolved: Promise.resolve("Lazy Data immediately resolved - " + rand()), - lazy1: resolve("Lazy 1", 1000), - lazy2: resolve("Lazy 2", 1500), - lazy3: resolve("Lazy 3", 2000), - lazyError: reject(new Error("Kaboom!"), 2500), - }); -} - -export function DeferredPage() { - let data = useLoaderData() as DeferredRouteLoaderData; - return ( -
- {/* Critical data renders immediately */} -

{data.critical1}

-

{data.critical2}

- - {/* Pre-resolved deferred data never triggers the fallback */} - should not see me!

}> - - - - - - {/* Deferred data can be rendered using a component + the useAsyncValue() hook */} - loading 1...

}> - - - - - - loading 2...

}> - - - - - - {/* Or you can bypass the hook and use a render function */} - loading 3...

}> - {(data: string) =>

{data}

} - - - {/* Deferred rejections render using the useAsyncError hook */} - loading (error)...

}> - }> - - - -
- ); -} - -function RenderAwaitedData() { - let data = useAsyncValue() as string; - return

{data}

; -} - -function RenderAwaitedError() { - let error = useAsyncError() as Error; - return ( -

- Error (errorElement)! -
- {error.message} {error.stack} -

- ); -} diff --git a/packages/react-router-dom/__tests__/data-browser-router-test.tsx b/packages/react-router-dom/__tests__/data-browser-router-test.tsx index ba06de48bc..6c43240fcc 100644 --- a/packages/react-router-dom/__tests__/data-browser-router-test.tsx +++ b/packages/react-router-dom/__tests__/data-browser-router-test.tsx @@ -1,43 +1,44 @@ -import { JSDOM } from "jsdom"; -import * as React from "react"; +import type { ErrorResponse, Fetcher } from "@remix-run/router"; +import "@testing-library/jest-dom"; import { act, - render, fireEvent, - waitFor, + render, screen, + waitFor, } from "@testing-library/react"; -import "@testing-library/jest-dom"; -import type { ErrorResponse, Fetcher } from "@remix-run/router"; +import { JSDOM } from "jsdom"; +import * as React from "react"; import type { RouteObject } from "react-router-dom"; import { + UNSAFE_DataRouterStateContext as DataRouterStateContext, Form, Link, + Outlet, Route, RouterProvider, - Outlet, createBrowserRouter, createHashRouter, + createRoutesFromElements, + defer, isRouteErrorResponse, matchRoutes, - useLoaderData, + redirect, useActionData, - useRouteError, - useNavigate, - useNavigation, - useSubmit, useFetcher, useFetchers, - UNSAFE_DataRouterStateContext as DataRouterStateContext, - defer, + useLoaderData, useLocation, useMatches, + useNavigate, + useNavigation, + useRouteError, useSearchParams, - createRoutesFromElements, + useSubmit, } from "react-router-dom"; -import { createDeferred } from "../../router/__tests__/utils/utils"; import getHtml from "../../react-router/__tests__/utils/getHtml"; +import { createDeferred } from "../../router/__tests__/utils/utils"; testDomRouter("", createBrowserRouter, (url) => getWindowImpl(url, false) @@ -4637,9 +4638,7 @@ function testDomRouter( " `); - // Resolve Comp2 loader and complete navigation - Comp1 fetcher is still - // reflected here since deleteFetcher doesn't updateState - // TODO: Is this expected? + // Resolve Comp2 loader and complete navigation dfd2.resolve("data 2"); await waitFor(() => screen.getByText(/2.*idle/)); expect(getHtml(container.querySelector("#output")!)) @@ -4648,7 +4647,7 @@ function testDomRouter( id="output" >

- ["idle"] + []

2 @@ -5246,6 +5245,487 @@ function testDomRouter( }); }); + describe("fetcher persistence", () => { + describe("default behavior", () => { + it("loading fetchers clean up on unmount by default", async () => { + let dfd = createDeferred(); + let loaderRequest: Request | null = null; + let router = createTestRouter( + [ + { + path: "/", + Component() { + let fetchers = useFetchers(); + return ( + <> + 

{`Num fetchers: ${fetchers.length}`}
+ Go to /page + + + ); + }, + children: [ + { + index: true, + Component() { + let fetcher = useFetcher(); + return ( + + ); + }, + }, + { + path: "page", + Component() { + return

Page

; + }, + }, + ], + }, + { + path: "/fetch", + loader: ({ request }) => { + loaderRequest = request; + return dfd.promise; + }, + }, + ], + { window: getWindow("/") } + ); + let { container } = render(); + + expect(getHtml(container)).toMatch("Num fetchers: 0"); + + fireEvent.click(screen.getByText("Load (idle)")); + expect(getHtml(container)).toMatch("Num fetchers: 1"); + expect(getHtml(container)).toMatch("Load (loading)"); + + fireEvent.click(screen.getByText("Go to /page")); + await waitFor(() => screen.getByText("Page")); + expect(getHtml(container)).toMatch("Num fetchers: 0"); + expect(getHtml(container)).toMatch("Page"); + + // Resolve after the navigation - no-op + expect(loaderRequest?.signal?.aborted).toBe(true); + dfd.resolve("FETCH"); + await waitFor(() => screen.getByText("Num fetchers: 0")); + expect(getHtml(container)).toMatch("Page"); + }); + + it("submitting fetchers persist until completion", async () => { + let dfd = createDeferred(); + let router = createTestRouter( + [ + { + path: "/", + Component() { + let fetchers = useFetchers(); + return ( + <> + 
{`Num fetchers: ${fetchers.length}`}
+ Go to /page + + + ); + }, + children: [ + { + index: true, + Component() { + let fetcher = useFetcher(); + return ( + + ); + }, + }, + { + path: "page", + Component() { + return

Page

; + }, + }, + ], + }, + { + path: "/fetch", + action: () => dfd.promise, + }, + ], + { window: getWindow("/") } + ); + let { container } = render(); + + expect(getHtml(container)).toMatch("Num fetchers: 0"); + + fireEvent.click(screen.getByText("Submit (idle)")); + expect(getHtml(container)).toMatch("Num fetchers: 1"); + expect(getHtml(container)).toMatch("Submit (submitting)"); + + fireEvent.click(screen.getByText("Go to /page")); + await waitFor(() => screen.getByText("Page")); + expect(getHtml(container)).toMatch("Num fetchers: 0"); + + // Resolve after the navigation - trigger cleanup + dfd.resolve("FETCH"); + await waitFor(() => screen.getByText("Num fetchers: 0")); + }); + }); + + describe("v7_fetcherPersist=true", () => { + it("loading fetchers persist until completion", async () => { + let dfd = createDeferred(); + let router = createTestRouter( + [ + { + path: "/", + Component() { + let fetchers = useFetchers(); + return ( + <> + 
{`Num fetchers: ${fetchers.length}`}
+ Go to /page + + + ); + }, + children: [ + { + index: true, + Component() { + let fetcher = useFetcher(); + return ( + + ); + }, + }, + { + path: "page", + Component() { + return

Page

; + }, + }, + ], + }, + { + path: "/fetch", + loader: () => dfd.promise, + }, + ], + { window: getWindow("/"), future: { v7_fetcherPersist: true } } + ); + let { container } = render(); + + expect(getHtml(container)).toMatch("Num fetchers: 0"); + + fireEvent.click(screen.getByText("Load (idle)")); + expect(getHtml(container)).toMatch("Num fetchers: 1"); + expect(getHtml(container)).toMatch("Load (loading)"); + + fireEvent.click(screen.getByText("Go to /page")); + await waitFor(() => screen.getByText("Page")); + expect(getHtml(container)).toMatch("Num fetchers: 1"); + expect(getHtml(container)).toMatch("Page"); + + // Resolve after the navigation - no-op + dfd.resolve("FETCH"); + await waitFor(() => screen.getByText("Num fetchers: 0")); + expect(getHtml(container)).toMatch("Page"); + }); + + it("submitting fetchers persist until completion", async () => { + let dfd = createDeferred(); + let router = createTestRouter( + [ + { + path: "/", + Component() { + let fetchers = useFetchers(); + return ( + <> + 
{`Num fetchers: ${fetchers.length}`}
+ Go to /page + + + ); + }, + children: [ + { + index: true, + Component() { + let fetcher = useFetcher(); + return ( + + ); + }, + }, + { + path: "page", + Component() { + return

Page

; + }, + }, + ], + }, + { + path: "/fetch", + action: () => dfd.promise, + }, + ], + { window: getWindow("/"), future: { v7_fetcherPersist: true } } + ); + let { container } = render(); + + expect(getHtml(container)).toMatch("Num fetchers: 0"); + + fireEvent.click(screen.getByText("Submit (idle)")); + expect(getHtml(container)).toMatch("Num fetchers: 1"); + expect(getHtml(container)).toMatch("Submit (submitting)"); + + fireEvent.click(screen.getByText("Go to /page")); + await waitFor(() => screen.getByText("Page")); + expect(getHtml(container)).toMatch("Num fetchers: 1"); + + // Resolve after the navigation - trigger cleanup + dfd.resolve("FETCH"); + await waitFor(() => screen.getByText("Num fetchers: 0")); + }); + + it("submitting fetchers w/revalidations are cleaned up on completion", async () => { + let count = 0; + let dfd = createDeferred(); + let router = createTestRouter( + [ + { + path: "/", + Component() { + let fetchers = useFetchers(); + return ( + <> + 
{`Num fetchers: ${fetchers.length}`}
+ Go to /page + + + ); + }, + children: [ + { + index: true, + Component() { + let fetcher = useFetcher({ persist: true }); + return ( + + ); + }, + }, + { + path: "page", + Component() { + let data = useLoaderData() as { count: number }; + return

{`Page (${data.count})`}

; + }, + async loader() { + await new Promise((r) => setTimeout(r, 10)); + return { count: ++count }; + }, + }, + ], + }, + { + path: "/fetch", + action: () => dfd.promise, + }, + ], + { window: getWindow("/"), future: { v7_fetcherPersist: true } } + ); + let { container } = render(); + + expect(getHtml(container)).toMatch("Num fetchers: 0"); + + fireEvent.click(screen.getByText("Submit (idle)")); + expect(getHtml(container)).toMatch("Num fetchers: 1"); + expect(getHtml(container)).toMatch("Submit (submitting)"); + + fireEvent.click(screen.getByText("Go to /page")); + await waitFor(() => screen.getByText("Page (1)")); + expect(getHtml(container)).toMatch("Num fetchers: 1"); + + // Resolve after the navigation and revalidation + dfd.resolve("FETCH"); + await waitFor(() => screen.getByText("Num fetchers: 0")); + expect(getHtml(container)).toMatch("Page (2)"); + }); + + it("submitting fetchers w/redirects are cleaned up on completion", async () => { + let dfd = createDeferred(); + let router = createTestRouter( + [ + { + path: "/", + Component() { + let fetchers = useFetchers(); + return ( + <> + 
{`Num fetchers: ${fetchers.length}`}
+ Go to /page + + + ); + }, + children: [ + { + index: true, + Component() { + let fetcher = useFetcher({ persist: true }); + return ( + + ); + }, + }, + { + path: "page", + Component() { + return

Page

; + }, + }, + { + path: "redirect", + Component() { + return

Redirect

; + }, + }, + ], + }, + { + path: "/fetch", + action: () => dfd.promise, + }, + ], + { window: getWindow("/"), future: { v7_fetcherPersist: true } } + ); + let { container } = render(); + + expect(getHtml(container)).toMatch("Num fetchers: 0"); + + fireEvent.click(screen.getByText("Submit (idle)")); + expect(getHtml(container)).toMatch("Num fetchers: 1"); + expect(getHtml(container)).toMatch("Submit (submitting)"); + + fireEvent.click(screen.getByText("Go to /page")); + await waitFor(() => screen.getByText("Page")); + expect(getHtml(container)).toMatch("Num fetchers: 1"); + + // Resolve after the navigation - trigger cleanup + // We don't process the redirect here since it was superseded by a + // navigation, but we assert that it gets cleaned up afterwards + dfd.resolve(redirect("/redirect")); + await waitFor(() => screen.getByText("Num fetchers: 0")); + expect(getHtml(container)).toMatch("Page"); + }); + + it("submitting fetcher.Form persist until completion", async () => { + let dfd = createDeferred(); + let router = createTestRouter( + [ + { + path: "/", + Component() { + let fetchers = useFetchers(); + return ( + <> + 
{`Num fetchers: ${fetchers.length}`}
+ Go to /page + + + ); + }, + children: [ + { + index: true, + Component() { + let fetcher = useFetcher(); + return ( + + + + ); + }, + }, + { + path: "page", + Component() { + return

Page

; + }, + }, + ], + }, + { + path: "/fetch", + action: () => dfd.promise, + }, + ], + { window: getWindow("/"), future: { v7_fetcherPersist: true } } + ); + let { container } = render(); + + expect(getHtml(container)).toMatch("Num fetchers: 0"); + + fireEvent.click(screen.getByText("Submit (idle)")); + expect(getHtml(container)).toMatch("Num fetchers: 1"); + expect(getHtml(container)).toMatch("Submit (submitting)"); + + fireEvent.click(screen.getByText("Go to /page")); + await waitFor(() => screen.getByText("Page")); + expect(getHtml(container)).toMatch("Num fetchers: 1"); + + // Resolve after the navigation and revalidation + dfd.resolve("FETCH"); + await waitFor(() => screen.getByText("Num fetchers: 0")); + }); + }); + }); + describe("
", () => { function setupTest( method: "get" | "post", diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index 817ed1d8b1..7e73c66d6b 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -59,6 +59,7 @@ import { UNSAFE_invariant as invariant, UNSAFE_warning as warning, matchPath, + IDLE_FETCHER, } from "@remix-run/router"; import type { @@ -644,7 +645,7 @@ function DataRoutes({ export interface BrowserRouterProps { basename?: string; children?: React.ReactNode; - future?: FutureConfig; + future?: Partial; window?: Window; } @@ -693,7 +694,7 @@ export function BrowserRouter({ export interface HashRouterProps { basename?: string; children?: React.ReactNode; - future?: FutureConfig; + future?: Partial; window?: Window; } @@ -1217,6 +1218,7 @@ enum DataRouterHook { } enum DataRouterStateHook { + UseFetcher = "useFetcher", UseFetchers = "useFetchers", UseScrollRestoration = "useScrollRestoration", } @@ -1567,6 +1569,7 @@ export function useFetcher({ key, }: { key?: string } = {}): FetcherWithComponents { let { router } = useDataRouterContext(DataRouterHook.UseFetcher); + let state = useDataRouterState(DataRouterStateHook.UseFetcher); let fetchersContext = React.useContext(FetchersContext); let route = React.useContext(RouteContext); let routeId = route.matches[route.matches.length - 1]?.route.id; @@ -1592,11 +1595,11 @@ export function useFetcher({ React.useEffect(() => { register(fetcherKey); return () => { + // Unregister from ref counting for the data layer unregister(fetcherKey); - if (!router) { - console.warn(`No router available to clean up from useFetcher()`); - return; - } + // Tell the router we've unmounted - if v7_fetcherPersist is enabled this + // will not delete immediately but instead queue up a delete after the + // fetcher returns to an `idle` state router.deleteFetcher(fetcherKey); }; }, [router, fetcherKey, register, unregister]); @@ -1604,12 +1607,12 @@ export function useFetcher({ // Fetcher additions let load = React.useCallback( (href: string) => { - invariant(router, "No router available for fetcher.load()"); invariant(routeId, "No routeId available for fetcher.load()"); router.fetch(fetcherKey, routeId, href); }, [fetcherKey, routeId, router] ); + let submitImpl = useSubmit(); let submit = React.useCallback( (target, opts) => { @@ -1621,6 +1624,7 @@ export function useFetcher({ }, [fetcherKey, submitImpl] ); + let FetcherForm = React.useMemo(() => { let FetcherForm = React.forwardRef( (props, ref) => { @@ -1636,7 +1640,7 @@ export function useFetcher({ }, [fetcherKey]); // Exposed FetcherWithComponents - let fetcher = router.getFetcher(fetcherKey); + let fetcher = state.fetchers.get(fetcherKey) || IDLE_FETCHER; let data = fetcherData.get(fetcherKey); let fetcherWithComponents = React.useMemo( () => ({ diff --git a/packages/react-router/lib/components.tsx b/packages/react-router/lib/components.tsx index 4ec7dbc856..3c4a95e2f2 100644 --- a/packages/react-router/lib/components.tsx +++ b/packages/react-router/lib/components.tsx @@ -185,7 +185,7 @@ export interface MemoryRouterProps { children?: React.ReactNode; initialEntries?: InitialEntry[]; initialIndex?: number; - future?: FutureConfig; + future?: Partial; } /** diff --git a/packages/router/router.ts b/packages/router/router.ts index 0f8a0f2b62..971646b6f8 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -343,6 +343,7 @@ export type HydrationState = Partial< * Future flags to toggle new feature behavior */ export interface FutureConfig { + v7_fetcherPersist: boolean; v7_normalizeFormMethod: boolean; v7_prependBasename: boolean; } @@ -763,6 +764,7 @@ export function createRouter(init: RouterInit): Router { let basename = init.basename || "/"; // Config driven behavior flags let future: FutureConfig = { + v7_fetcherPersist: false, v7_normalizeFormMethod: false, v7_prependBasename: false, ...init.future, @@ -885,6 +887,10 @@ export function createRouter(init: RouterInit): Router { // Most recent href/match for fetcher.load calls for fetchers let fetchLoadMatches = new Map(); + // Fetchers that have requested a delete when using v7_fetcherPersist, + // they'll be officially removed after they return to idle + let deletedFetchers = new Set(); + // Store DeferredData instances for active route matches. When a // route loader returns defer() we stick one in here. Then, when a nested // promise resolves we update loaderData. If a new navigation starts we @@ -1017,6 +1023,24 @@ export function createRouter(init: RouterInit): Router { subscribers.forEach((subscriber) => subscriber(state, { unstable_viewTransitionOpts: viewTransitionOpts }) ); + + // Remove idle fetchers from state since we only care about in-flight fetchers. + if (future.v7_fetcherPersist) { + state.fetchers.forEach((fetcher, key) => { + if (fetcher.state === "idle") { + if (deletedFetchers.has(key)) { + // If the fetcher has unmounted and called router.deleteFetcher(), + // we can totally delete the fetcher + deleteFetcher(key); + } else { + // Otherwise, it must still be mounted in the UI so we just remove + // it from state now that we've handed off the data to the React + // layer. Things such as fetchLoadMatches remain for revalidation. + state.fetchers.delete(key); + } + } + }); + } } // Complete a navigation returning the state.navigation back to the IDLE_NAVIGATION @@ -2009,7 +2033,7 @@ export function createRouter(init: RouterInit): Router { state.fetchers.set(key, doneFetcher); } - let didAbortFetchLoads = abortStaleFetchLoads(loadId); + abortStaleFetchLoads(loadId); // 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 @@ -2039,9 +2063,7 @@ export function createRouter(init: RouterInit): Router { matches, errors ), - ...(didAbortFetchLoads || revalidatingFetchers.length > 0 - ? { fetchers: new Map(state.fetchers) } - : {}), + fetchers: new Map(state.fetchers), }); isRevalidationRequired = false; } @@ -2376,9 +2398,19 @@ export function createRouter(init: RouterInit): Router { fetchLoadMatches.delete(key); fetchReloadIds.delete(key); fetchRedirectIds.delete(key); + deletedFetchers.delete(key); state.fetchers.delete(key); } + function deleteFetcherAndUpdateState(key: string): void { + if (future.v7_fetcherPersist) { + deletedFetchers.add(key); + } else { + deleteFetcher(key); + } + updateState({ fetchers: new Map(state.fetchers) }); + } + function abortFetcher(key: string) { let controller = fetchControllers.get(key); invariant(controller, `Expected fetch controller: ${key}`); @@ -2613,7 +2645,7 @@ export function createRouter(init: RouterInit): Router { createHref: (to: To) => init.history.createHref(to), encodeLocation: (to: To) => init.history.encodeLocation(to), getFetcher, - deleteFetcher, + deleteFetcher: deleteFetcherAndUpdateState, dispose, getBlocker, deleteBlocker,