From e96a93ce7bcd4255dd2ec75e7ee0032099bb2299 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 8 Nov 2023 12:50:05 -0500 Subject: [PATCH 1/8] Add unstable_flushSync option --- .changeset/flush-sync-router.md | 5 + .changeset/flush-sync.md | 6 + .../__tests__/flush-sync-navigations-test.tsx | 403 ++++++++++++++++++ packages/react-router-dom/dom.ts | 5 + packages/react-router-dom/index.tsx | 106 ++++- packages/react-router/lib/context.ts | 1 + packages/router/__tests__/flush-sync-test.ts | 168 ++++++++ .../__tests__/utils/data-router-setup.ts | 6 +- packages/router/router.ts | 227 ++++++---- 9 files changed, 826 insertions(+), 101 deletions(-) create mode 100644 .changeset/flush-sync-router.md create mode 100644 .changeset/flush-sync.md create mode 100644 packages/react-router-dom/__tests__/flush-sync-navigations-test.tsx create mode 100644 packages/router/__tests__/flush-sync-test.ts diff --git a/.changeset/flush-sync-router.md b/.changeset/flush-sync-router.md new file mode 100644 index 0000000000..9c0d778fe1 --- /dev/null +++ b/.changeset/flush-sync-router.md @@ -0,0 +1,5 @@ +--- +"@remix-run/router": minor +--- + +Add `unstable_flushSync` option to `router.navigate` and `router.fetch` to tell the React Router layer to opt-out of `React.startTransition` and into `ReactDOM.flushSync` for state updates diff --git a/.changeset/flush-sync.md b/.changeset/flush-sync.md new file mode 100644 index 0000000000..62cc831832 --- /dev/null +++ b/.changeset/flush-sync.md @@ -0,0 +1,6 @@ +--- +"react-router-dom": minor +"react-router": minor +--- + +Add `unstable_flushSync` option to `Link`/`Form`/`useNavigate`/`useSumbit`/`fetcher.load`/`fetcher.submit` to opt-out of `React.startTransition` and into `ReactDOM.flushSync` for state updates diff --git a/packages/react-router-dom/__tests__/flush-sync-navigations-test.tsx b/packages/react-router-dom/__tests__/flush-sync-navigations-test.tsx new file mode 100644 index 0000000000..225da76b08 --- /dev/null +++ b/packages/react-router-dom/__tests__/flush-sync-navigations-test.tsx @@ -0,0 +1,403 @@ +import * as React from "react"; +import { + RouterProvider, + createBrowserRouter, + Form, + Link, + useNavigate, + useSubmit, + useFetcher, +} from "react-router-dom"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { JSDOM } from "jsdom"; + +describe("flushSync", () => { + it("wraps Link updates in flushSync when specified", async () => { + let router = createBrowserRouter( + [ + { + path: "/", + Component() { + return ( + <> +

Home

+ Go to /about + + ); + }, + }, + { + path: "/about", + Component() { + return ( + <> +

About

+ + Go to / + + + ); + }, + }, + ], + { + window: getWindowImpl("/"), + } + ); + render( + + ); + + // This isn't the best way to test this but it seems that startTransition is + // performing sync updates in the test/JSDOM/whatever environment which is + // not how it behaves in the live DOM :/ + let spy = jest.fn(); + router.subscribe(spy); + + fireEvent.click(screen.getByText("Go to /about")); + await waitFor(() => screen.getByText("About")); + expect(spy).toBeCalledTimes(1); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: false }) + ); + + fireEvent.click(screen.getByText("Go to /")); + await waitFor(() => screen.getByText("Home")); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: true }) + ); + + expect(spy).toBeCalledTimes(2); + }); + + it("wraps useNavigate updates in flushSync when specified", async () => { + let router = createBrowserRouter( + [ + { + path: "/", + Component() { + let navigate = useNavigate(); + return ( + <> +

Home

+ + + ); + }, + }, + { + path: "/about", + Component() { + let navigate = useNavigate(); + return ( + <> +

About

+ + + ); + }, + }, + ], + { + window: getWindowImpl("/"), + } + ); + render( + + ); + + // This isn't the best way to test this but it seems that startTransition is + // performing sync updates in the test/JSDOM/whatever environment which is + // not how it behaves in the live DOM :/ + let spy = jest.fn(); + router.subscribe(spy); + + fireEvent.click(screen.getByText("Go to /about")); + await waitFor(() => screen.getByText("About")); + expect(spy).toBeCalledTimes(1); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: false }) + ); + + fireEvent.click(screen.getByText("Go to /")); + await waitFor(() => screen.getByText("Home")); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: true }) + ); + + expect(spy).toBeCalledTimes(2); + }); + + it("wraps Form updates in flushSync when specified", async () => { + let router = createBrowserRouter( + [ + { + path: "/", + action: () => null, + Component() { + return ( + <> +

Home

+
+ +
+ + ); + }, + }, + { + path: "/about", + action: () => null, + Component() { + return ( + <> +

About

+
+ +
+ + ); + }, + }, + ], + { + window: getWindowImpl("/"), + } + ); + render( + + ); + + // This isn't the best way to test this but it seems that startTransition is + // performing sync updates in the test/JSDOM/whatever environment which is + // not how it behaves in the live DOM :/ + let spy = jest.fn(); + router.subscribe(spy); + + fireEvent.click(screen.getByText("Go to /about")); + await waitFor(() => screen.getByText("About")); + expect(spy).toBeCalledTimes(2); + expect(spy.mock.calls[0][1].unstable_flushSync).toBe(false); + expect(spy.mock.calls[1][1].unstable_flushSync).toBe(false); + + fireEvent.click(screen.getByText("Go to /")); + await waitFor(() => screen.getByText("Home")); + expect(spy).toBeCalledTimes(4); + expect(spy.mock.calls[2][1].unstable_flushSync).toBe(true); + expect(spy.mock.calls[3][1].unstable_flushSync).toBe(true); + }); + + it("wraps useSubmit updates in flushSync when specified", async () => { + let router = createBrowserRouter( + [ + { + path: "/", + action: () => null, + Component() { + let submit = useSubmit(); + return ( + <> +

Home

+ + + ); + }, + }, + { + path: "/about", + action: () => null, + Component() { + let submit = useSubmit(); + return ( + <> +

About

+ + + ); + }, + }, + ], + { + window: getWindowImpl("/"), + } + ); + render( + + ); + + // This isn't the best way to test this but it seems that startTransition is + // performing sync updates in the test/JSDOM/whatever environment which is + // not how it behaves in the live DOM :/ + let spy = jest.fn(); + router.subscribe(spy); + + fireEvent.click(screen.getByText("Go to /about")); + await waitFor(() => screen.getByText("About")); + expect(spy).toBeCalledTimes(2); + expect(spy.mock.calls[0][1].unstable_flushSync).toBe(false); + expect(spy.mock.calls[1][1].unstable_flushSync).toBe(false); + + fireEvent.click(screen.getByText("Go to /")); + await waitFor(() => screen.getByText("Home")); + expect(spy).toBeCalledTimes(4); + expect(spy.mock.calls[2][1].unstable_flushSync).toBe(true); + expect(spy.mock.calls[3][1].unstable_flushSync).toBe(true); + }); + + it("wraps fetcher.load updates in flushSync when specified", async () => { + let router = createBrowserRouter( + [ + { + path: "/", + Component() { + let fetcher1 = useFetcher(); + let fetcher2 = useFetcher(); + return ( + <> +

Home

+ + 
{`async:${fetcher1.data}:${fetcher1.state}`}
+ + 
{`sync:${fetcher2.data}:${fetcher2.state}`}
+ + ); + }, + }, + { + path: "/fetch", + loader: async () => { + await new Promise((r) => setTimeout(r, 10)); + return "LOADER"; + }, + }, + ], + { + window: getWindowImpl("/"), + } + ); + render( + + ); + + // This isn't the best way to test this but it seems that startTransition is + // performing sync updates in the test/JSDOM/whatever environment which is + // not how it behaves in the live DOM :/ + let spy = jest.fn(); + router.subscribe(spy); + + fireEvent.click(screen.getByText("Load async")); + await waitFor(() => screen.getByText("async:LOADER:idle")); + expect(spy).toBeCalledTimes(2); + expect(spy.mock.calls[0][1].unstable_flushSync).toBe(false); + expect(spy.mock.calls[1][1].unstable_flushSync).toBe(false); + + fireEvent.click(screen.getByText("Load sync")); + await waitFor(() => screen.getByText("sync:LOADER:idle")); + expect(spy).toBeCalledTimes(4); + expect(spy.mock.calls[2][1].unstable_flushSync).toBe(true); + expect(spy.mock.calls[3][1].unstable_flushSync).toBe(true); + }); + + it("wraps fetcher.submit updates in flushSync when specified", async () => { + let router = createBrowserRouter( + [ + { + path: "/", + action: () => "ACTION", + Component() { + let fetcher1 = useFetcher(); + let fetcher2 = useFetcher(); + return ( + <> +

Home

+ + 
{`async:${fetcher1.data}:${fetcher1.state}`}
+ + 
{`sync:${fetcher2.data}:${fetcher2.state}`}
+ + ); + }, + }, + ], + { + window: getWindowImpl("/"), + } + ); + render( + + ); + + // This isn't the best way to test this but it seems that startTransition is + // performing sync updates in the test/JSDOM/whatever environment which is + // not how it behaves in the live DOM :/ + let spy = jest.fn(); + router.subscribe(spy); + + fireEvent.click(screen.getByText("Submit async")); + await waitFor(() => screen.getByText("async:ACTION:idle")); + expect(spy).toBeCalledTimes(3); + expect(spy.mock.calls[0][1].unstable_flushSync).toBe(false); + expect(spy.mock.calls[1][1].unstable_flushSync).toBe(false); + expect(spy.mock.calls[2][1].unstable_flushSync).toBe(false); + + fireEvent.click(screen.getByText("Submit sync")); + await waitFor(() => screen.getByText("sync:ACTION:idle")); + expect(spy).toBeCalledTimes(6); + expect(spy.mock.calls[3][1].unstable_flushSync).toBe(true); + expect(spy.mock.calls[4][1].unstable_flushSync).toBe(true); + expect(spy.mock.calls[5][1].unstable_flushSync).toBe(true); + }); +}); + +function getWindowImpl(initialUrl: string, isHash = false): Window { + // Need to use our own custom DOM in order to get a working history + const dom = new JSDOM(``, { url: "http://localhost/" }); + dom.window.history.replaceState(null, "", (isHash ? "#" : "") + initialUrl); + return dom.window as unknown as Window; +} diff --git a/packages/react-router-dom/dom.ts b/packages/react-router-dom/dom.ts index 1402c2f79a..960e0b5036 100644 --- a/packages/react-router-dom/dom.ts +++ b/packages/react-router-dom/dom.ts @@ -204,6 +204,11 @@ export interface SubmitOptions { */ preventScrollReset?: boolean; + /** + * Enable flushSync for this navigation's state updates + */ + unstable_flushSync?: boolean; + /** * Enable view transitions on this submission navigation */ diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index ec1bc769d2..d64a6d2ef0 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -3,6 +3,7 @@ * you'll need to update the rollup config for react-router-dom-v5-compat. */ import * as React from "react"; +import * as ReactDOM from "react-dom"; import type { DataRouteObject, FutureConfig, @@ -338,6 +339,7 @@ type ViewTransitionContextObject = } | { isTransitioning: true; + flushSync: boolean; currentLocation: Location; nextLocation: Location; }; @@ -390,6 +392,8 @@ export { FetchersContext as UNSAFE_FetchersContext }; */ const START_TRANSITION = "startTransition"; const startTransitionImpl = React[START_TRANSITION]; +const FLUSH_SYNC = "flushSync"; +const flushSyncImpl = ReactDOM[FLUSH_SYNC]; function startTransitionSafe(cb: () => void) { if (startTransitionImpl) { @@ -399,6 +403,14 @@ function startTransitionSafe(cb: () => void) { } } +function flushSyncSafe(cb: () => void) { + if (flushSyncImpl) { + flushSyncImpl(cb); + } else { + cb(); + } +} + interface ViewTransition { finished: Promise; ready: Promise; @@ -468,7 +480,11 @@ export function RouterProvider({ let setState = React.useCallback( ( newState: RouterState, - { deletedFetchers, unstable_viewTransitionOpts: viewTransitionOpts } + { + deletedFetchers, + unstable_flushSync: flushSync, + unstable_viewTransitionOpts: viewTransitionOpts, + } ) => { deletedFetchers.forEach((key) => fetcherData.current.delete(key)); newState.fetchers.forEach((fetcher, key) => { @@ -477,17 +493,62 @@ export function RouterProvider({ } }); - if ( - !viewTransitionOpts || + let isViewTransitionUnavailable = router.window == null || - typeof router.window.document.startViewTransition !== "function" - ) { - // Mid-navigation state update, or startViewTransition isn't available - optInStartTransition(() => setStateImpl(newState)); - } else if (transition && renderDfd) { + typeof router.window.document.startViewTransition !== "function"; + + // If this isn't a view transition or it's not available in this browser, + // just update and be done with it + if (!viewTransitionOpts || isViewTransitionUnavailable) { + if (flushSync) { + flushSyncSafe(() => setStateImpl(newState)); + } else { + optInStartTransition(() => setStateImpl(newState)); + } + return; + } + + // flushSync + startViewTransition + if (flushSync) { + // Flush through the context to mark DOM elements as transition=ing + flushSyncSafe(() => { + // Cancel any pending transitions + if (transition) { + renderDfd && renderDfd.resolve(); + transition.skipTransition(); + } + setVtContext({ + isTransitioning: true, + flushSync: true, + currentLocation: viewTransitionOpts.currentLocation, + nextLocation: viewTransitionOpts.nextLocation, + }); + }); + + // Update the DOM + let t = router.window!.document.startViewTransition(() => { + flushSyncSafe(() => setStateImpl(newState)); + }); + + // Clean up after the animation completes + t.finished.finally(() => { + flushSyncSafe(() => { + setRenderDfd(undefined); + setTransition(undefined); + setPendingState(undefined); + setVtContext({ isTransitioning: false }); + }); + }); + + flushSyncSafe(() => setTransition(t)); + return; + } + + // startTransition + startViewTransition + if (transition) { // Interrupting an in-progress transition, cancel and let everything flush // out, and then kick off a new transition from the interruption state - renderDfd.resolve(); + renderDfd && renderDfd.resolve(); transition.skipTransition(); setInterruption({ state: newState, @@ -499,6 +560,7 @@ export function RouterProvider({ setPendingState(newState); setVtContext({ isTransitioning: true, + flushSync: false, currentLocation: viewTransitionOpts.currentLocation, nextLocation: viewTransitionOpts.nextLocation, }); @@ -514,10 +576,10 @@ export function RouterProvider({ // When we start a view transition, create a Deferred we can use for the // eventual "completed" render React.useEffect(() => { - if (vtContext.isTransitioning) { + if (vtContext.isTransitioning && !vtContext.flushSync) { setRenderDfd(new Deferred()); } - }, [vtContext.isTransitioning]); + }, [vtContext]); // Once the deferred is created, kick off startViewTransition() to update the // DOM and then wait on the Deferred to resolve (indicating the DOM update has @@ -559,6 +621,7 @@ export function RouterProvider({ setPendingState(interruption.state); setVtContext({ isTransitioning: true, + flushSync: false, currentLocation: interruption.currentLocation, nextLocation: interruption.nextLocation, }); @@ -799,6 +862,7 @@ export interface LinkProps preventScrollReset?: boolean; relative?: RelativeRoutingType; to: To; + unstable_flushSync?: boolean; unstable_viewTransition?: boolean; } @@ -823,6 +887,7 @@ export const Link = React.forwardRef( target, to, preventScrollReset, + unstable_flushSync, unstable_viewTransition, ...rest }, @@ -873,6 +938,7 @@ export const Link = React.forwardRef( target, preventScrollReset, relative, + unstable_flushSync, unstable_viewTransition, }); function handleClick( @@ -916,7 +982,6 @@ export interface NavLinkProps style?: | React.CSSProperties | ((props: NavLinkRenderProps) => React.CSSProperties | undefined); - unstable_viewTransition?: boolean; } /** @@ -1107,6 +1172,11 @@ export interface FormProps extends FetcherFormProps { */ state?: any; + /** + * Enable flushSync navigation's staten updates + */ + unstable_flushSync?: boolean; + /** * Enable view transitions on this Form navigation */ @@ -1140,6 +1210,7 @@ export const Form = React.forwardRef( onSubmit, relative, preventScrollReset, + unstable_flushSync, unstable_viewTransition, ...props }, @@ -1170,6 +1241,7 @@ export const Form = React.forwardRef( state, relative, preventScrollReset, + unstable_flushSync, unstable_viewTransition, }); }; @@ -1265,6 +1337,7 @@ export function useLinkClickHandler( state, preventScrollReset, relative, + unstable_flushSync, unstable_viewTransition, }: { target?: React.HTMLAttributeAnchorTarget; @@ -1272,6 +1345,7 @@ export function useLinkClickHandler( state?: any; preventScrollReset?: boolean; relative?: RelativeRoutingType; + unstable_flushSync?: boolean; unstable_viewTransition?: boolean; } = {} ): (event: React.MouseEvent) => void { @@ -1296,6 +1370,7 @@ export function useLinkClickHandler( state, preventScrollReset, relative, + unstable_flushSync, unstable_viewTransition, }); } @@ -1310,6 +1385,7 @@ export function useLinkClickHandler( to, preventScrollReset, relative, + unstable_flushSync, unstable_viewTransition, ] ); @@ -1444,6 +1520,7 @@ export function useSubmit(): SubmitFunction { body, formMethod: options.method || (method as HTMLFormMethod), formEncType: options.encType || (encType as FormEncType), + unstable_flushSync: options.unstable_flushSync, }); } else { router.navigate(options.action || action, { @@ -1455,6 +1532,7 @@ export function useSubmit(): SubmitFunction { replace: options.replace, state: options.state, fromRouteId: currentRouteId, + unstable_flushSync: options.unstable_flushSync, unstable_viewTransition: options.unstable_viewTransition, }); } @@ -1564,9 +1642,9 @@ export function useFetcher({ // Fetcher additions let load = React.useCallback( - (href: string) => { + (href: string, opts?: { unstable_flushSync?: boolean }) => { invariant(routeId, "No routeId available for fetcher.load()"); - router.fetch(fetcherKey, routeId, href); + router.fetch(fetcherKey, routeId, href, opts); }, [fetcherKey, routeId, router] ); diff --git a/packages/react-router/lib/context.ts b/packages/react-router/lib/context.ts index 212dc7160a..3e47ac2d05 100644 --- a/packages/react-router/lib/context.ts +++ b/packages/react-router/lib/context.ts @@ -94,6 +94,7 @@ export interface NavigateOptions { state?: any; preventScrollReset?: boolean; relative?: RelativeRoutingType; + unstable_flushSync?: boolean; unstable_viewTransition?: boolean; } diff --git a/packages/router/__tests__/flush-sync-test.ts b/packages/router/__tests__/flush-sync-test.ts new file mode 100644 index 0000000000..9b454bd51e --- /dev/null +++ b/packages/router/__tests__/flush-sync-test.ts @@ -0,0 +1,168 @@ +import { IDLE_NAVIGATION } from "../router"; +import { cleanup, setup } from "./utils/data-router-setup"; +import { createFormData, tick } from "./utils/utils"; + +describe("flushSync", () => { + // Detect any failures inside the router navigate code + afterEach(() => cleanup()); + + it("supports GET navigations with flushSync", async () => { + let t = setup({ + routes: [ + { path: "/" }, + { id: "a", path: "/a", loader: true }, + { id: "b", path: "/b", loader: true }, + ], + }); + let spy = jest.fn(); + let unsubscribe = t.router.subscribe(spy); + + let A = await t.navigate("/a"); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: false }) + ); + await A.loaders.a.resolve("A"); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: false }) + ); + + let B = await t.navigate("/b", { unstable_flushSync: true }); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: true }) + ); + await B.loaders.b.resolve("B"); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: true }) + ); + + unsubscribe(); + t.router.dispose(); + }); + + it("supports POST navigations with flushSync", async () => { + let t = setup({ + routes: [ + { path: "/" }, + { id: "a", path: "/a", action: true }, + { id: "b", path: "/b", action: true }, + ], + }); + let spy = jest.fn(); + let unsubscribe = t.router.subscribe(spy); + + let A = await t.navigate("/a", { + formMethod: "post", + formData: createFormData({}), + }); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: false }) + ); + await A.actions.a.resolve("A"); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: false }) + ); + + let B = await t.navigate("/b", { + formMethod: "post", + formData: createFormData({}), + unstable_flushSync: true, + }); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: true }) + ); + await B.actions.b.resolve("B"); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: true }) + ); + + unsubscribe(); + t.router.dispose(); + }); + + it("supports fetch loads with flushSync", async () => { + let t = setup({ + routes: [{ id: "root", path: "/", loader: true }], + }); + let spy = jest.fn(); + let unsubscribe = t.router.subscribe(spy); + + let key = "key"; + let A = await t.fetch("/", key); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: false }) + ); + expect(t.router.state.fetchers.get(key)?.state).toBe("loading"); + + await A.loaders.root.resolve("ROOT"); + expect(t.router.state.fetchers.get(key)?.data).toBe("ROOT"); + + let B = await t.fetch("/", key, { unstable_flushSync: true }); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: true }) + ); + expect(t.router.state.fetchers.get(key)?.state).toBe("loading"); + + await B.loaders.root.resolve("ROOT2"); + expect(t.router.state.fetchers.get(key)?.data).toBe("ROOT2"); + + unsubscribe(); + t.router.dispose(); + }); + + it("supports fetch submissions with flushSync", async () => { + let t = setup({ + routes: [{ id: "root", path: "/", action: true }], + }); + let spy = jest.fn(); + let unsubscribe = t.router.subscribe(spy); + + let key = "key"; + let A = await t.fetch("/", key, { + formMethod: "post", + formData: createFormData({}), + }); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: false }) + ); + expect(t.router.state.fetchers.get(key)?.state).toBe("submitting"); + + await A.actions.root.resolve("ROOT"); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: false }) + ); + expect(t.router.state.fetchers.get(key)?.data).toBe("ROOT"); + + let B = await t.fetch("/", key, { + formMethod: "post", + formData: createFormData({}), + unstable_flushSync: true, + }); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: true }) + ); + expect(t.router.state.fetchers.get(key)?.state).toBe("submitting"); + + await B.actions.root.resolve("ROOT2"); + expect(t.router.state.fetchers.get(key)?.data).toBe("ROOT2"); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: true }) + ); + + unsubscribe(); + t.router.dispose(); + }); +}); diff --git a/packages/router/__tests__/utils/data-router-setup.ts b/packages/router/__tests__/utils/data-router-setup.ts index 9fae3d2cab..94bcee48fa 100644 --- a/packages/router/__tests__/utils/data-router-setup.ts +++ b/packages/router/__tests__/utils/data-router-setup.ts @@ -221,7 +221,7 @@ export function setup({ activeActionFetchId, ].sort(); let helperKey = sortedIds - .map((id) => `${id}:lazy:${r.id}`) + .map((id) => `${id}:lazy:${enhancedRoute.id}`) .find((k) => activeHelpers.has(k)); invariant(helperKey != null, `No helperKey found`); let helpers = activeHelpers.get(helperKey); @@ -239,7 +239,7 @@ export function setup({ activeLoaderType === "fetch" ? activeLoaderFetchId : activeLoaderNavigationId; - let helperKey = `${activeLoaderType}:${navigationId}:loader:${r.id}`; + let helperKey = `${activeLoaderType}:${navigationId}:loader:${enhancedRoute.id}`; let helpers = activeHelpers.get(helperKey); invariant(helpers, `No helpers found for: ${helperKey}`); helpers.stub(args); @@ -254,7 +254,7 @@ export function setup({ activeActionType === "fetch" ? activeActionFetchId : activeActionNavigationId; - let helperKey = `${activeActionType}:${navigationId}:action:${r.id}`; + let helperKey = `${activeActionType}:${navigationId}:action:${enhancedRoute.id}`; let helpers = activeHelpers.get(helperKey); invariant(helpers, `No helpers found for: ${helperKey}`); helpers.stub(args); diff --git a/packages/router/router.ts b/packages/router/router.ts index 47977e0a29..4f1825bbaf 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -411,6 +411,7 @@ export interface RouterSubscriber { opts: { deletedFetchers: string[]; unstable_viewTransitionOpts?: ViewTransitionOpts; + unstable_flushSync?: boolean; } ): void; } @@ -436,6 +437,7 @@ export type RelativeRoutingType = "route" | "path"; type BaseNavigateOrFetchOptions = { preventScrollReset?: boolean; relative?: RelativeRoutingType; + unstable_flushSync?: boolean; }; // Only allowed for navigations @@ -841,6 +843,9 @@ export function createRouter(init: RouterInit): Router { // Should the current navigation enable document.startViewTransition? let pendingViewTransitionEnabled = false; + // Should the current navigation use flushSync for state updates? + let pendingFlushSync = false; + // Store applied view transitions so we can apply them on POP let appliedViewTransitions: Map> = new Map< string, @@ -962,7 +967,7 @@ export function createRouter(init: RouterInit): Router { reset() { let blockers = new Map(state.blockers); blockers.set(blockerKey!, IDLE_BLOCKER); - updateState({ blockers }); + updateState({ blockers }, { flushSync: false }); }, }); return; @@ -1018,7 +1023,10 @@ export function createRouter(init: RouterInit): Router { // Update our state and notify the calling context of the change function updateState( newState: Partial, - viewTransitionOpts?: ViewTransitionOpts + opts: { + flushSync: boolean; + viewTransitionOpts?: ViewTransitionOpts; + } ): void { state = { ...state, @@ -1045,10 +1053,14 @@ export function createRouter(init: RouterInit): Router { }); } - subscribers.forEach((subscriber) => + // Iterate over a local copy so that if flushSync is used and we end up + // removing and adding a new subscriber due to the useCallback dependencies, + // we don't get ourselves into a loop calling the new subscriber immediately + [...subscribers].forEach((subscriber) => subscriber(state, { deletedFetchers: deletedFetchersKeys, - unstable_viewTransitionOpts: viewTransitionOpts, + unstable_viewTransitionOpts: opts.viewTransitionOpts, + unstable_flushSync: opts.flushSync, }) ); @@ -1188,13 +1200,17 @@ export function createRouter(init: RouterInit): Router { preventScrollReset, blockers, }, - viewTransitionOpts + { + viewTransitionOpts, + flushSync: pendingFlushSync, + } ); // Reset stateful navigation vars pendingAction = HistoryAction.Pop; pendingPreventScrollReset = false; pendingViewTransitionEnabled = false; + pendingFlushSync = false; isUninterruptedRevalidation = false; isRevalidationRequired = false; cancelledDeferredRoutes = []; @@ -1266,6 +1282,8 @@ export function createRouter(init: RouterInit): Router { ? opts.preventScrollReset === true : undefined; + let flushSync = (opts && opts.unstable_flushSync) === true; + let blockerKey = shouldBlockNavigation({ currentLocation, nextLocation, @@ -1290,7 +1308,7 @@ export function createRouter(init: RouterInit): Router { reset() { let blockers = new Map(state.blockers); blockers.set(blockerKey!, IDLE_BLOCKER); - updateState({ blockers }); + updateState({ blockers }, { flushSync }); }, }); return; @@ -1304,6 +1322,7 @@ export function createRouter(init: RouterInit): Router { preventScrollReset, replace: opts && opts.replace, enableViewTransition: opts && opts.unstable_viewTransition, + flushSync, }); } @@ -1312,7 +1331,7 @@ export function createRouter(init: RouterInit): Router { // loaders during the next loader round function revalidate() { interruptActiveLoads(); - updateState({ revalidation: "loading" }); + updateState({ revalidation: "loading" }, { flushSync: false }); // If we're currently submitting an action, we don't need to start a new // navigation, we'll just let the follow up loader execution call all loaders @@ -1355,6 +1374,7 @@ export function createRouter(init: RouterInit): Router { preventScrollReset?: boolean; replace?: boolean; enableViewTransition?: boolean; + flushSync?: boolean; } ): Promise { // Abort any in-progress navigations and start a new one. Unset any ongoing @@ -1372,6 +1392,7 @@ export function createRouter(init: RouterInit): Router { pendingPreventScrollReset = (opts && opts.preventScrollReset) === true; pendingViewTransitionEnabled = (opts && opts.enableViewTransition) === true; + pendingFlushSync = (opts && opts.flushSync) === true; let routesToUse = inFlightDataRoutes || dataRoutes; let loadingNavigation = opts && opts.overrideNavigation; @@ -1498,7 +1519,7 @@ export function createRouter(init: RouterInit): Router { // Put us in a submitting state let navigation = getSubmittingNavigation(location, submission); - updateState({ navigation }); + updateState({ navigation }, { flushSync: pendingFlushSync }); // Call our action and get the result let result: DataResult; @@ -1654,17 +1675,22 @@ export function createRouter(init: RouterInit): Router { state.fetchers.set(rf.key, revalidatingFetcher); }); let actionData = pendingActionData || state.actionData; - updateState({ - navigation: loadingNavigation, - ...(actionData - ? Object.keys(actionData).length === 0 - ? { actionData: null } - : { actionData } - : {}), - ...(revalidatingFetchers.length > 0 - ? { fetchers: new Map(state.fetchers) } - : {}), - }); + updateState( + { + navigation: loadingNavigation, + ...(actionData + ? Object.keys(actionData).length === 0 + ? { actionData: null } + : { actionData } + : {}), + ...(revalidatingFetchers.length > 0 + ? { fetchers: new Map(state.fetchers) } + : {}), + }, + { + flushSync: pendingFlushSync, + } + ); } revalidatingFetchers.forEach((rf) => { @@ -1764,18 +1790,6 @@ export function createRouter(init: RouterInit): Router { }; } - function getFetcher(key: string): Fetcher { - if (future.v7_fetcherPersist) { - activeFetchers.set(key, (activeFetchers.get(key) || 0) + 1); - // If this fetcher was previously marked for deletion, unmark it since we - // have a new instance - if (deletedFetchers.has(key)) { - deletedFetchers.delete(key); - } - } - return state.fetchers.get(key) || IDLE_FETCHER; - } - // Trigger a fetcher load/submit for the given fetcher key function fetch( key: string, @@ -1792,6 +1806,7 @@ export function createRouter(init: RouterInit): Router { } if (fetchControllers.has(key)) abortFetcher(key); + let flushSync = (opts && opts.unstable_flushSync) === true; let routesToUse = inFlightDataRoutes || dataRoutes; let normalizedPath = normalizeTo( @@ -1809,7 +1824,8 @@ export function createRouter(init: RouterInit): Router { setFetcherError( key, routeId, - getInternalRouterError(404, { pathname: normalizedPath }) + getInternalRouterError(404, { pathname: normalizedPath }), + { flushSync } ); return; } @@ -1822,7 +1838,7 @@ export function createRouter(init: RouterInit): Router { ); if (error) { - setFetcherError(key, routeId, error); + setFetcherError(key, routeId, error, { flushSync }); return; } @@ -1831,14 +1847,30 @@ export function createRouter(init: RouterInit): Router { pendingPreventScrollReset = (opts && opts.preventScrollReset) === true; if (submission && isMutationMethod(submission.formMethod)) { - handleFetcherAction(key, routeId, path, match, matches, submission); + handleFetcherAction( + key, + routeId, + path, + match, + matches, + flushSync, + submission + ); return; } // Store off the match so we can call it's shouldRevalidate on subsequent // revalidations fetchLoadMatches.set(key, { routeId, path }); - handleFetcherLoader(key, routeId, path, match, matches, submission); + handleFetcherLoader( + key, + routeId, + path, + match, + matches, + flushSync, + submission + ); } // Call the action for the matched fetcher.submit(), and then handle redirects, @@ -1849,6 +1881,7 @@ export function createRouter(init: RouterInit): Router { path: string, match: AgnosticDataRouteMatch, requestMatches: AgnosticDataRouteMatch[], + flushSync: boolean, submission: Submission ) { interruptActiveLoads(); @@ -1860,15 +1893,15 @@ export function createRouter(init: RouterInit): Router { pathname: path, routeId: routeId, }); - setFetcherError(key, routeId, error); + setFetcherError(key, routeId, error, { flushSync }); return; } // Put this fetcher into it's submitting state let existingFetcher = state.fetchers.get(key); - let fetcher = getSubmittingFetcher(submission, existingFetcher); - state.fetchers.set(key, fetcher); - updateState({ fetchers: new Map(state.fetchers) }); + updateFetcherState(key, getSubmittingFetcher(submission, existingFetcher), { + flushSync, + }); // Call the action for the fetcher let abortController = new AbortController(); @@ -1901,8 +1934,7 @@ export function createRouter(init: RouterInit): Router { } if (deletedFetchers.has(key)) { - state.fetchers.set(key, getDoneFetcher(undefined)); - updateState({ fetchers: new Map(state.fetchers) }); + updateFetcherState(key, getDoneFetcher(undefined), { flushSync }); return; } @@ -1913,16 +1945,11 @@ export function createRouter(init: RouterInit): Router { // should take precedence over this redirect navigation. We already // set isRevalidationRequired so all loaders for the new route should // fire unless opted out via shouldRevalidate - let doneFetcher = getDoneFetcher(undefined); - state.fetchers.set(key, doneFetcher); - updateState({ fetchers: new Map(state.fetchers) }); + updateFetcherState(key, getDoneFetcher(undefined), { flushSync }); return; } else { fetchRedirectIds.add(key); - let loadingFetcher = getLoadingFetcher(submission); - state.fetchers.set(key, loadingFetcher); - updateState({ fetchers: new Map(state.fetchers) }); - + updateFetcherState(key, getLoadingFetcher(submission), { flushSync }); return startRedirectNavigation(state, actionResult, { fetcherSubmission: submission, }); @@ -1931,7 +1958,7 @@ export function createRouter(init: RouterInit): Router { // Process any non-redirect errors thrown if (isErrorResult(actionResult)) { - setFetcherError(key, routeId, actionResult.error); + setFetcherError(key, routeId, actionResult.error, { flushSync }); return; } @@ -1999,7 +2026,7 @@ export function createRouter(init: RouterInit): Router { } }); - updateState({ fetchers: new Map(state.fetchers) }); + updateState({ fetchers: new Map(state.fetchers) }, { flushSync }); let abortPendingFetchRevalidations = () => revalidatingFetchers.forEach((rf) => abortFetcher(rf.key)); @@ -2085,16 +2112,19 @@ export function createRouter(init: RouterInit): Router { // otherwise just update with the fetcher data, preserving any existing // loaderData for loaders that did not need to reload. We have to // manually merge here since we aren't going through completeNavigation - updateState({ - errors, - loaderData: mergeLoaderData( - state.loaderData, - loaderData, - matches, - errors - ), - fetchers: new Map(state.fetchers), - }); + updateState( + { + errors, + loaderData: mergeLoaderData( + state.loaderData, + loaderData, + matches, + errors + ), + fetchers: new Map(state.fetchers), + }, + { flushSync } + ); isRevalidationRequired = false; } } @@ -2106,16 +2136,18 @@ export function createRouter(init: RouterInit): Router { path: string, match: AgnosticDataRouteMatch, matches: AgnosticDataRouteMatch[], + flushSync: boolean, submission?: Submission ) { let existingFetcher = state.fetchers.get(key); - // Put this fetcher into it's loading state - let loadingFetcher = getLoadingFetcher( - submission, - existingFetcher ? existingFetcher.data : undefined + updateFetcherState( + key, + getLoadingFetcher( + submission, + existingFetcher ? existingFetcher.data : undefined + ), + { flushSync } ); - state.fetchers.set(key, loadingFetcher); - updateState({ fetchers: new Map(state.fetchers) }); // Call the loader for this fetcher route match let abortController = new AbortController(); @@ -2158,8 +2190,7 @@ export function createRouter(init: RouterInit): Router { } if (deletedFetchers.has(key)) { - state.fetchers.set(key, getDoneFetcher(undefined)); - updateState({ fetchers: new Map(state.fetchers) }); + updateFetcherState(key, getDoneFetcher(undefined), { flushSync }); return; } @@ -2168,9 +2199,7 @@ export function createRouter(init: RouterInit): Router { if (pendingNavigationLoadId > originatingLoadId) { // A new navigation was kicked off after our loader started, so that // should take precedence over this redirect navigation - let doneFetcher = getDoneFetcher(undefined); - state.fetchers.set(key, doneFetcher); - updateState({ fetchers: new Map(state.fetchers) }); + updateFetcherState(key, getDoneFetcher(undefined), { flushSync }); return; } else { fetchRedirectIds.add(key); @@ -2181,16 +2210,14 @@ export function createRouter(init: RouterInit): Router { // Process any non-redirect errors thrown if (isErrorResult(result)) { - setFetcherError(key, routeId, result.error); + setFetcherError(key, routeId, result.error, { flushSync }); return; } invariant(!isDeferredResult(result), "Unhandled fetcher deferred data"); // Put the fetcher back into an idle state - let doneFetcher = getDoneFetcher(result.data); - state.fetchers.set(key, doneFetcher); - updateState({ fetchers: new Map(state.fetchers) }); + updateFetcherState(key, getDoneFetcher(result.data), { flushSync }); } /** @@ -2399,15 +2426,47 @@ export function createRouter(init: RouterInit): Router { }); } - function setFetcherError(key: string, routeId: string, error: any) { + function updateFetcherState( + key: string, + fetcher: Fetcher, + opts: { flushSync: boolean } + ) { + state.fetchers.set(key, fetcher); + updateState( + { fetchers: new Map(state.fetchers) }, + { flushSync: opts.flushSync } + ); + } + + function setFetcherError( + key: string, + routeId: string, + error: any, + opts: { flushSync: boolean } + ) { let boundaryMatch = findNearestBoundary(state.matches, routeId); deleteFetcher(key); - updateState({ - errors: { - [boundaryMatch.route.id]: error, + updateState( + { + errors: { + [boundaryMatch.route.id]: error, + }, + fetchers: new Map(state.fetchers), }, - fetchers: new Map(state.fetchers), - }); + { flushSync: opts.flushSync } + ); + } + + function getFetcher(key: string): Fetcher { + if (future.v7_fetcherPersist) { + activeFetchers.set(key, (activeFetchers.get(key) || 0) + 1); + // If this fetcher was previously marked for deletion, unmark it since we + // have a new instance + if (deletedFetchers.has(key)) { + deletedFetchers.delete(key); + } + } + return state.fetchers.get(key) || IDLE_FETCHER; } function deleteFetcher(key: string): void { @@ -2440,7 +2499,7 @@ export function createRouter(init: RouterInit): Router { } else { deleteFetcher(key); } - updateState({ fetchers: new Map(state.fetchers) }); + updateState({ fetchers: new Map(state.fetchers) }, { flushSync: false }); } function abortFetcher(key: string) { @@ -2523,7 +2582,7 @@ export function createRouter(init: RouterInit): Router { let blockers = new Map(state.blockers); blockers.set(key, newBlocker); - updateState({ blockers }); + updateState({ blockers }, { flushSync: false }); } function shouldBlockNavigation({ @@ -2597,7 +2656,7 @@ export function createRouter(init: RouterInit): Router { initialScrollRestored = true; let y = getSavedScrollPosition(state.location, state.matches); if (y != null) { - updateState({ restoreScrollPosition: y }); + updateState({ restoreScrollPosition: y }, { flushSync: false }); } } From e192105bd4954b80cfc91d9aae180fa690618b9d Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 8 Nov 2023 12:55:46 -0500 Subject: [PATCH 2/8] Bump bundle --- package.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 1b12bdb1f8..a90d6fc12d 100644 --- a/package.json +++ b/package.json @@ -110,7 +110,7 @@ }, "filesize": { "packages/router/dist/router.umd.min.js": { - "none": "49.0 kB" + "none": "49.2 kB" }, "packages/react-router/dist/react-router.production.min.js": { "none": "13.9 kB" @@ -119,10 +119,10 @@ "none": "16.3 kB" }, "packages/react-router-dom/dist/react-router-dom.production.min.js": { - "none": "16.1 kB" + "none": "16.8 kB" }, "packages/react-router-dom/dist/umd/react-router-dom.production.min.js": { - "none": "22.3 kB" + "none": "23 kB" } } } From 7f4863342484c5191fe5188d15f048c78f7f6b84 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 8 Nov 2023 13:18:04 -0500 Subject: [PATCH 3/8] fix types --- packages/react-router-dom/index.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index d64a6d2ef0..462e3e771a 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -1142,6 +1142,11 @@ export interface FetcherFormProps * `event.preventDefault()` then this form will not do anything. */ onSubmit?: React.FormEventHandler; + + /** + * Enable flushSync navigation's staten updates + */ + unstable_flushSync?: boolean; } export interface FormProps extends FetcherFormProps { @@ -1172,11 +1177,6 @@ export interface FormProps extends FetcherFormProps { */ state?: any; - /** - * Enable flushSync navigation's staten updates - */ - unstable_flushSync?: boolean; - /** * Enable view transitions on this Form navigation */ @@ -1598,7 +1598,7 @@ export type FetcherWithComponents = Fetcher & { FetcherFormProps & React.RefAttributes >; submit: FetcherSubmitFunction; - load: (href: string) => void; + load: (href: string, opts?: { unstable_flushSync?: boolean }) => void; }; // TODO: (v7) Change the useFetcher generic default from `any` to `unknown` From b869460b1ea78ccfde536d80fdfd242f05a44efd Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 13 Nov 2023 12:40:03 -0500 Subject: [PATCH 4/8] Remove from declarative APIs --- .changeset/flush-sync.md | 2 +- .../__tests__/flush-sync-navigations-test.tsx | 125 +----------------- packages/react-router-dom/index.tsx | 14 -- 3 files changed, 8 insertions(+), 133 deletions(-) diff --git a/.changeset/flush-sync.md b/.changeset/flush-sync.md index 62cc831832..ee8253c589 100644 --- a/.changeset/flush-sync.md +++ b/.changeset/flush-sync.md @@ -3,4 +3,4 @@ "react-router": minor --- -Add `unstable_flushSync` option to `Link`/`Form`/`useNavigate`/`useSumbit`/`fetcher.load`/`fetcher.submit` to opt-out of `React.startTransition` and into `ReactDOM.flushSync` for state updates +Add `unstable_flushSync` option to `useNavigate`/`useSumbit`/`fetcher.load`/`fetcher.submit` to opt-out of `React.startTransition` and into `ReactDOM.flushSync` for state updates diff --git a/packages/react-router-dom/__tests__/flush-sync-navigations-test.tsx b/packages/react-router-dom/__tests__/flush-sync-navigations-test.tsx index 225da76b08..c4cf9e2463 100644 --- a/packages/react-router-dom/__tests__/flush-sync-navigations-test.tsx +++ b/packages/react-router-dom/__tests__/flush-sync-navigations-test.tsx @@ -12,66 +12,6 @@ import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { JSDOM } from "jsdom"; describe("flushSync", () => { - it("wraps Link updates in flushSync when specified", async () => { - let router = createBrowserRouter( - [ - { - path: "/", - Component() { - return ( - <> -

Home

- Go to /about - - ); - }, - }, - { - path: "/about", - Component() { - return ( - <> -

About

- - Go to / - - - ); - }, - }, - ], - { - window: getWindowImpl("/"), - } - ); - render( - - ); - - // This isn't the best way to test this but it seems that startTransition is - // performing sync updates in the test/JSDOM/whatever environment which is - // not how it behaves in the live DOM :/ - let spy = jest.fn(); - router.subscribe(spy); - - fireEvent.click(screen.getByText("Go to /about")); - await waitFor(() => screen.getByText("About")); - expect(spy).toBeCalledTimes(1); - expect(spy).toHaveBeenLastCalledWith( - expect.anything(), - expect.objectContaining({ unstable_flushSync: false }) - ); - - fireEvent.click(screen.getByText("Go to /")); - await waitFor(() => screen.getByText("Home")); - expect(spy).toHaveBeenLastCalledWith( - expect.anything(), - expect.objectContaining({ unstable_flushSync: true }) - ); - - expect(spy).toBeCalledTimes(2); - }); - it("wraps useNavigate updates in flushSync when specified", async () => { let router = createBrowserRouter( [ @@ -134,65 +74,8 @@ describe("flushSync", () => { ); expect(spy).toBeCalledTimes(2); - }); - it("wraps Form updates in flushSync when specified", async () => { - let router = createBrowserRouter( - [ - { - path: "/", - action: () => null, - Component() { - return ( - <> -

Home

-
- -
- - ); - }, - }, - { - path: "/about", - action: () => null, - Component() { - return ( - <> -

About

-
- -
- - ); - }, - }, - ], - { - window: getWindowImpl("/"), - } - ); - render( - - ); - - // This isn't the best way to test this but it seems that startTransition is - // performing sync updates in the test/JSDOM/whatever environment which is - // not how it behaves in the live DOM :/ - let spy = jest.fn(); - router.subscribe(spy); - - fireEvent.click(screen.getByText("Go to /about")); - await waitFor(() => screen.getByText("About")); - expect(spy).toBeCalledTimes(2); - expect(spy.mock.calls[0][1].unstable_flushSync).toBe(false); - expect(spy.mock.calls[1][1].unstable_flushSync).toBe(false); - - fireEvent.click(screen.getByText("Go to /")); - await waitFor(() => screen.getByText("Home")); - expect(spy).toBeCalledTimes(4); - expect(spy.mock.calls[2][1].unstable_flushSync).toBe(true); - expect(spy.mock.calls[3][1].unstable_flushSync).toBe(true); + router.dispose(); }); it("wraps useSubmit updates in flushSync when specified", async () => { @@ -265,6 +148,8 @@ describe("flushSync", () => { expect(spy).toBeCalledTimes(4); expect(spy.mock.calls[2][1].unstable_flushSync).toBe(true); expect(spy.mock.calls[3][1].unstable_flushSync).toBe(true); + + router.dispose(); }); it("wraps fetcher.load updates in flushSync when specified", async () => { @@ -327,6 +212,8 @@ describe("flushSync", () => { expect(spy).toBeCalledTimes(4); expect(spy.mock.calls[2][1].unstable_flushSync).toBe(true); expect(spy.mock.calls[3][1].unstable_flushSync).toBe(true); + + router.dispose(); }); it("wraps fetcher.submit updates in flushSync when specified", async () => { @@ -392,6 +279,8 @@ describe("flushSync", () => { expect(spy.mock.calls[3][1].unstable_flushSync).toBe(true); expect(spy.mock.calls[4][1].unstable_flushSync).toBe(true); expect(spy.mock.calls[5][1].unstable_flushSync).toBe(true); + + router.dispose(); }); }); diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index 462e3e771a..1f666067f7 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -862,7 +862,6 @@ export interface LinkProps preventScrollReset?: boolean; relative?: RelativeRoutingType; to: To; - unstable_flushSync?: boolean; unstable_viewTransition?: boolean; } @@ -887,7 +886,6 @@ export const Link = React.forwardRef( target, to, preventScrollReset, - unstable_flushSync, unstable_viewTransition, ...rest }, @@ -938,7 +936,6 @@ export const Link = React.forwardRef( target, preventScrollReset, relative, - unstable_flushSync, unstable_viewTransition, }); function handleClick( @@ -1142,11 +1139,6 @@ export interface FetcherFormProps * `event.preventDefault()` then this form will not do anything. */ onSubmit?: React.FormEventHandler; - - /** - * Enable flushSync navigation's staten updates - */ - unstable_flushSync?: boolean; } export interface FormProps extends FetcherFormProps { @@ -1210,7 +1202,6 @@ export const Form = React.forwardRef( onSubmit, relative, preventScrollReset, - unstable_flushSync, unstable_viewTransition, ...props }, @@ -1241,7 +1232,6 @@ export const Form = React.forwardRef( state, relative, preventScrollReset, - unstable_flushSync, unstable_viewTransition, }); }; @@ -1337,7 +1327,6 @@ export function useLinkClickHandler( state, preventScrollReset, relative, - unstable_flushSync, unstable_viewTransition, }: { target?: React.HTMLAttributeAnchorTarget; @@ -1345,7 +1334,6 @@ export function useLinkClickHandler( state?: any; preventScrollReset?: boolean; relative?: RelativeRoutingType; - unstable_flushSync?: boolean; unstable_viewTransition?: boolean; } = {} ): (event: React.MouseEvent) => void { @@ -1370,7 +1358,6 @@ export function useLinkClickHandler( state, preventScrollReset, relative, - unstable_flushSync, unstable_viewTransition, }); } @@ -1385,7 +1372,6 @@ export function useLinkClickHandler( to, preventScrollReset, relative, - unstable_flushSync, unstable_viewTransition, ] ); From f8cb919898624eb01c86a32e176a5a0309dc1f14 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 13 Nov 2023 15:42:17 -0500 Subject: [PATCH 5/8] Only apply flushSync on the first state update --- .../__tests__/flush-sync-navigations-test.tsx | 10 +- packages/router/__tests__/flush-sync-test.ts | 10 +- packages/router/router.ts | 129 +++++++++--------- 3 files changed, 78 insertions(+), 71 deletions(-) diff --git a/packages/react-router-dom/__tests__/flush-sync-navigations-test.tsx b/packages/react-router-dom/__tests__/flush-sync-navigations-test.tsx index c4cf9e2463..4d7fcff9e2 100644 --- a/packages/react-router-dom/__tests__/flush-sync-navigations-test.tsx +++ b/packages/react-router-dom/__tests__/flush-sync-navigations-test.tsx @@ -2,8 +2,6 @@ import * as React from "react"; import { RouterProvider, createBrowserRouter, - Form, - Link, useNavigate, useSubmit, useFetcher, @@ -147,7 +145,7 @@ describe("flushSync", () => { await waitFor(() => screen.getByText("Home")); expect(spy).toBeCalledTimes(4); expect(spy.mock.calls[2][1].unstable_flushSync).toBe(true); - expect(spy.mock.calls[3][1].unstable_flushSync).toBe(true); + expect(spy.mock.calls[3][1].unstable_flushSync).toBe(false); router.dispose(); }); @@ -211,7 +209,7 @@ describe("flushSync", () => { await waitFor(() => screen.getByText("sync:LOADER:idle")); expect(spy).toBeCalledTimes(4); expect(spy.mock.calls[2][1].unstable_flushSync).toBe(true); - expect(spy.mock.calls[3][1].unstable_flushSync).toBe(true); + expect(spy.mock.calls[3][1].unstable_flushSync).toBe(false); router.dispose(); }); @@ -277,8 +275,8 @@ describe("flushSync", () => { await waitFor(() => screen.getByText("sync:ACTION:idle")); expect(spy).toBeCalledTimes(6); expect(spy.mock.calls[3][1].unstable_flushSync).toBe(true); - expect(spy.mock.calls[4][1].unstable_flushSync).toBe(true); - expect(spy.mock.calls[5][1].unstable_flushSync).toBe(true); + expect(spy.mock.calls[4][1].unstable_flushSync).toBe(false); + expect(spy.mock.calls[5][1].unstable_flushSync).toBe(false); router.dispose(); }); diff --git a/packages/router/__tests__/flush-sync-test.ts b/packages/router/__tests__/flush-sync-test.ts index 9b454bd51e..deff895602 100644 --- a/packages/router/__tests__/flush-sync-test.ts +++ b/packages/router/__tests__/flush-sync-test.ts @@ -36,7 +36,7 @@ describe("flushSync", () => { await B.loaders.b.resolve("B"); expect(spy).toHaveBeenLastCalledWith( expect.anything(), - expect.objectContaining({ unstable_flushSync: true }) + expect.objectContaining({ unstable_flushSync: false }) ); unsubscribe(); @@ -80,7 +80,7 @@ describe("flushSync", () => { await B.actions.b.resolve("B"); expect(spy).toHaveBeenLastCalledWith( expect.anything(), - expect.objectContaining({ unstable_flushSync: true }) + expect.objectContaining({ unstable_flushSync: false }) ); unsubscribe(); @@ -114,6 +114,10 @@ describe("flushSync", () => { await B.loaders.root.resolve("ROOT2"); expect(t.router.state.fetchers.get(key)?.data).toBe("ROOT2"); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: false }) + ); unsubscribe(); t.router.dispose(); @@ -159,7 +163,7 @@ describe("flushSync", () => { expect(t.router.state.fetchers.get(key)?.data).toBe("ROOT2"); expect(spy).toHaveBeenLastCalledWith( expect.anything(), - expect.objectContaining({ unstable_flushSync: true }) + expect.objectContaining({ unstable_flushSync: false }) ); unsubscribe(); diff --git a/packages/router/router.ts b/packages/router/router.ts index 4f1825bbaf..7422e15492 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -411,7 +411,7 @@ export interface RouterSubscriber { opts: { deletedFetchers: string[]; unstable_viewTransitionOpts?: ViewTransitionOpts; - unstable_flushSync?: boolean; + unstable_flushSync: boolean; } ): void; } @@ -843,9 +843,6 @@ export function createRouter(init: RouterInit): Router { // Should the current navigation enable document.startViewTransition? let pendingViewTransitionEnabled = false; - // Should the current navigation use flushSync for state updates? - let pendingFlushSync = false; - // Store applied view transitions so we can apply them on POP let appliedViewTransitions: Map> = new Map< string, @@ -967,7 +964,7 @@ export function createRouter(init: RouterInit): Router { reset() { let blockers = new Map(state.blockers); blockers.set(blockerKey!, IDLE_BLOCKER); - updateState({ blockers }, { flushSync: false }); + updateState({ blockers }); }, }); return; @@ -1024,9 +1021,9 @@ export function createRouter(init: RouterInit): Router { function updateState( newState: Partial, opts: { - flushSync: boolean; + flushSync?: boolean; viewTransitionOpts?: ViewTransitionOpts; - } + } = {} ): void { state = { ...state, @@ -1060,7 +1057,7 @@ export function createRouter(init: RouterInit): Router { subscriber(state, { deletedFetchers: deletedFetchersKeys, unstable_viewTransitionOpts: opts.viewTransitionOpts, - unstable_flushSync: opts.flushSync, + unstable_flushSync: opts.flushSync === true, }) ); @@ -1078,7 +1075,8 @@ export function createRouter(init: RouterInit): Router { // - Can pass any other state in newState function completeNavigation( location: Location, - newState: Partial> + newState: Partial>, + { flushSync }: { flushSync?: boolean } = {} ): void { // Deduce if we're in a loading/actionReload state: // - We have committed actionData in the store @@ -1202,7 +1200,7 @@ export function createRouter(init: RouterInit): Router { }, { viewTransitionOpts, - flushSync: pendingFlushSync, + flushSync: flushSync === true, } ); @@ -1210,7 +1208,6 @@ export function createRouter(init: RouterInit): Router { pendingAction = HistoryAction.Pop; pendingPreventScrollReset = false; pendingViewTransitionEnabled = false; - pendingFlushSync = false; isUninterruptedRevalidation = false; isRevalidationRequired = false; cancelledDeferredRoutes = []; @@ -1308,7 +1305,7 @@ export function createRouter(init: RouterInit): Router { reset() { let blockers = new Map(state.blockers); blockers.set(blockerKey!, IDLE_BLOCKER); - updateState({ blockers }, { flushSync }); + updateState({ blockers }); }, }); return; @@ -1331,7 +1328,7 @@ export function createRouter(init: RouterInit): Router { // loaders during the next loader round function revalidate() { interruptActiveLoads(); - updateState({ revalidation: "loading" }, { flushSync: false }); + updateState({ revalidation: "loading" }); // If we're currently submitting an action, we don't need to start a new // navigation, we'll just let the follow up loader execution call all loaders @@ -1392,11 +1389,11 @@ export function createRouter(init: RouterInit): Router { pendingPreventScrollReset = (opts && opts.preventScrollReset) === true; pendingViewTransitionEnabled = (opts && opts.enableViewTransition) === true; - pendingFlushSync = (opts && opts.flushSync) === true; let routesToUse = inFlightDataRoutes || dataRoutes; let loadingNavigation = opts && opts.overrideNavigation; let matches = matchRoutes(routesToUse, location, basename); + let flushSync = (opts && opts.flushSync) === true; // Short circuit with a 404 on the root error boundary if we match nothing if (!matches) { @@ -1405,13 +1402,17 @@ export function createRouter(init: RouterInit): Router { getShortCircuitMatches(routesToUse); // Cancel all pending deferred on 404s since we don't keep any routes cancelActiveDeferreds(); - completeNavigation(location, { - matches: notFoundMatches, - loaderData: {}, - errors: { - [route.id]: error, + completeNavigation( + location, + { + matches: notFoundMatches, + loaderData: {}, + errors: { + [route.id]: error, + }, }, - }); + { flushSync } + ); return; } @@ -1427,7 +1428,7 @@ export function createRouter(init: RouterInit): Router { isHashChangeOnly(state.location, location) && !(opts && opts.submission && isMutationMethod(opts.submission.formMethod)) ) { - completeNavigation(location, { matches }); + completeNavigation(location, { matches }, { flushSync }); return; } @@ -1461,7 +1462,7 @@ export function createRouter(init: RouterInit): Router { location, opts.submission, matches, - { replace: opts.replace } + { replace: opts.replace, flushSync } ); if (actionOutput.shortCircuited) { @@ -1471,6 +1472,7 @@ export function createRouter(init: RouterInit): Router { pendingActionData = actionOutput.pendingActionData; pendingError = actionOutput.pendingActionError; loadingNavigation = getLoadingNavigation(location, opts.submission); + flushSync = false; // Create a GET request for the loaders request = new Request(request.url, { signal: request.signal }); @@ -1485,6 +1487,7 @@ export function createRouter(init: RouterInit): Router { opts && opts.submission, opts && opts.fetcherSubmission, opts && opts.replace, + flushSync, pendingActionData, pendingError ); @@ -1513,13 +1516,13 @@ export function createRouter(init: RouterInit): Router { location: Location, submission: Submission, matches: AgnosticDataRouteMatch[], - opts: { replace?: boolean } = {} + opts: { replace?: boolean; flushSync?: boolean } = {} ): Promise { interruptActiveLoads(); // Put us in a submitting state let navigation = getSubmittingNavigation(location, submission); - updateState({ navigation }, { flushSync: pendingFlushSync }); + updateState({ navigation }, { flushSync: opts.flushSync === true }); // Call our action and get the result let result: DataResult; @@ -1604,6 +1607,7 @@ export function createRouter(init: RouterInit): Router { submission?: Submission, fetcherSubmission?: Submission, replace?: boolean, + flushSync?: boolean, pendingActionData?: RouteData, pendingError?: RouteData ): Promise { @@ -1650,14 +1654,18 @@ export function createRouter(init: RouterInit): Router { // Short circuit if we have no loaders to run if (matchesToLoad.length === 0 && revalidatingFetchers.length === 0) { let updatedFetchers = markFetchRedirectsDone(); - completeNavigation(location, { - matches, - loaderData: {}, - // Commit pending error if we're short circuiting - errors: pendingError || null, - ...(pendingActionData ? { actionData: pendingActionData } : {}), - ...(updatedFetchers ? { fetchers: new Map(state.fetchers) } : {}), - }); + completeNavigation( + location, + { + matches, + loaderData: {}, + // Commit pending error if we're short circuiting + errors: pendingError || null, + ...(pendingActionData ? { actionData: pendingActionData } : {}), + ...(updatedFetchers ? { fetchers: new Map(state.fetchers) } : {}), + }, + { flushSync } + ); return { shortCircuited: true }; } @@ -1688,7 +1696,7 @@ export function createRouter(init: RouterInit): Router { : {}), }, { - flushSync: pendingFlushSync, + flushSync, } ); } @@ -1934,7 +1942,7 @@ export function createRouter(init: RouterInit): Router { } if (deletedFetchers.has(key)) { - updateFetcherState(key, getDoneFetcher(undefined), { flushSync }); + updateFetcherState(key, getDoneFetcher(undefined)); return; } @@ -1945,11 +1953,11 @@ export function createRouter(init: RouterInit): Router { // should take precedence over this redirect navigation. We already // set isRevalidationRequired so all loaders for the new route should // fire unless opted out via shouldRevalidate - updateFetcherState(key, getDoneFetcher(undefined), { flushSync }); + updateFetcherState(key, getDoneFetcher(undefined)); return; } else { fetchRedirectIds.add(key); - updateFetcherState(key, getLoadingFetcher(submission), { flushSync }); + updateFetcherState(key, getLoadingFetcher(submission)); return startRedirectNavigation(state, actionResult, { fetcherSubmission: submission, }); @@ -1958,7 +1966,7 @@ export function createRouter(init: RouterInit): Router { // Process any non-redirect errors thrown if (isErrorResult(actionResult)) { - setFetcherError(key, routeId, actionResult.error, { flushSync }); + setFetcherError(key, routeId, actionResult.error); return; } @@ -2026,7 +2034,7 @@ export function createRouter(init: RouterInit): Router { } }); - updateState({ fetchers: new Map(state.fetchers) }, { flushSync }); + updateState({ fetchers: new Map(state.fetchers) }); let abortPendingFetchRevalidations = () => revalidatingFetchers.forEach((rf) => abortFetcher(rf.key)); @@ -2112,19 +2120,16 @@ export function createRouter(init: RouterInit): Router { // otherwise just update with the fetcher data, preserving any existing // loaderData for loaders that did not need to reload. We have to // manually merge here since we aren't going through completeNavigation - updateState( - { - errors, - loaderData: mergeLoaderData( - state.loaderData, - loaderData, - matches, - errors - ), - fetchers: new Map(state.fetchers), - }, - { flushSync } - ); + updateState({ + errors, + loaderData: mergeLoaderData( + state.loaderData, + loaderData, + matches, + errors + ), + fetchers: new Map(state.fetchers), + }); isRevalidationRequired = false; } } @@ -2190,7 +2195,7 @@ export function createRouter(init: RouterInit): Router { } if (deletedFetchers.has(key)) { - updateFetcherState(key, getDoneFetcher(undefined), { flushSync }); + updateFetcherState(key, getDoneFetcher(undefined)); return; } @@ -2199,7 +2204,7 @@ export function createRouter(init: RouterInit): Router { if (pendingNavigationLoadId > originatingLoadId) { // A new navigation was kicked off after our loader started, so that // should take precedence over this redirect navigation - updateFetcherState(key, getDoneFetcher(undefined), { flushSync }); + updateFetcherState(key, getDoneFetcher(undefined)); return; } else { fetchRedirectIds.add(key); @@ -2210,14 +2215,14 @@ export function createRouter(init: RouterInit): Router { // Process any non-redirect errors thrown if (isErrorResult(result)) { - setFetcherError(key, routeId, result.error, { flushSync }); + setFetcherError(key, routeId, result.error); return; } invariant(!isDeferredResult(result), "Unhandled fetcher deferred data"); // Put the fetcher back into an idle state - updateFetcherState(key, getDoneFetcher(result.data), { flushSync }); + updateFetcherState(key, getDoneFetcher(result.data)); } /** @@ -2429,12 +2434,12 @@ export function createRouter(init: RouterInit): Router { function updateFetcherState( key: string, fetcher: Fetcher, - opts: { flushSync: boolean } + opts: { flushSync?: boolean } = {} ) { state.fetchers.set(key, fetcher); updateState( { fetchers: new Map(state.fetchers) }, - { flushSync: opts.flushSync } + { flushSync: (opts && opts.flushSync) === true } ); } @@ -2442,7 +2447,7 @@ export function createRouter(init: RouterInit): Router { key: string, routeId: string, error: any, - opts: { flushSync: boolean } + opts: { flushSync?: boolean } = {} ) { let boundaryMatch = findNearestBoundary(state.matches, routeId); deleteFetcher(key); @@ -2453,7 +2458,7 @@ export function createRouter(init: RouterInit): Router { }, fetchers: new Map(state.fetchers), }, - { flushSync: opts.flushSync } + { flushSync: (opts && opts.flushSync) === true } ); } @@ -2499,7 +2504,7 @@ export function createRouter(init: RouterInit): Router { } else { deleteFetcher(key); } - updateState({ fetchers: new Map(state.fetchers) }, { flushSync: false }); + updateState({ fetchers: new Map(state.fetchers) }); } function abortFetcher(key: string) { @@ -2582,7 +2587,7 @@ export function createRouter(init: RouterInit): Router { let blockers = new Map(state.blockers); blockers.set(key, newBlocker); - updateState({ blockers }, { flushSync: false }); + updateState({ blockers }); } function shouldBlockNavigation({ @@ -2656,7 +2661,7 @@ export function createRouter(init: RouterInit): Router { initialScrollRestored = true; let y = getSavedScrollPosition(state.location, state.matches); if (y != null) { - updateState({ restoreScrollPosition: y }, { flushSync: false }); + updateState({ restoreScrollPosition: y }); } } From 93d41a261c7b40f02d75f89ded89123252778ba7 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 13 Nov 2023 15:45:23 -0500 Subject: [PATCH 6/8] Lower bundle --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index a90d6fc12d..0709215874 100644 --- a/package.json +++ b/package.json @@ -119,10 +119,10 @@ "none": "16.3 kB" }, "packages/react-router-dom/dist/react-router-dom.production.min.js": { - "none": "16.8 kB" + "none": "16.7 kB" }, "packages/react-router-dom/dist/umd/react-router-dom.production.min.js": { - "none": "23 kB" + "none": "22.9 kB" } } } From 8a224b4e78997c622dd8441bab0aef9aab15e981 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 13 Nov 2023 16:04:45 -0500 Subject: [PATCH 7/8] Update docs --- docs/hooks/use-fetcher.md | 10 +++++++++- docs/hooks/use-navigate.md | 10 ++++++++++ docs/hooks/use-submit.md | 8 ++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/docs/hooks/use-fetcher.md b/docs/hooks/use-fetcher.md index 3d4df47c19..d9beb2da0e 100644 --- a/docs/hooks/use-fetcher.md +++ b/docs/hooks/use-fetcher.md @@ -107,7 +107,7 @@ function SomeComponent() { ## Methods -## `fetcher.load()` +## `fetcher.load(href, options)` Loads data from a route loader. @@ -133,6 +133,12 @@ If you find yourself calling this function inside of click handlers, you can pro Any `fetcher.load` calls that are active on the page will be re-executed as part of revalidation (either after a navigation submission, another fetcher submission, or a `useRevalidator()` call) +### `options.unstable_flushSync` + +The `unstable_flushSync` option tells React Router DOM to wrap the initial state update for this `fetcher.load` in a [`ReactDOM.flushSync`][flush-sync] call instead of the default [`React.startTransition`][start-transition]. This allows you to perform synchronous DOM actions immediately after the update is flushed to the DOM. + +`ReactDOM.flushSync` de-optimizes React and can hurt the performance of your app. + ## `fetcher.submit()` The imperative version of ``. If a user interaction should initiate the fetch, you should use ``. But if you, the programmer are initiating the fetch (not in response to a user clicking a button, etc.), then use this function. @@ -279,3 +285,5 @@ fetcher.formMethod; // "post" [api-development-strategy]: ../guides/api-development-strategy [use-submit]: ./use-submit [use-fetchers]: ./use-fetchers +[flush-sync]: https://react.dev/reference/react-dom/flushSync +[start-transition]: https://react.dev/reference/react/startTransition diff --git a/docs/hooks/use-navigate.md b/docs/hooks/use-navigate.md index 3f4334757e..150181a541 100644 --- a/docs/hooks/use-navigate.md +++ b/docs/hooks/use-navigate.md @@ -20,6 +20,8 @@ interface NavigateOptions { state?: any; preventScrollReset?: boolean; relative?: RelativeRoutingType; + unstable_flushSync?: boolean; + unstable_viewTransition?: boolean; } type RelativeRoutingType = "route" | "path"; @@ -85,6 +87,12 @@ function EditContact() { } ``` +## `options.unstable_flushSync` + +The `unstable_flushSync` option tells React Router DOM to wrap the initial state update for this navigation in a [`ReactDOM.flushSync`][flush-sync] call instead of the default [`React.startTransition`][start-transition]. This allows you to perform synchronous DOM actions immediately after the update is flushed to the DOM. + +`ReactDOM.flushSync` de-optimizes React and can hurt the performance of your app. + ## `options.unstable_viewTransition` The `unstable_viewTransition` option enables a [View Transition][view-transitions] for this navigation by wrapping the final state update in `document.startViewTransition()`. If you need to apply specific styles for this view transition, you will also need to leverage the [`unstable_useViewTransitionState()`][use-view-transition-state]. @@ -102,3 +110,5 @@ The `unstable_viewTransition` option enables a [View Transition][view-transition [use-view-transition-state]: ../hooks//use-view-transition-state [view-transitions]: https://developer.mozilla.org/en-US/docs/Web/API/View_Transitions_API [picking-a-router]: ../routers/picking-a-router +[flush-sync]: https://react.dev/reference/react-dom/flushSync +[start-transition]: https://react.dev/reference/react/startTransition diff --git a/docs/hooks/use-submit.md b/docs/hooks/use-submit.md index 566745bf53..c11b8c3bf3 100644 --- a/docs/hooks/use-submit.md +++ b/docs/hooks/use-submit.md @@ -160,5 +160,13 @@ Because submissions are navigations, the options may also contain the other navi - `state` - `unstable_viewTransition` +### `options.unstable_flushSync` + +The `unstable_flushSync` option tells React Router DOM to wrap the initial state update for this submission in a [`ReactDOM.flushSync`][flush-sync] call instead of the default [`React.startTransition`][start-transition]. This allows you to perform synchronous DOM actions immediately after the update is flushed to the DOM. + +`ReactDOM.flushSync` de-optimizes React and can hurt the performance of your app. + [pickingarouter]: ../routers/picking-a-router [form]: ../components/form +[flush-sync]: https://react.dev/reference/react-dom/flushSync +[start-transition]: https://react.dev/reference/react/startTransition From 3ed602e14971d197ffed4127ab9a3e3e7bbc9012 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 13 Nov 2023 16:38:28 -0500 Subject: [PATCH 8/8] Docs updates --- docs/hooks/use-fetcher.md | 8 +++++--- docs/hooks/use-navigate.md | 4 ++++ docs/hooks/use-submit.md | 2 ++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/docs/hooks/use-fetcher.md b/docs/hooks/use-fetcher.md index d9beb2da0e..0d5fbb3685 100644 --- a/docs/hooks/use-fetcher.md +++ b/docs/hooks/use-fetcher.md @@ -107,7 +107,7 @@ function SomeComponent() { ## Methods -## `fetcher.load(href, options)` +### `fetcher.load(href, options)` Loads data from a route loader. @@ -133,13 +133,15 @@ If you find yourself calling this function inside of click handlers, you can pro Any `fetcher.load` calls that are active on the page will be re-executed as part of revalidation (either after a navigation submission, another fetcher submission, or a `useRevalidator()` call) -### `options.unstable_flushSync` +#### `options.unstable_flushSync` The `unstable_flushSync` option tells React Router DOM to wrap the initial state update for this `fetcher.load` in a [`ReactDOM.flushSync`][flush-sync] call instead of the default [`React.startTransition`][start-transition]. This allows you to perform synchronous DOM actions immediately after the update is flushed to the DOM. `ReactDOM.flushSync` de-optimizes React and can hurt the performance of your app. -## `fetcher.submit()` +Please note that this API is marked unstable and may be subject to breaking changes without a major release + +### `fetcher.submit()` The imperative version of ``. If a user interaction should initiate the fetch, you should use ``. But if you, the programmer are initiating the fetch (not in response to a user clicking a button, etc.), then use this function. diff --git a/docs/hooks/use-navigate.md b/docs/hooks/use-navigate.md index 150181a541..10ac05ced9 100644 --- a/docs/hooks/use-navigate.md +++ b/docs/hooks/use-navigate.md @@ -91,8 +91,12 @@ function EditContact() { The `unstable_flushSync` option tells React Router DOM to wrap the initial state update for this navigation in a [`ReactDOM.flushSync`][flush-sync] call instead of the default [`React.startTransition`][start-transition]. This allows you to perform synchronous DOM actions immediately after the update is flushed to the DOM. +`unstable_flushSync` only works when using a data router, see [Picking a Router][picking-a-router] + `ReactDOM.flushSync` de-optimizes React and can hurt the performance of your app. +Please note that this API is marked unstable and may be subject to breaking changes without a major release + ## `options.unstable_viewTransition` The `unstable_viewTransition` option enables a [View Transition][view-transitions] for this navigation by wrapping the final state update in `document.startViewTransition()`. If you need to apply specific styles for this view transition, you will also need to leverage the [`unstable_useViewTransitionState()`][use-view-transition-state]. diff --git a/docs/hooks/use-submit.md b/docs/hooks/use-submit.md index c11b8c3bf3..6d738de47a 100644 --- a/docs/hooks/use-submit.md +++ b/docs/hooks/use-submit.md @@ -166,6 +166,8 @@ The `unstable_flushSync` option tells React Router DOM to wrap the initial state `ReactDOM.flushSync` de-optimizes React and can hurt the performance of your app. +Please note that this API is marked unstable and may be subject to breaking changes without a major release + [pickingarouter]: ../routers/picking-a-router [form]: ../components/form [flush-sync]: https://react.dev/reference/react-dom/flushSync