Skip to content

Commit

Permalink
fix inadvertent re-renders when using Component instead of element (#…
Browse files Browse the repository at this point in the history
…10287)

Co-authored-by: Mark Dalgleish <mark.john.dalgleish@gmail.com>
  • Loading branch information
brophdawg11 and markdalgleish committed Apr 11, 2023
1 parent 2a3c850 commit b7683f1
Show file tree
Hide file tree
Showing 13 changed files with 162 additions and 82 deletions.
5 changes: 5 additions & 0 deletions .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`.
6 changes: 6 additions & 0 deletions .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
6 changes: 3 additions & 3 deletions package.json
Expand Up @@ -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"
Expand Down
15 changes: 10 additions & 5 deletions packages/react-router-dom/__tests__/data-static-router-test.tsx
Expand Up @@ -993,14 +993,15 @@ describe("A <StaticRouterProvider>", () => {
expect(router.routes).toMatchInlineSnapshot(`
[
{
"ErrorBoundary": [Function],
"ErrorBoundary": undefined,
"children": [
{
"ErrorBoundary": [Function],
"ErrorBoundary": undefined,
"children": undefined,
"element": <h2>
Hi again!
</h2>,
"errorElement": <ErrorBoundary />,
"hasErrorBoundary": true,
"id": "0-0",
"path": "path",
Expand All @@ -1009,6 +1010,7 @@ describe("A <StaticRouterProvider>", () => {
"element": <h1>
Hi!
</h1>,
"errorElement": <ErrorBoundary />,
"hasErrorBoundary": true,
"id": "0",
"path": "the",
Expand All @@ -1022,14 +1024,15 @@ describe("A <StaticRouterProvider>", () => {
"pathname": "/the",
"pathnameBase": "/the",
"route": {
"ErrorBoundary": [Function],
"ErrorBoundary": undefined,
"children": [
{
"ErrorBoundary": [Function],
"ErrorBoundary": undefined,
"children": undefined,
"element": <h2>
Hi again!
</h2>,
"errorElement": <ErrorBoundary />,
"hasErrorBoundary": true,
"id": "0-0",
"path": "path",
Expand All @@ -1038,6 +1041,7 @@ describe("A <StaticRouterProvider>", () => {
"element": <h1>
Hi!
</h1>,
"errorElement": <ErrorBoundary />,
"hasErrorBoundary": true,
"id": "0",
"path": "the",
Expand All @@ -1048,11 +1052,12 @@ describe("A <StaticRouterProvider>", () => {
"pathname": "/the/path",
"pathnameBase": "/the/path",
"route": {
"ErrorBoundary": [Function],
"ErrorBoundary": undefined,
"children": undefined,
"element": <h2>
Hi again!
</h2>,
"errorElement": <ErrorBoundary />,
"hasErrorBoundary": true,
"id": "0-0",
"path": "path",
Expand Down
2 changes: 1 addition & 1 deletion 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) {
Expand Down
6 changes: 3 additions & 3 deletions packages/react-router-dom/index.tsx
Expand Up @@ -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,
Expand Down Expand Up @@ -220,7 +220,7 @@ export function createBrowserRouter(
history: createBrowserHistory({ window: opts?.window }),
hydrationData: opts?.hydrationData || parseHydrationData(),
routes,
detectErrorBoundary,
mapRouteProperties,
}).initialize();
}

Expand All @@ -234,7 +234,7 @@ export function createHashRouter(
history: createHashHistory({ window: opts?.window }),
hydrationData: opts?.hydrationData || parseHydrationData(),
routes,
detectErrorBoundary,
mapRouteProperties,
}).initialize();
}

Expand Down
10 changes: 4 additions & 6 deletions packages/react-router-dom/server.tsx
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
Expand All @@ -220,7 +218,7 @@ export function createStaticHandler(
) {
return routerCreateStaticHandler(routes, {
...opts,
detectErrorBoundary,
mapRouteProperties,
});
}

Expand All @@ -231,7 +229,7 @@ export function createStaticRouter(
let manifest: RouteManifest = {};
let dataRoutes = convertRoutesToDataRoutes(
routes,
detectErrorBoundary,
mapRouteProperties,
undefined,
manifest
);
Expand Down
2 changes: 1 addition & 1 deletion 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) {
Expand Down
58 changes: 39 additions & 19 deletions packages/react-router/index.ts
@@ -1,3 +1,4 @@
import * as React from "react";
import type {
ActionFunction,
ActionFunctionArgs,
Expand Down Expand Up @@ -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<RouteObject> & { 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(
Expand All @@ -249,7 +269,7 @@ export function createMemoryRouter(
}),
hydrationData: opts?.hydrationData,
routes,
detectErrorBoundary,
mapRouteProperties,
}).initialize();
}

Expand All @@ -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,
};
16 changes: 6 additions & 10 deletions packages/react-router/lib/hooks.tsx
Expand Up @@ -495,6 +495,8 @@ function DefaultErrorComponent() {
);
}

const defaultErrorElement = <DefaultErrorComponent />;

type RenderErrorBoundaryProps = React.PropsWithChildren<{
location: Location;
error: any;
Expand Down Expand Up @@ -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 = <match.route.ErrorBoundary />;
} else if (match.route.errorElement) {
errorElement = match.route.errorElement;
} else {
errorElement = <DefaultErrorComponent />;
}
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 = <match.route.Component />;
} else if (match.route.element) {
children = match.route.element;
} else {
children = outlet;
}
return (
<RenderedRoute
Expand Down
17 changes: 11 additions & 6 deletions packages/router/README.md
Expand Up @@ -2,7 +2,7 @@

The `@remix-run/router` package is a framework-agnostic routing package (sometimes referred to as a browser-emulator) that serves as the heart of [React Router][react-router] and [Remix][remix] and provides all the core functionality for routing coupled with data loading and data mutations. It comes with built-in handling of errors, race-conditions, interruptions, cancellations, lazy-loading data, and much, much more.

If you're using React Router, you should never `import` anything directly from the `@remix-run/router` or `react-router` packages, but you should have everything you need in either `react-router-dom` or `react-router-native`. Both of those packages re-export everything from `@remix-run/router` and `react-router`.
If you're using React Router, you should never `import` anything directly from the `@remix-run/router` - you should have everything you need in `react-router-dom` (or `react-router`/`react-router-native` if you're not rendering in the browser). All of those packages should re-export everything you would otherwise need from `@remix-run/router`.

> **Warning**
>
Expand All @@ -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()`;
Expand Down

0 comments on commit b7683f1

Please sign in to comment.