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..ee8253c589 --- /dev/null +++ b/.changeset/flush-sync.md @@ -0,0 +1,6 @@ +--- +"react-router-dom": minor +"react-router": minor +--- + +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/docs/hooks/use-fetcher.md b/docs/hooks/use-fetcher.md index 3d4df47c19..0d5fbb3685 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,7 +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) -## `fetcher.submit()` +#### `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. + +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. @@ -279,3 +287,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..10ac05ced9 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,16 @@ 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. + +`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]. @@ -102,3 +114,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..6d738de47a 100644 --- a/docs/hooks/use-submit.md +++ b/docs/hooks/use-submit.md @@ -160,5 +160,15 @@ 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. + +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 +[start-transition]: https://react.dev/reference/react/startTransition diff --git a/package.json b/package.json index 1b12bdb1f8..0709215874 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.7 kB" }, "packages/react-router-dom/dist/umd/react-router-dom.production.min.js": { - "none": "22.3 kB" + "none": "22.9 kB" } } } 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..4d7fcff9e2 --- /dev/null +++ b/packages/react-router-dom/__tests__/flush-sync-navigations-test.tsx @@ -0,0 +1,290 @@ +import * as React from "react"; +import { + RouterProvider, + createBrowserRouter, + useNavigate, + useSubmit, + useFetcher, +} from "react-router-dom"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { JSDOM } from "jsdom"; + +describe("flushSync", () => { + 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); + + router.dispose(); + }); + + 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(false); + + router.dispose(); + }); + + 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(false); + + router.dispose(); + }); + + 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(false); + expect(spy.mock.calls[5][1].unstable_flushSync).toBe(false); + + router.dispose(); + }); +}); + +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..1f666067f7 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, }); @@ -916,7 +979,6 @@ export interface NavLinkProps style?: | React.CSSProperties | ((props: NavLinkRenderProps) => React.CSSProperties | undefined); - unstable_viewTransition?: boolean; } /** @@ -1444,6 +1506,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 +1518,7 @@ export function useSubmit(): SubmitFunction { replace: options.replace, state: options.state, fromRouteId: currentRouteId, + unstable_flushSync: options.unstable_flushSync, unstable_viewTransition: options.unstable_viewTransition, }); } @@ -1520,7 +1584,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` @@ -1564,9 +1628,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..deff895602 --- /dev/null +++ b/packages/router/__tests__/flush-sync-test.ts @@ -0,0 +1,172 @@ +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: false }) + ); + + 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: false }) + ); + + 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"); + expect(spy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ unstable_flushSync: false }) + ); + + 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: false }) + ); + + 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..7422e15492 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 @@ -1018,7 +1020,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 +1050,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 === true, }) ); @@ -1066,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 @@ -1188,7 +1198,10 @@ export function createRouter(init: RouterInit): Router { preventScrollReset, blockers, }, - viewTransitionOpts + { + viewTransitionOpts, + flushSync: flushSync === true, + } ); // Reset stateful navigation vars @@ -1266,6 +1279,8 @@ export function createRouter(init: RouterInit): Router { ? opts.preventScrollReset === true : undefined; + let flushSync = (opts && opts.unstable_flushSync) === true; + let blockerKey = shouldBlockNavigation({ currentLocation, nextLocation, @@ -1304,6 +1319,7 @@ export function createRouter(init: RouterInit): Router { preventScrollReset, replace: opts && opts.replace, enableViewTransition: opts && opts.unstable_viewTransition, + flushSync, }); } @@ -1355,6 +1371,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 @@ -1376,6 +1393,7 @@ export function createRouter(init: RouterInit): Router { 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) { @@ -1384,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; } @@ -1406,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; } @@ -1440,7 +1462,7 @@ export function createRouter(init: RouterInit): Router { location, opts.submission, matches, - { replace: opts.replace } + { replace: opts.replace, flushSync } ); if (actionOutput.shortCircuited) { @@ -1450,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 }); @@ -1464,6 +1487,7 @@ export function createRouter(init: RouterInit): Router { opts && opts.submission, opts && opts.fetcherSubmission, opts && opts.replace, + flushSync, pendingActionData, pendingError ); @@ -1492,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 }); + updateState({ navigation }, { flushSync: opts.flushSync === true }); // Call our action and get the result let result: DataResult; @@ -1583,6 +1607,7 @@ export function createRouter(init: RouterInit): Router { submission?: Submission, fetcherSubmission?: Submission, replace?: boolean, + flushSync?: boolean, pendingActionData?: RouteData, pendingError?: RouteData ): Promise { @@ -1629,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 }; } @@ -1654,17 +1683,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, + } + ); } revalidatingFetchers.forEach((rf) => { @@ -1764,18 +1798,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 +1814,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 +1832,8 @@ export function createRouter(init: RouterInit): Router { setFetcherError( key, routeId, - getInternalRouterError(404, { pathname: normalizedPath }) + getInternalRouterError(404, { pathname: normalizedPath }), + { flushSync } ); return; } @@ -1822,7 +1846,7 @@ export function createRouter(init: RouterInit): Router { ); if (error) { - setFetcherError(key, routeId, error); + setFetcherError(key, routeId, error, { flushSync }); return; } @@ -1831,14 +1855,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 +1889,7 @@ export function createRouter(init: RouterInit): Router { path: string, match: AgnosticDataRouteMatch, requestMatches: AgnosticDataRouteMatch[], + flushSync: boolean, submission: Submission ) { interruptActiveLoads(); @@ -1860,15 +1901,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 +1942,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)); return; } @@ -1913,16 +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 - let doneFetcher = getDoneFetcher(undefined); - state.fetchers.set(key, doneFetcher); - updateState({ fetchers: new Map(state.fetchers) }); + updateFetcherState(key, getDoneFetcher(undefined)); return; } else { fetchRedirectIds.add(key); - let loadingFetcher = getLoadingFetcher(submission); - state.fetchers.set(key, loadingFetcher); - updateState({ fetchers: new Map(state.fetchers) }); - + updateFetcherState(key, getLoadingFetcher(submission)); return startRedirectNavigation(state, actionResult, { fetcherSubmission: submission, }); @@ -2106,16 +2141,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 +2195,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)); return; } @@ -2168,9 +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 - let doneFetcher = getDoneFetcher(undefined); - state.fetchers.set(key, doneFetcher); - updateState({ fetchers: new Map(state.fetchers) }); + updateFetcherState(key, getDoneFetcher(undefined)); return; } else { fetchRedirectIds.add(key); @@ -2188,9 +2222,7 @@ export function createRouter(init: RouterInit): Router { 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)); } /** @@ -2399,15 +2431,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 && opts.flushSync) === true } + ); + } + + 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 && opts.flushSync) === true } + ); + } + + 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 {