diff --git a/.changeset/v7-start-transition.md b/.changeset/v7-start-transition.md new file mode 100644 index 0000000000..22a204b4f5 --- /dev/null +++ b/.changeset/v7-start-transition.md @@ -0,0 +1,26 @@ +--- +"react-router": minor +"react-router-dom": minor +--- + +Move [`React.startTransition`](https://react.dev/reference/react/startTransition) behind a [future flag](https://reactrouter.com/en/main/guides/api-development-strategy) to avoid issues with existing incompatible `Suspense` usages. We recommend folks adopting this flag to be better compatible with React concurrent mode, but if you run into issues you can continue without the use of `startTransition` until v7. Issues usually boils down to creating net-new promises during the render cycle, so if you run into issues you should either lift your promise creation out of the render cycle or put it behind a `useMemo`. + +Existing behavior will no longer include `React.startTransition`: + +```jsx + + {/*...*/} + + + +``` + +If you wish to enable `React.startTransition`, pass the future flag to your component: + +```jsx + + {/*...*/} + + + +``` diff --git a/docs/guides/api-development-strategy.md b/docs/guides/api-development-strategy.md index 51655a7156..aca10bc852 100644 --- a/docs/guides/api-development-strategy.md +++ b/docs/guides/api-development-strategy.md @@ -49,12 +49,46 @@ The lifecycle is thus either: ## Current Future Flags -Here's the current future flags in React Router v6 today: +Here's the current future flags in React Router v6 today. + +### `@remix-run/router` Future Flags + +These flags are only applicable when using a [Data Router][picking-a-router] and are passed when creating the `router` instance: + +```js +const router = createBrowserRouter(routes, { + future: { + v7_normalizeFormMethod: true, + }, +}); +``` | Flag | Description | | ------------------------ | --------------------------------------------------------------------- | | `v7_normalizeFormMethod` | Normalize `useNavigation().formMethod` to be an uppercase HTTP Method | +### React Router Future Flags + +These flags apply to both Data and non-Data Routers and are passed to the rendered React component: + +```jsx + + {/*...*/} + +``` + +```jsx + +``` + +| Flag | Description | +| -------------------- | --------------------------------------------------------------------------- | +| `v7_startTransition` | Wrap all router state updates in [`React.startTransition`][starttransition] | + [future-flags-blog-post]: https://remix.run/blog/future-flags [feature-flowchart]: https://remix.run/docs-images/feature-flowchart.png [picking-a-router]: ../routers/picking-a-router +[starttransition]: https://react.dev/reference/react/startTransition diff --git a/package.json b/package.json index 18618426b5..6c75c12ad1 100644 --- a/package.json +++ b/package.json @@ -112,16 +112,16 @@ "none": "45 kB" }, "packages/react-router/dist/react-router.production.min.js": { - "none": "13.4 kB" + "none": "13.5 kB" }, "packages/react-router/dist/umd/react-router.production.min.js": { "none": "15.8 kB" }, "packages/react-router-dom/dist/react-router-dom.production.min.js": { - "none": "12.0 kB" + "none": "12.1 kB" }, "packages/react-router-dom/dist/umd/react-router-dom.production.min.js": { - "none": "18.0 kB" + "none": "18.1 kB" } } } diff --git a/packages/react-router-dom/__tests__/concurrent-mode-navigations-test.tsx b/packages/react-router-dom/__tests__/concurrent-mode-navigations-test.tsx index 6e7839a012..5f494ffcd0 100644 --- a/packages/react-router-dom/__tests__/concurrent-mode-navigations-test.tsx +++ b/packages/react-router-dom/__tests__/concurrent-mode-navigations-test.tsx @@ -19,7 +19,6 @@ import { waitFor, } from "@testing-library/react"; import { JSDOM } from "jsdom"; -import LazyComponent from "./components//LazyComponent"; describe("Handles concurrent mode features during navigations", () => { function getComponents() { @@ -117,7 +116,7 @@ describe("Handles concurrent mode features during navigations", () => { getComponents(); let { container } = render( - + } /> { getComponents(); let { container } = render( - + } /> { getComponents(); let { container } = render( - + } /> { ) ); - let { container } = render(); + let { container } = render( + + ); await assertNavigation(container, resolve, resolveLazy); }); @@ -288,7 +295,7 @@ describe("Handles concurrent mode features during navigations", () => { getComponents(); let { container } = render( - + } /> } /> @@ -306,7 +313,10 @@ describe("Handles concurrent mode features during navigations", () => { getComponents(); let { container } = render( - + } /> } /> @@ -324,7 +334,10 @@ describe("Handles concurrent mode features during navigations", () => { getComponents(); let { container } = render( - + } /> } /> @@ -350,7 +363,9 @@ describe("Handles concurrent mode features during navigations", () => { ) ); - let { container } = render(); + let { container } = render( + + ); await assertNavigation(container, resolve, resolveLazy); }); diff --git a/packages/react-router-dom/__tests__/exports-test.tsx b/packages/react-router-dom/__tests__/exports-test.tsx index e3b479d6f1..7e87c75a81 100644 --- a/packages/react-router-dom/__tests__/exports-test.tsx +++ b/packages/react-router-dom/__tests__/exports-test.tsx @@ -4,6 +4,7 @@ import * as ReactRouterDOM from "react-router-dom"; let nonReExportedKeys = new Set([ "UNSAFE_mapRouteProperties", "UNSAFE_useRoutesImpl", + "UNSAFE_startTransitionImpl", ]); describe("react-router-dom", () => { diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index d2356a84f0..98bc1f7e8c 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -4,6 +4,7 @@ */ import * as React from "react"; import type { + FutureConfig, Location, NavigateOptions, NavigationType, @@ -26,6 +27,7 @@ import { UNSAFE_NavigationContext as NavigationContext, UNSAFE_RouteContext as RouteContext, UNSAFE_mapRouteProperties as mapRouteProperties, + UNSAFE_startTransitionImpl as startTransitionImpl, UNSAFE_useRouteId as useRouteId, } from "react-router"; import type { @@ -33,7 +35,7 @@ import type { Fetcher, FormEncType, FormMethod, - FutureConfig, + FutureConfig as RouterFutureConfig, GetScrollRestorationKeyFunction, HashHistory, History, @@ -209,7 +211,7 @@ declare global { interface DOMRouterOpts { basename?: string; - future?: Partial>; + future?: Partial>; hydrationData?: HydrationState; window?: Window; } @@ -297,34 +299,17 @@ function deserializeErrors( export interface BrowserRouterProps { basename?: string; children?: React.ReactNode; + future?: FutureConfig; window?: Window; } -// Webpack + React 17 fails to compile on any of the following: -// * import { startTransition } from "react" -// * import * as React from from "react"; -// "startTransition" in React ? React.startTransition(() => setState()) : setState() -// * import * as React from from "react"; -// "startTransition" in React ? React["startTransition"](() => setState()) : setState() -// -// Moving it to a constant such as the following solves the Webpack/React 17 issue: -// * import * as React from from "react"; -// const START_TRANSITION = "startTransition"; -// START_TRANSITION in React ? React[START_TRANSITION](() => setState()) : setState() -// -// However, that introduces webpack/terser minification issues in production builds -// in React 18 where minification/obfuscation ends up removing the call of -// React.startTransition entirely from the first half of the ternary. Grabbing -// this reference once up front resolves that issue. -const START_TRANSITION = "startTransition"; -const startTransitionImpl = React[START_TRANSITION]; - /** * A `` for use in web browsers. Provides the cleanest URLs. */ export function BrowserRouter({ basename, children, + future, window, }: BrowserRouterProps) { let historyRef = React.useRef(); @@ -337,13 +322,14 @@ export function BrowserRouter({ action: history.action, location: history.location, }); + let { v7_startTransition } = future || {}; let setState = React.useCallback( (newState: { action: NavigationType; location: Location }) => { - startTransitionImpl + v7_startTransition && startTransitionImpl ? startTransitionImpl(() => setStateImpl(newState)) : setStateImpl(newState); }, - [setStateImpl] + [setStateImpl, v7_startTransition] ); React.useLayoutEffect(() => history.listen(setState), [history, setState]); @@ -362,6 +348,7 @@ export function BrowserRouter({ export interface HashRouterProps { basename?: string; children?: React.ReactNode; + future?: FutureConfig; window?: Window; } @@ -369,7 +356,12 @@ export interface HashRouterProps { * A `` for use in web browsers. Stores the location in the hash * portion of the URL so it is not sent to the server. */ -export function HashRouter({ basename, children, window }: HashRouterProps) { +export function HashRouter({ + basename, + children, + future, + window, +}: HashRouterProps) { let historyRef = React.useRef(); if (historyRef.current == null) { historyRef.current = createHashHistory({ window, v5Compat: true }); @@ -380,13 +372,14 @@ export function HashRouter({ basename, children, window }: HashRouterProps) { action: history.action, location: history.location, }); + let { v7_startTransition } = future || {}; let setState = React.useCallback( (newState: { action: NavigationType; location: Location }) => { - startTransitionImpl + v7_startTransition && startTransitionImpl ? startTransitionImpl(() => setStateImpl(newState)) : setStateImpl(newState); }, - [setStateImpl] + [setStateImpl, v7_startTransition] ); React.useLayoutEffect(() => history.listen(setState), [history, setState]); @@ -405,6 +398,7 @@ export function HashRouter({ basename, children, window }: HashRouterProps) { export interface HistoryRouterProps { basename?: string; children?: React.ReactNode; + future?: FutureConfig; history: History; } @@ -414,18 +408,24 @@ export interface HistoryRouterProps { * two versions of the history library to your bundles unless you use the same * version of the history library that React Router uses internally. */ -function HistoryRouter({ basename, children, history }: HistoryRouterProps) { +function HistoryRouter({ + basename, + children, + future, + history, +}: HistoryRouterProps) { let [state, setStateImpl] = React.useState({ action: history.action, location: history.location, }); + let { v7_startTransition } = future || {}; let setState = React.useCallback( (newState: { action: NavigationType; location: Location }) => { - startTransitionImpl + v7_startTransition && startTransitionImpl ? startTransitionImpl(() => setStateImpl(newState)) : setStateImpl(newState); }, - [setStateImpl] + [setStateImpl, v7_startTransition] ); React.useLayoutEffect(() => history.listen(setState), [history, setState]); diff --git a/packages/react-router-native/__tests__/exports-test.tsx b/packages/react-router-native/__tests__/exports-test.tsx index a73c1e42ba..ab49f423d6 100644 --- a/packages/react-router-native/__tests__/exports-test.tsx +++ b/packages/react-router-native/__tests__/exports-test.tsx @@ -4,6 +4,7 @@ import * as ReactRouterNative from "react-router-native"; let nonReExportedKeys = new Set([ "UNSAFE_mapRouteProperties", "UNSAFE_useRoutesImpl", + "UNSAFE_startTransitionImpl", ]); describe("react-router-native", () => { diff --git a/packages/react-router/__tests__/navigate-test.tsx b/packages/react-router/__tests__/navigate-test.tsx index 7c12862284..8a3c03615f 100644 --- a/packages/react-router/__tests__/navigate-test.tsx +++ b/packages/react-router/__tests__/navigate-test.tsx @@ -564,51 +564,54 @@ describe("", () => { " `); }); +}); - it("handles setState in render in StrictMode using a data router (sync loader)", async () => { - let renders: number[] = []; - const router = createMemoryRouter([ - { - path: "/", - children: [ - { - index: true, - Component() { - let [count, setCount] = React.useState(0); - if (count === 0) { - setCount(1); - } - return ; +describe("concurrent mode", () => { + describe("v7_startTransition = false", () => { + it("handles setState in render in StrictMode using a data router (sync loader)", async () => { + let renders: number[] = []; + const router = createMemoryRouter([ + { + path: "/", + children: [ + { + index: true, + Component() { + let [count, setCount] = React.useState(0); + if (count === 0) { + setCount(1); + } + return ; + }, }, - }, - { - path: "b", - Component() { - let { state } = useLocation() as { state: { count: number } }; - renders.push(state.count); - return ( - <> -

Page B

-

{state.count}

- - ); + { + path: "b", + Component() { + let { state } = useLocation() as { state: { count: number } }; + renders.push(state.count); + return ( + <> +

Page B

+

{state.count}

+ + ); + }, }, - }, - ], - }, - ]); + ], + }, + ]); - let navigateSpy = jest.spyOn(router, "navigate"); + let navigateSpy = jest.spyOn(router, "navigate"); - let { container } = render( - - - - ); + let { container } = render( + + + + ); - await waitFor(() => screen.getByText("Page B")); + await waitFor(() => screen.getByText("Page B")); - expect(getHtml(container)).toMatchInlineSnapshot(` + expect(getHtml(container)).toMatchInlineSnapshot(` "

Page B @@ -618,64 +621,133 @@ describe("", () => {

" `); - expect(navigateSpy).toHaveBeenCalledTimes(2); - expect(navigateSpy.mock.calls[0]).toMatchObject([ - { pathname: "/b" }, - { state: { count: 1 } }, - ]); - expect(navigateSpy.mock.calls[1]).toMatchObject([ - { pathname: "/b" }, - { state: { count: 1 } }, - ]); - expect(renders).toEqual([1, 1]); - }); + expect(navigateSpy).toHaveBeenCalledTimes(2); + expect(navigateSpy.mock.calls[0]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 1 } }, + ]); + expect(navigateSpy.mock.calls[1]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 1 } }, + ]); + expect(renders).toEqual([1, 1]); + }); - it("handles setState in effect in StrictMode using a data router (sync loader)", async () => { - let renders: number[] = []; - const router = createMemoryRouter([ - { - path: "/", - children: [ - { - index: true, - Component() { - let [count, setCount] = React.useState(0); - React.useEffect(() => { + it("handles setState in effect in StrictMode using a data router (sync loader)", async () => { + let renders: number[] = []; + const router = createMemoryRouter([ + { + path: "/", + children: [ + { + index: true, + Component() { + let [count, setCount] = React.useState(0); + React.useEffect(() => { + if (count === 0) { + setCount(1); + } + }, [count]); + return ; + }, + }, + { + path: "b", + Component() { + let { state } = useLocation() as { state: { count: number } }; + renders.push(state.count); + return ( + <> +

Page B

+

{state.count}

+ + ); + }, + }, + ], + }, + ]); + + let navigateSpy = jest.spyOn(router, "navigate"); + + let { container } = render( + + + + ); + + await waitFor(() => screen.getByText("Page B")); + + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+

+ Page B +

+

+ 0 +

+
" + `); + expect(navigateSpy).toHaveBeenCalledTimes(2); + expect(navigateSpy.mock.calls[0]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 0 } }, + ]); + expect(navigateSpy.mock.calls[1]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 0 } }, + ]); + expect(renders).toEqual([0, 0]); + }); + + it("handles setState in render in StrictMode using a data router (async loader)", async () => { + let renders: number[] = []; + const router = createMemoryRouter([ + { + path: "/", + children: [ + { + index: true, + Component() { + let [count, setCount] = React.useState(0); if (count === 0) { setCount(1); } - }, [count]); - return ; + return ; + }, }, - }, - { - path: "b", - Component() { - let { state } = useLocation() as { state: { count: number } }; - renders.push(state.count); - return ( - <> -

Page B

-

{state.count}

- - ); + { + path: "b", + async loader() { + await new Promise((r) => setTimeout(r, 10)); + return null; + }, + Component() { + let { state } = useLocation() as { state: { count: number } }; + renders.push(state.count); + return ( + <> +

Page B

+

{state.count}

+ + ); + }, }, - }, - ], - }, - ]); + ], + }, + ]); - let navigateSpy = jest.spyOn(router, "navigate"); + let navigateSpy = jest.spyOn(router, "navigate"); - let { container } = render( - - - - ); + let { container } = render( + + + + ); - await waitFor(() => screen.getByText("Page B")); + await waitFor(() => screen.getByText("Page B")); - expect(getHtml(container)).toMatchInlineSnapshot(` + expect(getHtml(container)).toMatchInlineSnapshot(` "

Page B @@ -685,66 +757,219 @@ describe("", () => {

" `); - expect(navigateSpy).toHaveBeenCalledTimes(3); - expect(navigateSpy.mock.calls[0]).toMatchObject([ - { pathname: "/b" }, - { state: { count: 0 } }, - ]); - expect(navigateSpy.mock.calls[1]).toMatchObject([ - { pathname: "/b" }, - { state: { count: 0 } }, - ]); - expect(renders).toEqual([1, 1]); + expect(navigateSpy).toHaveBeenCalledTimes(2); + expect(navigateSpy.mock.calls[0]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 1 } }, + ]); + expect(navigateSpy.mock.calls[1]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 1 } }, + ]); + // /a/b rendered with the same state value both times + expect(renders).toEqual([1, 1]); + }); + + it("handles setState in effect in StrictMode using a data router (async loader)", async () => { + let renders: number[] = []; + const router = createMemoryRouter([ + { + path: "/", + children: [ + { + index: true, + Component() { + // When state managed by react and changes during render, we'll + // only "see" the value from the first pass through here in our + // effects + let [count, setCount] = React.useState(0); + React.useEffect(() => { + if (count === 0) { + setCount(1); + } + }, [count]); + return ; + }, + }, + { + path: "b", + async loader() { + await new Promise((r) => setTimeout(r, 10)); + return null; + }, + Component() { + let { state } = useLocation() as { state: { count: number } }; + renders.push(state.count); + return ( + <> +

Page B

+

{state.count}

+ + ); + }, + }, + ], + }, + ]); + + let navigateSpy = jest.spyOn(router, "navigate"); + + let { container } = render( + + + + ); + + await waitFor(() => screen.getByText("Page B")); + + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+

+ Page B +

+

+ 1 +

+
" + `); + expect(navigateSpy).toHaveBeenCalledTimes(3); + expect(navigateSpy.mock.calls[0]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 0 } }, + ]); + expect(navigateSpy.mock.calls[1]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 0 } }, + ]); + // StrictMode only applies the double-effect execution on component mount, + // not component update + expect(navigateSpy.mock.calls[2]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 1 } }, + ]); + // /a/b rendered with the latest state value both times + expect(renders).toEqual([1, 1]); + }); }); - it("handles setState in render in StrictMode using a data router (async loader)", async () => { - let renders: number[] = []; - const router = createMemoryRouter([ - { - path: "/", - children: [ - { - index: true, - Component() { - let [count, setCount] = React.useState(0); - if (count === 0) { - setCount(1); - } - return ; + describe("v7_startTransition = true", () => { + it("handles setState in render in StrictMode using a data router (sync loader)", async () => { + let renders: number[] = []; + const router = createMemoryRouter([ + { + path: "/", + children: [ + { + index: true, + Component() { + let [count, setCount] = React.useState(0); + if (count === 0) { + setCount(1); + } + return ; + }, }, - }, - { - path: "b", - async loader() { - await new Promise((r) => setTimeout(r, 10)); - return null; + { + path: "b", + Component() { + let { state } = useLocation() as { state: { count: number } }; + renders.push(state.count); + return ( + <> +

Page B

+

{state.count}

+ + ); + }, + }, + ], + }, + ]); + + let navigateSpy = jest.spyOn(router, "navigate"); + + let { container } = render( + + + + ); + + await waitFor(() => screen.getByText("Page B")); + + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+

+ Page B +

+

+ 1 +

+
" + `); + expect(navigateSpy).toHaveBeenCalledTimes(2); + expect(navigateSpy.mock.calls[0]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 1 } }, + ]); + expect(navigateSpy.mock.calls[1]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 1 } }, + ]); + expect(renders).toEqual([1, 1]); + }); + + it("handles setState in effect in StrictMode using a data router (sync loader)", async () => { + let renders: number[] = []; + const router = createMemoryRouter([ + { + path: "/", + children: [ + { + index: true, + Component() { + let [count, setCount] = React.useState(0); + React.useEffect(() => { + if (count === 0) { + setCount(1); + } + }, [count]); + return ; + }, }, - Component() { - let { state } = useLocation() as { state: { count: number } }; - renders.push(state.count); - return ( - <> -

Page B

-

{state.count}

- - ); + { + path: "b", + Component() { + let { state } = useLocation() as { state: { count: number } }; + renders.push(state.count); + return ( + <> +

Page B

+

{state.count}

+ + ); + }, }, - }, - ], - }, - ]); + ], + }, + ]); - let navigateSpy = jest.spyOn(router, "navigate"); + let navigateSpy = jest.spyOn(router, "navigate"); - let { container } = render( - - - - ); + let { container } = render( + + + + ); - await waitFor(() => screen.getByText("Page B")); + await waitFor(() => screen.getByText("Page B")); - expect(getHtml(container)).toMatchInlineSnapshot(` + expect(getHtml(container)).toMatchInlineSnapshot(` "

Page B @@ -754,72 +979,151 @@ describe("", () => {

" `); - expect(navigateSpy).toHaveBeenCalledTimes(2); - expect(navigateSpy.mock.calls[0]).toMatchObject([ - { pathname: "/b" }, - { state: { count: 1 } }, - ]); - expect(navigateSpy.mock.calls[1]).toMatchObject([ - { pathname: "/b" }, - { state: { count: 1 } }, - ]); - // /a/b rendered with the same state value both times - expect(renders).toEqual([1, 1]); - }); + expect(navigateSpy).toHaveBeenCalledTimes(3); + expect(navigateSpy.mock.calls[0]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 0 } }, + ]); + expect(navigateSpy.mock.calls[1]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 0 } }, + ]); + expect(navigateSpy.mock.calls[2]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 1 } }, + ]); + expect(renders).toEqual([1, 1]); + }); - it("handles setState in effect in StrictMode using a data router (async loader)", async () => { - let renders: number[] = []; - const router = createMemoryRouter([ - { - path: "/", - children: [ - { - index: true, - Component() { - // When state managed by react and changes during render, we'll - // only "see" the value from the first pass through here in our - // effects - let [count, setCount] = React.useState(0); - React.useEffect(() => { + it("handles setState in render in StrictMode using a data router (async loader)", async () => { + let renders: number[] = []; + const router = createMemoryRouter([ + { + path: "/", + children: [ + { + index: true, + Component() { + let [count, setCount] = React.useState(0); if (count === 0) { setCount(1); } - }, [count]); - return ; + return ; + }, }, - }, - { - path: "b", - async loader() { - await new Promise((r) => setTimeout(r, 10)); - return null; + { + path: "b", + async loader() { + await new Promise((r) => setTimeout(r, 10)); + return null; + }, + Component() { + let { state } = useLocation() as { state: { count: number } }; + renders.push(state.count); + return ( + <> +

Page B

+

{state.count}

+ + ); + }, }, - Component() { - let { state } = useLocation() as { state: { count: number } }; - renders.push(state.count); - return ( - <> -

Page B

-

{state.count}

- - ); + ], + }, + ]); + + let navigateSpy = jest.spyOn(router, "navigate"); + + let { container } = render( + + + + ); + + await waitFor(() => screen.getByText("Page B")); + + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+

+ Page B +

+

+ 1 +

+
" + `); + expect(navigateSpy).toHaveBeenCalledTimes(2); + expect(navigateSpy.mock.calls[0]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 1 } }, + ]); + expect(navigateSpy.mock.calls[1]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 1 } }, + ]); + // /a/b rendered with the same state value both times + expect(renders).toEqual([1, 1]); + }); + + it("handles setState in effect in StrictMode using a data router (async loader)", async () => { + let renders: number[] = []; + const router = createMemoryRouter([ + { + path: "/", + children: [ + { + index: true, + Component() { + // When state managed by react and changes during render, we'll + // only "see" the value from the first pass through here in our + // effects + let [count, setCount] = React.useState(0); + React.useEffect(() => { + if (count === 0) { + setCount(1); + } + }, [count]); + return ; + }, }, - }, - ], - }, - ]); + { + path: "b", + async loader() { + await new Promise((r) => setTimeout(r, 10)); + return null; + }, + Component() { + let { state } = useLocation() as { state: { count: number } }; + renders.push(state.count); + return ( + <> +

Page B

+

{state.count}

+ + ); + }, + }, + ], + }, + ]); - let navigateSpy = jest.spyOn(router, "navigate"); + let navigateSpy = jest.spyOn(router, "navigate"); - let { container } = render( - - - - ); + let { container } = render( + + + + ); - await waitFor(() => screen.getByText("Page B")); + await waitFor(() => screen.getByText("Page B")); - expect(getHtml(container)).toMatchInlineSnapshot(` + expect(getHtml(container)).toMatchInlineSnapshot(` "

Page B @@ -829,23 +1133,24 @@ describe("", () => {

" `); - expect(navigateSpy).toHaveBeenCalledTimes(3); - expect(navigateSpy.mock.calls[0]).toMatchObject([ - { pathname: "/b" }, - { state: { count: 0 } }, - ]); - expect(navigateSpy.mock.calls[1]).toMatchObject([ - { pathname: "/b" }, - { state: { count: 0 } }, - ]); - // StrictMode only applies the double-effect execution on component mount, - // not component update - expect(navigateSpy.mock.calls[2]).toMatchObject([ - { pathname: "/b" }, - { state: { count: 1 } }, - ]); - // /a/b rendered with the latest state value both times - expect(renders).toEqual([1, 1]); + expect(navigateSpy).toHaveBeenCalledTimes(3); + expect(navigateSpy.mock.calls[0]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 0 } }, + ]); + expect(navigateSpy.mock.calls[1]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 0 } }, + ]); + // StrictMode only applies the double-effect execution on component mount, + // not component update + expect(navigateSpy.mock.calls[2]).toMatchObject([ + { pathname: "/b" }, + { state: { count: 1 } }, + ]); + // /a/b rendered with the latest state value both times + expect(renders).toEqual([1, 1]); + }); }); }); diff --git a/packages/react-router/__tests__/useNavigate-test.tsx b/packages/react-router/__tests__/useNavigate-test.tsx index 4512c03c5f..95499722d5 100644 --- a/packages/react-router/__tests__/useNavigate-test.tsx +++ b/packages/react-router/__tests__/useNavigate-test.tsx @@ -7,7 +7,6 @@ import { Route, useNavigate, useLocation, - useRoutes, createMemoryRouter, createRoutesFromElements, Outlet, diff --git a/packages/react-router/index.ts b/packages/react-router/index.ts index 8167fcea9c..ad1e1fd56e 100644 --- a/packages/react-router/index.ts +++ b/packages/react-router/index.ts @@ -23,7 +23,7 @@ import type { To, InitialEntry, LazyRouteFunction, - FutureConfig, + FutureConfig as RouterFutureConfig, } from "@remix-run/router"; import { AbortedDeferredError, @@ -43,6 +43,7 @@ import { UNSAFE_warning as warning, } from "@remix-run/router"; +import startTransitionImpl from "./lib/polyfills/start-transition"; import type { AwaitProps, MemoryRouterProps, @@ -55,6 +56,7 @@ import type { RouterProps, RoutesProps, RouterProviderProps, + FutureConfig, } from "./lib/components"; import { createRoutesFromChildren, @@ -127,6 +129,7 @@ export type { DataRouteMatch, DataRouteObject, Fetcher, + FutureConfig, Hash, IndexRouteObject, IndexRouteProps, @@ -256,7 +259,7 @@ export function createMemoryRouter( routes: RouteObject[], opts?: { basename?: string; - future?: Partial>; + future?: Partial>; hydrationData?: HydrationState; initialEntries?: InitialEntry[]; initialIndex?: number; @@ -301,4 +304,5 @@ export { mapRouteProperties as UNSAFE_mapRouteProperties, useRouteId as UNSAFE_useRouteId, useRoutesImpl as UNSAFE_useRoutesImpl, + startTransitionImpl as UNSAFE_startTransitionImpl, }; diff --git a/packages/react-router/lib/components.tsx b/packages/react-router/lib/components.tsx index e4d4eee360..444b496bf3 100644 --- a/packages/react-router/lib/components.tsx +++ b/packages/react-router/lib/components.tsx @@ -22,6 +22,7 @@ import { UNSAFE_getPathContributingMatches as getPathContributingMatches, } from "@remix-run/router"; +import startTransitionImpl from "./polyfills/start-transition"; import type { DataRouteObject, IndexRouteObject, @@ -49,47 +50,35 @@ import { useLocation, } from "./hooks"; +export interface FutureConfig { + v7_startTransition: boolean; +} + export interface RouterProviderProps { fallbackElement?: React.ReactNode; router: RemixRouter; + future?: FutureConfig; } -// Webpack + React 17 fails to compile on any of the following: -// * import { startTransition } from "react" -// * import * as React from from "react"; -// "startTransition" in React ? React.startTransition(() => setState()) : setState() -// * import * as React from from "react"; -// "startTransition" in React ? React["startTransition"](() => setState()) : setState() -// -// Moving it to a constant such as the following solves the Webpack/React 17 issue: -// * import * as React from from "react"; -// const START_TRANSITION = "startTransition"; -// START_TRANSITION in React ? React[START_TRANSITION](() => setState()) : setState() -// -// However, that introduces webpack/terser minification issues in production builds -// in React 18 where minification/obfuscation ends up removing the call of -// React.startTransition entirely from the first half of the ternary. Grabbing -// this reference once up front resolves that issue. -const START_TRANSITION = "startTransition"; -const startTransitionImpl = React[START_TRANSITION]; - /** * Given a Remix Router instance, render the appropriate UI */ export function RouterProvider({ fallbackElement, router, + future, }: RouterProviderProps): React.ReactElement { // Need to use a layout effect here so we are subscribed early enough to // pick up on any render-driven redirects/navigations (useEffect/) let [state, setStateImpl] = React.useState(router.state); + let { v7_startTransition } = future || {}; let setState = React.useCallback( (newState: RouterState) => { - startTransitionImpl + v7_startTransition && startTransitionImpl ? startTransitionImpl(() => setStateImpl(newState)) : setStateImpl(newState); }, - [setStateImpl] + [setStateImpl, v7_startTransition] ); React.useLayoutEffect(() => router.subscribe(setState), [router, setState]); @@ -168,6 +157,7 @@ export interface MemoryRouterProps { children?: React.ReactNode; initialEntries?: InitialEntry[]; initialIndex?: number; + future?: FutureConfig; } /** @@ -180,6 +170,7 @@ export function MemoryRouter({ children, initialEntries, initialIndex, + future, }: MemoryRouterProps): React.ReactElement { let historyRef = React.useRef(); if (historyRef.current == null) { @@ -195,13 +186,14 @@ export function MemoryRouter({ action: history.action, location: history.location, }); + let { v7_startTransition } = future || {}; let setState = React.useCallback( (newState: { action: NavigationType; location: Location }) => { - startTransitionImpl + v7_startTransition && startTransitionImpl ? startTransitionImpl(() => setStateImpl(newState)) : setStateImpl(newState); }, - [setStateImpl] + [setStateImpl, v7_startTransition] ); React.useLayoutEffect(() => history.listen(setState), [history, setState]); diff --git a/packages/react-router/lib/polyfills/start-transition.ts b/packages/react-router/lib/polyfills/start-transition.ts new file mode 100644 index 0000000000..72c6b8bba3 --- /dev/null +++ b/packages/react-router/lib/polyfills/start-transition.ts @@ -0,0 +1,28 @@ +import * as React from "react"; + +/** + Not a true "polyfill" since we guard via the feature flag at runtime, + but close enough :) + + Webpack + React 17 fails to compile on any of the following because webpack + complains that `startTransition` doesn't exist in `React`: + * import { startTransition } from "react" + * import * as React from from "react"; + "startTransition" in React ? React.startTransition(() => setState()) : setState() + * import * as React from from "react"; + "startTransition" in React ? React["startTransition"](() => setState()) : setState() + + Moving it to a constant such as the following solves the Webpack/React 17 issue: + * import * as React from from "react"; + const START_TRANSITION = "startTransition"; + START_TRANSITION in React ? React[START_TRANSITION](() => setState()) : setState() + + However, that introduces webpack/terser minification issues in production builds + in React 18 where minification/obfuscation ends up removing the call of + React.startTransition entirely from the first half of the ternary. Grabbing + this exported reference once up front resolves that issue. + + See https://github.com/remix-run/react-router/issues/10579 +*/ +const START_TRANSITION = "startTransition"; +export default React[START_TRANSITION];