Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix inadvertent re-renders when using Component instead of element #10287

Merged
merged 7 commits into from Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/fix-component-rerenders.md
@@ -0,0 +1,7 @@
---
"react-router": patch
"react-router-dom": patch
"@remix-run/router": patch
---

Fix inadvertent re-renders when using `Component` instead of `element`
15 changes: 10 additions & 5 deletions packages/react-router-dom/__tests__/data-static-router-test.tsx
Expand Up @@ -992,14 +992,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 @@ -1008,6 +1009,7 @@ describe("A <StaticRouterProvider>", () => {
"element": <h1>
Hi!
</h1>,
"errorElement": <ErrorBoundary />,
"hasErrorBoundary": true,
"id": "0",
"path": "the",
Expand All @@ -1021,14 +1023,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 @@ -1037,6 +1040,7 @@ describe("A <StaticRouterProvider>", () => {
"element": <h1>
Hi!
</h1>,
"errorElement": <ErrorBoundary />,
"hasErrorBoundary": true,
"id": "0",
"path": "the",
Expand All @@ -1047,11 +1051,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_enhanceAgnosticRoute"]);

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_enhanceAgnosticRoute as enhanceAgnosticRoute,
} 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,
enhanceAgnosticRoute,
}).initialize();
}

Expand All @@ -234,7 +234,7 @@ export function createHashRouter(
history: createHashHistory({ window: opts?.window }),
hydrationData: opts?.hydrationData || parseHydrationData(),
routes,
detectErrorBoundary,
enhanceAgnosticRoute,
}).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_enhanceAgnosticRoute as enhanceAgnosticRoute } 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" | "enhanceAgnosticRoute"
>;

export function createStaticHandler(
Expand All @@ -220,7 +218,7 @@ export function createStaticHandler(
) {
return routerCreateStaticHandler(routes, {
...opts,
detectErrorBoundary,
enhanceAgnosticRoute,
});
}

Expand All @@ -231,7 +229,7 @@ export function createStaticRouter(
let manifest: RouteManifest = {};
let dataRoutes = convertRoutesToDataRoutes(
routes,
detectErrorBoundary,
enhanceAgnosticRoute,
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_enhanceAgnosticRoute"]);

describe("react-router-native", () => {
for (let key in ReactRouter) {
Expand Down
58 changes: 40 additions & 18 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,48 @@ 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 enhanceAgnosticRoute(route: RouteObject) {
let updates: Partial<RouteObject> = {};

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);
// there if you change this -- please and thank you!
if (route.ErrorBoundary || route.errorElement) {
updates.hasErrorBoundary = true;
}

return updates;
}

export function createMemoryRouter(
Expand All @@ -249,7 +271,7 @@ export function createMemoryRouter(
}),
hydrationData: opts?.hydrationData,
routes,
detectErrorBoundary,
enhanceAgnosticRoute,
}).initialize();
}

Expand All @@ -273,5 +295,5 @@ export {
RouteContext as UNSAFE_RouteContext,
DataRouterContext as UNSAFE_DataRouterContext,
DataRouterStateContext as UNSAFE_DataRouterStateContext,
detectErrorBoundary as UNSAFE_detectErrorBoundary,
enhanceAgnosticRoute as UNSAFE_enhanceAgnosticRoute,
};
6 changes: 3 additions & 3 deletions packages/react-router/lib/hooks.tsx
Expand Up @@ -491,6 +491,8 @@ function DefaultErrorComponent() {
);
}

const defaultErrorElement = <DefaultErrorComponent />;

type RenderErrorBoundaryProps = React.PropsWithChildren<{
location: Location;
error: any;
Expand Down Expand Up @@ -637,10 +639,8 @@ export function _renderMatches(
if (dataRouterState) {
if (match.route.ErrorBoundary) {
brophdawg11 marked this conversation as resolved.
Show resolved Hide resolved
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));
Expand Down