diff --git a/.changeset/fix-component-rerenders-router.md b/.changeset/fix-component-rerenders-router.md new file mode 100644 index 0000000000..ddb74f5a0a --- /dev/null +++ b/.changeset/fix-component-rerenders-router.md @@ -0,0 +1,5 @@ +--- +"@remix-run/router": patch +--- + +Deprecate the `createRouter` `detectErrorBoundary` option in favor of the new `mapRouteProperties` option for converting a framework-agnostic route to a framework-aware route. This allows us to set more than just the `hasErrorBoundary` property during route pre-processing, and is now used for mapping `Component -> element` and `ErrorBoundary -> errorElement` in `react-router`. diff --git a/.changeset/fix-component-rerenders.md b/.changeset/fix-component-rerenders.md new file mode 100644 index 0000000000..1dd8d12ac5 --- /dev/null +++ b/.changeset/fix-component-rerenders.md @@ -0,0 +1,6 @@ +--- +"react-router": patch +"react-router-dom": patch +--- + +Fix inadvertent re-renders when using `Component` instead of `element` on a route definition diff --git a/package.json b/package.json index 819d84e8e7..69ce64c5a4 100644 --- a/package.json +++ b/package.json @@ -105,13 +105,13 @@ }, "filesize": { "packages/router/dist/router.umd.min.js": { - "none": "44 kB" + "none": "44.1 kB" }, "packages/react-router/dist/react-router.production.min.js": { - "none": "13 kB" + "none": "13.1 kB" }, "packages/react-router/dist/umd/react-router.production.min.js": { - "none": "15.1 kB" + "none": "15.3 kB" }, "packages/react-router-dom/dist/react-router-dom.production.min.js": { "none": "11.6 kB" diff --git a/packages/react-router-dom/__tests__/data-static-router-test.tsx b/packages/react-router-dom/__tests__/data-static-router-test.tsx index 41fe0a8df6..546b4704ba 100644 --- a/packages/react-router-dom/__tests__/data-static-router-test.tsx +++ b/packages/react-router-dom/__tests__/data-static-router-test.tsx @@ -993,14 +993,15 @@ describe("A ", () => { expect(router.routes).toMatchInlineSnapshot(` [ { - "ErrorBoundary": [Function], + "ErrorBoundary": undefined, "children": [ { - "ErrorBoundary": [Function], + "ErrorBoundary": undefined, "children": undefined, "element":

Hi again!

, + "errorElement": , "hasErrorBoundary": true, "id": "0-0", "path": "path", @@ -1009,6 +1010,7 @@ describe("A ", () => { "element":

Hi!

, + "errorElement": , "hasErrorBoundary": true, "id": "0", "path": "the", @@ -1022,14 +1024,15 @@ describe("A ", () => { "pathname": "/the", "pathnameBase": "/the", "route": { - "ErrorBoundary": [Function], + "ErrorBoundary": undefined, "children": [ { - "ErrorBoundary": [Function], + "ErrorBoundary": undefined, "children": undefined, "element":

Hi again!

, + "errorElement": , "hasErrorBoundary": true, "id": "0-0", "path": "path", @@ -1038,6 +1041,7 @@ describe("A ", () => { "element":

Hi!

, + "errorElement": , "hasErrorBoundary": true, "id": "0", "path": "the", @@ -1048,11 +1052,12 @@ describe("A ", () => { "pathname": "/the/path", "pathnameBase": "/the/path", "route": { - "ErrorBoundary": [Function], + "ErrorBoundary": undefined, "children": undefined, "element":

Hi again!

, + "errorElement": , "hasErrorBoundary": true, "id": "0-0", "path": "path", diff --git a/packages/react-router-dom/__tests__/exports-test.tsx b/packages/react-router-dom/__tests__/exports-test.tsx index 91392a2f85..46e24e5297 100644 --- a/packages/react-router-dom/__tests__/exports-test.tsx +++ b/packages/react-router-dom/__tests__/exports-test.tsx @@ -1,7 +1,7 @@ import * as ReactRouter from "react-router"; import * as ReactRouterDOM from "react-router-dom"; -let nonReExportedKeys = new Set(["UNSAFE_detectErrorBoundary"]); +let nonReExportedKeys = new Set(["UNSAFE_mapRouteProperties"]); describe("react-router-dom", () => { for (let key in ReactRouter) { diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index dcfa2b8f7d..09fa1f4146 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -23,7 +23,7 @@ import { UNSAFE_DataRouterStateContext as DataRouterStateContext, UNSAFE_NavigationContext as NavigationContext, UNSAFE_RouteContext as RouteContext, - UNSAFE_detectErrorBoundary as detectErrorBoundary, + UNSAFE_mapRouteProperties as mapRouteProperties, } from "react-router"; import type { BrowserHistory, @@ -220,7 +220,7 @@ export function createBrowserRouter( history: createBrowserHistory({ window: opts?.window }), hydrationData: opts?.hydrationData || parseHydrationData(), routes, - detectErrorBoundary, + mapRouteProperties, }).initialize(); } @@ -234,7 +234,7 @@ export function createHashRouter( history: createHashHistory({ window: opts?.window }), hydrationData: opts?.hydrationData || parseHydrationData(), routes, - detectErrorBoundary, + mapRouteProperties, }).initialize(); } diff --git a/packages/react-router-dom/server.tsx b/packages/react-router-dom/server.tsx index a2afd75715..e89006e066 100644 --- a/packages/react-router-dom/server.tsx +++ b/packages/react-router-dom/server.tsx @@ -17,6 +17,7 @@ import { createStaticHandler as routerCreateStaticHandler, UNSAFE_convertRoutesToDataRoutes as convertRoutesToDataRoutes, } from "@remix-run/router"; +import { UNSAFE_mapRouteProperties as mapRouteProperties } from "react-router"; import type { Location, RouteObject, To } from "react-router-dom"; import { Routes } from "react-router-dom"; import { @@ -206,12 +207,9 @@ function getStatelessNavigator() { }; } -let detectErrorBoundary = (route: RouteObject) => - Boolean(route.ErrorBoundary) || Boolean(route.errorElement); - type CreateStaticHandlerOptions = Omit< RouterCreateStaticHandlerOptions, - "detectErrorBoundary" + "detectErrorBoundary" | "mapRouteProperties" >; export function createStaticHandler( @@ -220,7 +218,7 @@ export function createStaticHandler( ) { return routerCreateStaticHandler(routes, { ...opts, - detectErrorBoundary, + mapRouteProperties, }); } @@ -231,7 +229,7 @@ export function createStaticRouter( let manifest: RouteManifest = {}; let dataRoutes = convertRoutesToDataRoutes( routes, - detectErrorBoundary, + mapRouteProperties, undefined, manifest ); diff --git a/packages/react-router-native/__tests__/exports-test.tsx b/packages/react-router-native/__tests__/exports-test.tsx index 0c219c4205..d851649e80 100644 --- a/packages/react-router-native/__tests__/exports-test.tsx +++ b/packages/react-router-native/__tests__/exports-test.tsx @@ -1,7 +1,7 @@ import * as ReactRouter from "react-router"; import * as ReactRouterNative from "react-router-native"; -let nonReExportedKeys = new Set(["UNSAFE_detectErrorBoundary"]); +let nonReExportedKeys = new Set(["UNSAFE_mapRouteProperties"]); describe("react-router-native", () => { for (let key in ReactRouter) { diff --git a/packages/react-router/index.ts b/packages/react-router/index.ts index a48c0c6492..992e0f556e 100644 --- a/packages/react-router/index.ts +++ b/packages/react-router/index.ts @@ -1,3 +1,4 @@ +import * as React from "react"; import type { ActionFunction, ActionFunctionArgs, @@ -207,27 +208,46 @@ export { useRoutes, }; -function detectErrorBoundary(route: RouteObject) { - if (__DEV__) { - if (route.Component && route.element) { - warning( - false, - "You should not include both `Component` and `element` on your route - " + - "`element` will be ignored." - ); +function mapRouteProperties(route: RouteObject) { + let updates: Partial & { hasErrorBoundary: boolean } = { + // Note: this check also occurs in createRoutesFromChildren so update + // there if you change this -- please and thank you! + hasErrorBoundary: route.ErrorBoundary != null || route.errorElement != null, + }; + + if (route.Component) { + if (__DEV__) { + if (route.element) { + warning( + false, + "You should not include both `Component` and `element` on your route - " + + "`Component` will be used." + ); + } } - if (route.ErrorBoundary && route.errorElement) { - warning( - false, - "You should not include both `ErrorBoundary` and `errorElement` on your route - " + - "`errorElement` will be ignored." - ); + Object.assign(updates, { + element: React.createElement(route.Component), + Component: undefined, + }); + } + + if (route.ErrorBoundary) { + if (__DEV__) { + if (route.errorElement) { + warning( + false, + "You should not include both `ErrorBoundary` and `errorElement` on your route - " + + "`ErrorBoundary` will be used." + ); + } } + Object.assign(updates, { + errorElement: React.createElement(route.ErrorBoundary), + ErrorBoundary: undefined, + }); } - // Note: this check also occurs in createRoutesFromChildren so update - // there if you change this - return Boolean(route.ErrorBoundary) || Boolean(route.errorElement); + return updates; } export function createMemoryRouter( @@ -249,7 +269,7 @@ export function createMemoryRouter( }), hydrationData: opts?.hydrationData, routes, - detectErrorBoundary, + mapRouteProperties, }).initialize(); } @@ -273,5 +293,5 @@ export { RouteContext as UNSAFE_RouteContext, DataRouterContext as UNSAFE_DataRouterContext, DataRouterStateContext as UNSAFE_DataRouterStateContext, - detectErrorBoundary as UNSAFE_detectErrorBoundary, + mapRouteProperties as UNSAFE_mapRouteProperties, }; diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index 50d86133a6..c054436e63 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -495,6 +495,8 @@ function DefaultErrorComponent() { ); } +const defaultErrorElement = ; + type RenderErrorBoundaryProps = React.PropsWithChildren<{ location: Location; error: any; @@ -639,23 +641,17 @@ export function _renderMatches( // Only data routers handle errors let errorElement: React.ReactNode | null = null; if (dataRouterState) { - if (match.route.ErrorBoundary) { - errorElement = ; - } else if (match.route.errorElement) { - errorElement = match.route.errorElement; - } else { - errorElement = ; - } + errorElement = match.route.errorElement || defaultErrorElement; } let matches = parentMatches.concat(renderedMatches.slice(0, index + 1)); let getChildren = () => { - let children: React.ReactNode = outlet; + let children: React.ReactNode; if (error) { children = errorElement; - } else if (match.route.Component) { - children = ; } else if (match.route.element) { children = match.route.element; + } else { + children = outlet; } return ( **Warning** > @@ -16,11 +16,16 @@ A Router instance can be created using `createRouter`: // Create and initialize a router. "initialize" contains all side effects // including history listeners and kicking off the initial data fetch let router = createRouter({ - // Routes array - routes: , - // History instance - history, -}).initialize() + // Required properties + routes, // Routes array + history, // History instance + + // Optional properties + basename, // Base path + mapRouteProperties, // Map function framework-agnostic routes to framework-aware routes + future, // Future flags + hydrationData, // Hydration data if using server-side-rendering +}).initialize(); ``` Internally, the Router represents the state in an object of the following format, which is available through `router.state`. You can also register a subscriber of the signature `(state: RouterState) => void` to execute when the state updates via `router.subscribe()`; diff --git a/packages/router/router.ts b/packages/router/router.ts index 9c518b3f34..3427af0149 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -32,6 +32,7 @@ import type { V7_FormMethod, HTMLFormMethod, MutationFormMethod, + MapRoutePropertiesFunction, } from "./utils"; import { ErrorResponse, @@ -343,7 +344,11 @@ export interface RouterInit { routes: AgnosticRouteObject[]; history: History; basename?: string; + /** + * @deprecated Use `mapRouteProperties` instead + */ detectErrorBoundary?: DetectErrorBoundaryFunction; + mapRouteProperties?: MapRoutePropertiesFunction; future?: FutureConfig; hydrationData?: HydrationState; } @@ -658,8 +663,10 @@ const isBrowser = typeof window.document.createElement !== "undefined"; const isServer = !isBrowser; -const defaultDetectErrorBoundary = (route: AgnosticRouteObject) => - Boolean(route.hasErrorBoundary); +const defaultMapRouteProperties: MapRoutePropertiesFunction = (route) => ({ + hasErrorBoundary: Boolean(route.hasErrorBoundary), +}); + //#endregion //////////////////////////////////////////////////////////////////////////////// @@ -675,15 +682,25 @@ export function createRouter(init: RouterInit): Router { "You must provide a non-empty routes array to createRouter" ); - let detectErrorBoundary = - init.detectErrorBoundary || defaultDetectErrorBoundary; + let mapRouteProperties: MapRoutePropertiesFunction; + if (init.mapRouteProperties) { + mapRouteProperties = init.mapRouteProperties; + } else if (init.detectErrorBoundary) { + // If they are still using the deprecated version, wrap it with the new API + let detectErrorBoundary = init.detectErrorBoundary; + mapRouteProperties = (route) => ({ + hasErrorBoundary: detectErrorBoundary(route), + }); + } else { + mapRouteProperties = defaultMapRouteProperties; + } // Routes keyed by ID let manifest: RouteManifest = {}; // Routes in tree format for matching let dataRoutes = convertRoutesToDataRoutes( init.routes, - detectErrorBoundary, + mapRouteProperties, undefined, manifest ); @@ -1327,7 +1344,7 @@ export function createRouter(init: RouterInit): Router { actionMatch, matches, manifest, - detectErrorBoundary, + mapRouteProperties, router.basename ); @@ -1685,7 +1702,7 @@ export function createRouter(init: RouterInit): Router { match, requestMatches, manifest, - detectErrorBoundary, + mapRouteProperties, router.basename ); @@ -1928,7 +1945,7 @@ export function createRouter(init: RouterInit): Router { match, matches, manifest, - detectErrorBoundary, + mapRouteProperties, router.basename ); @@ -2146,7 +2163,7 @@ export function createRouter(init: RouterInit): Router { match, matches, manifest, - detectErrorBoundary, + mapRouteProperties, router.basename ) ), @@ -2158,7 +2175,7 @@ export function createRouter(init: RouterInit): Router { f.match, f.matches, manifest, - detectErrorBoundary, + mapRouteProperties, router.basename ); } else { @@ -2477,7 +2494,11 @@ export const UNSAFE_DEFERRED_SYMBOL = Symbol("deferred"); export interface CreateStaticHandlerOptions { basename?: string; + /** + * @deprecated Use `mapRouteProperties` instead + */ detectErrorBoundary?: DetectErrorBoundaryFunction; + mapRouteProperties?: MapRoutePropertiesFunction; } export function createStaticHandler( @@ -2490,15 +2511,26 @@ export function createStaticHandler( ); let manifest: RouteManifest = {}; - let detectErrorBoundary = - opts?.detectErrorBoundary || defaultDetectErrorBoundary; + let basename = (opts ? opts.basename : null) || "/"; + let mapRouteProperties: MapRoutePropertiesFunction; + if (opts?.mapRouteProperties) { + mapRouteProperties = opts.mapRouteProperties; + } else if (opts?.detectErrorBoundary) { + // If they are still using the deprecated version, wrap it with the new API + let detectErrorBoundary = opts.detectErrorBoundary; + mapRouteProperties = (route) => ({ + hasErrorBoundary: detectErrorBoundary(route), + }); + } else { + mapRouteProperties = defaultMapRouteProperties; + } + let dataRoutes = convertRoutesToDataRoutes( routes, - detectErrorBoundary, + mapRouteProperties, undefined, manifest ); - let basename = (opts ? opts.basename : null) || "/"; /** * The query() method is intended for document requests, in which we want to @@ -2752,7 +2784,7 @@ export function createStaticHandler( actionMatch, matches, manifest, - detectErrorBoundary, + mapRouteProperties, basename, true, isRouteRequest, @@ -2920,7 +2952,7 @@ export function createStaticHandler( match, matches, manifest, - detectErrorBoundary, + mapRouteProperties, basename, true, isRouteRequest, @@ -3272,7 +3304,7 @@ function shouldRevalidateLoader( */ async function loadLazyRouteModule( route: AgnosticDataRouteObject, - detectErrorBoundary: DetectErrorBoundaryFunction, + mapRouteProperties: MapRoutePropertiesFunction, manifest: RouteManifest ) { if (!route.lazy) { @@ -3327,7 +3359,7 @@ async function loadLazyRouteModule( } // Mutate the route with the provided updates. Do this first so we pass - // the updated version to detectErrorBoundary + // the updated version to mapRouteProperties Object.assign(routeToUpdate, routeUpdates); // Mutate the `hasErrorBoundary` property on the route based on the route @@ -3335,9 +3367,10 @@ async function loadLazyRouteModule( // route again. Object.assign(routeToUpdate, { // To keep things framework agnostic, we use the provided - // `detectErrorBoundary` function to set the `hasErrorBoundary` route - // property since the logic will differ between frameworks. - hasErrorBoundary: detectErrorBoundary({ ...routeToUpdate }), + // `mapRouteProperties` (or wrapped `detectErrorBoundary`) function to + // set the framework-aware properties (`element`/`hasErrorBoundary`) since + // the logic will differ between frameworks. + ...mapRouteProperties(routeToUpdate), lazy: undefined, }); } @@ -3348,7 +3381,7 @@ async function callLoaderOrAction( match: AgnosticDataRouteMatch, matches: AgnosticDataRouteMatch[], manifest: RouteManifest, - detectErrorBoundary: DetectErrorBoundaryFunction, + mapRouteProperties: MapRoutePropertiesFunction, basename = "/", isStaticRequest: boolean = false, isRouteRequest: boolean = false, @@ -3378,12 +3411,12 @@ async function callLoaderOrAction( // Run statically defined handler in parallel with lazy() let values = await Promise.all([ runHandler(handler), - loadLazyRouteModule(match.route, detectErrorBoundary, manifest), + loadLazyRouteModule(match.route, mapRouteProperties, manifest), ]); result = values[0]; } else { // Load lazy route module, then run any returned handler - await loadLazyRouteModule(match.route, detectErrorBoundary, manifest); + await loadLazyRouteModule(match.route, mapRouteProperties, manifest); handler = match.route[type]; if (handler) { diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 777ef3fae4..0b8b18d8c7 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -169,11 +169,23 @@ export interface ShouldRevalidateFunction { /** * Function provided by the framework-aware layers to set `hasErrorBoundary` * from the framework-aware `errorElement` prop + * + * @deprecated Use `mapRouteProperties` instead */ export interface DetectErrorBoundaryFunction { (route: AgnosticRouteObject): boolean; } +/** + * Function provided by the framework-aware layers to set any framework-specific + * properties from framework-agnostic properties + */ +export interface MapRoutePropertiesFunction { + (route: AgnosticRouteObject): { + hasErrorBoundary: boolean; + } & Record; +} + /** * Keys we cannot change from within a lazy() function. We spread all other keys * onto the route. Either they're meaningful to the router, or they'll get @@ -345,7 +357,7 @@ function isIndexRoute( // solely with AgnosticDataRouteObject's within the Router export function convertRoutesToDataRoutes( routes: AgnosticRouteObject[], - detectErrorBoundary: DetectErrorBoundaryFunction, + mapRouteProperties: MapRoutePropertiesFunction, parentPath: number[] = [], manifest: RouteManifest = {} ): AgnosticDataRouteObject[] { @@ -365,7 +377,7 @@ export function convertRoutesToDataRoutes( if (isIndexRoute(route)) { let indexRoute: AgnosticDataIndexRouteObject = { ...route, - hasErrorBoundary: detectErrorBoundary(route), + ...mapRouteProperties(route), id, }; manifest[id] = indexRoute; @@ -373,8 +385,8 @@ export function convertRoutesToDataRoutes( } else { let pathOrLayoutRoute: AgnosticDataNonIndexRouteObject = { ...route, + ...mapRouteProperties(route), id, - hasErrorBoundary: detectErrorBoundary(route), children: undefined, }; manifest[id] = pathOrLayoutRoute; @@ -382,7 +394,7 @@ export function convertRoutesToDataRoutes( if (route.children) { pathOrLayoutRoute.children = convertRoutesToDataRoutes( route.children, - detectErrorBoundary, + mapRouteProperties, treePath, manifest );