From cde2950e394ffc4d4b55eac1a56499e2fcee4c08 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 19 Dec 2022 17:13:22 -0500 Subject: [PATCH 01/12] Add ability for integration tests to specify future flags --- integration/helpers/create-fixture.ts | 18 ++++++++++++++++++ .../helpers/node-template/remix.config.js | 4 ++++ 2 files changed, 22 insertions(+) diff --git a/integration/helpers/create-fixture.ts b/integration/helpers/create-fixture.ts index 7cda8189132..d479dfb789a 100644 --- a/integration/helpers/create-fixture.ts +++ b/integration/helpers/create-fixture.ts @@ -8,6 +8,7 @@ import { sync as spawnSync } from "cross-spawn"; import type { JsonObject } from "type-fest"; import type { ServerMode } from "@remix-run/server-runtime/mode"; +import type { AppConfig } from "../../build/node_modules/@remix-run/dev"; import type { ServerBuild } from "../../build/node_modules/@remix-run/server-runtime"; import { createRequestHandler } from "../../build/node_modules/@remix-run/server-runtime"; import { createRequestHandler as createExpressHandler } from "../../build/node_modules/@remix-run/express"; @@ -20,6 +21,7 @@ interface FixtureInit { files?: { [filename: string]: string }; template?: "cf-template" | "deno-template" | "node-template"; setup?: "node" | "cloudflare"; + future?: AppConfig["future"]; } export type Fixture = Awaited>; @@ -174,6 +176,22 @@ export async function createFixtureProject( ); } } + + if (init.future) { + let contents = fse.readFileSync( + path.join(projectDir, "remix.config.js"), + "utf-8" + ); + if (!contents.includes("future: {},")) { + throw new Error("Invalid formatted remix.config.js in template"); + } + contents = contents.replace( + "future: {},", + "future: " + JSON.stringify(init.future) + "," + ); + fse.writeFileSync(path.join(projectDir, "remix.config.js"), contents); + } + await writeTestFiles(init, projectDir); build(projectDir, init.buildStdio, init.sourcemap); diff --git a/integration/helpers/node-template/remix.config.js b/integration/helpers/node-template/remix.config.js index adf2a0b5d3e..b7f693265aa 100644 --- a/integration/helpers/node-template/remix.config.js +++ b/integration/helpers/node-template/remix.config.js @@ -5,4 +5,8 @@ module.exports = { // assetsBuildDirectory: "public/build", // serverBuildPath: "build/index.js", // publicPath: "/build/", + + // !!! Don't adust this without changing the code that overwrites this + // in createFixtureProject() + future: {}, }; From 2bdf7f11a99115260e9b29db8916829345c6c65f Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 19 Dec 2022 17:15:22 -0500 Subject: [PATCH 02/12] Remove duplicate RemixErrorBoundary --- packages/remix-react/components.tsx | 18 +------ packages/remix-react/errorBoundaries.tsx | 63 +----------------------- 2 files changed, 3 insertions(+), 78 deletions(-) diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index 13815fd3efc..ccb25b58306 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -42,7 +42,6 @@ import type { AppData } from "./data"; import type { EntryContext, RemixContextObject } from "./entry"; import { RemixRootDefaultErrorBoundary, - RemixErrorBoundary, RemixRootDefaultCatchBoundary, RemixCatchBoundary, } from "./errorBoundaries"; @@ -151,7 +150,6 @@ export function RemixRouteError({ id }: { id: string }) { ); let error = useRouteError(); - let location = useLocation(); let { CatchBoundary, ErrorBoundary } = routeModules[id]; // POC for potential v2 error boundary handling @@ -177,13 +175,7 @@ export function RemixRouteError({ id }: { id: string }) { ErrorBoundary ) { // Internal framework-thrown ErrorResponses - return ( - - ); + return ; } if (CatchBoundary) { // User-thrown ErrorResponses @@ -198,13 +190,7 @@ export function RemixRouteError({ id }: { id: string }) { if (error instanceof Error && ErrorBoundary) { // User- or framework-thrown Errors - return ( - - ); + return ; } throw error; diff --git a/packages/remix-react/errorBoundaries.tsx b/packages/remix-react/errorBoundaries.tsx index f2f8916e860..a91d96f67f0 100644 --- a/packages/remix-react/errorBoundaries.tsx +++ b/packages/remix-react/errorBoundaries.tsx @@ -2,70 +2,9 @@ import React, { useContext } from "react"; import type { ErrorResponse } from "@remix-run/router"; import type { Location } from "react-router-dom"; -import type { - CatchBoundaryComponent, - ErrorBoundaryComponent, -} from "./routeModules"; +import type { CatchBoundaryComponent } from "./routeModules"; import type { ThrownResponse } from "./errors"; -type RemixErrorBoundaryProps = React.PropsWithChildren<{ - location: Location; - component: ErrorBoundaryComponent; - error?: Error; -}>; - -type RemixErrorBoundaryState = { - error: null | Error; - location: Location; -}; - -export class RemixErrorBoundary extends React.Component< - RemixErrorBoundaryProps, - RemixErrorBoundaryState -> { - constructor(props: RemixErrorBoundaryProps) { - super(props); - - this.state = { error: props.error || null, location: props.location }; - } - - static getDerivedStateFromError(error: Error) { - return { error }; - } - - static getDerivedStateFromProps( - props: RemixErrorBoundaryProps, - state: RemixErrorBoundaryState - ) { - // When we get into an error state, the user will likely click "back" to the - // previous page that didn't have an error. Because this wraps the entire - // application (even the HTML!) that will have no effect--the error page - // continues to display. This gives us a mechanism to recover from the error - // when the location changes. - // - // Whether we're in an error state or not, we update the location in state - // so that when we are in an error state, it gets reset when a new location - // comes in and the user recovers from the error. - if (state.location !== props.location) { - return { error: props.error || null, location: props.location }; - } - - // If we're not changing locations, preserve the location but still surface - // any new errors that may come through. We retain the existing error, we do - // this because the error provided from the app state may be cleared without - // the location changing. - return { error: props.error || state.error, location: state.location }; - } - - render() { - if (this.state.error) { - return ; - } else { - return this.props.children; - } - } -} - /** * When app's don't provide a root level ErrorBoundary, we default to this. */ From ae9c4c155b05a202166ec05edb45745663515616 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 19 Dec 2022 17:17:16 -0500 Subject: [PATCH 03/12] Add v2_errorBoundary future flag --- .changeset/odd-numbers-film.md | 28 +++ integration/error-boundary-v2-test.ts | 237 +++++++++++++++++++ packages/remix-dev/config.ts | 2 + packages/remix-react/components.tsx | 23 +- packages/remix-react/entry.ts | 1 + packages/remix-react/errorBoundaries.tsx | 27 ++- packages/remix-react/index.tsx | 2 + packages/remix-react/routeModules.ts | 9 +- packages/remix-server-runtime/entry.ts | 1 + packages/remix-server-runtime/server.ts | 8 +- packages/remix-testing/create-remix-stub.tsx | 3 +- 11 files changed, 327 insertions(+), 14 deletions(-) create mode 100644 .changeset/odd-numbers-film.md create mode 100644 integration/error-boundary-v2-test.ts diff --git a/.changeset/odd-numbers-film.md b/.changeset/odd-numbers-film.md new file mode 100644 index 00000000000..b3094e79309 --- /dev/null +++ b/.changeset/odd-numbers-film.md @@ -0,0 +1,28 @@ +--- +"@remix-run/react": patch +"@remix-run/server-runtime": patch +--- + +Add `future.v2_errorBoundary` flag to opt-into v2 `ErrorBoundary` behavior. This removes the separate `CatchBoundary` and `ErrorBoundary` and consolidates them into a single `ErrorBoundary` following the logic used by `errorElement` in React You can then use `isRouteErrorResponse` to differentiate between thrown `Response`/`Error` instances. + +```jsx +// Current (Remix v1 default) +export function CatchBoundary() { + let caught = useCatch(); + return

{caught.status} {caught.data}

; +} + +export function ErrorBoundary({ error }) { + return

{error.message}

; +} + + +// Using future.v2_errorBoundary +export function ErrorBoundary() { + let error = useRouteError(); + + return isRouteErrorResponse(error) ? +

{error.status} {error.data}

: +

{error.message}

; +} +``` \ No newline at end of file diff --git a/integration/error-boundary-v2-test.ts b/integration/error-boundary-v2-test.ts new file mode 100644 index 00000000000..bbacdfa3359 --- /dev/null +++ b/integration/error-boundary-v2-test.ts @@ -0,0 +1,237 @@ +import type { Page } from "@playwright/test"; +import { test, expect } from "@playwright/test"; +import { ServerMode } from "@remix-run/server-runtime/mode"; + +import { createAppFixture, createFixture, js } from "./helpers/create-fixture"; +import type { Fixture, AppFixture } from "./helpers/create-fixture"; +import { PlaywrightFixture } from "./helpers/playwright-fixture"; + +test.describe("V2 Singular ErrorBoundary (future.v2_errorBoundary)", () => { + let fixture: Fixture; + let appFixture: AppFixture; + let oldConsoleError: () => void; + + test.beforeAll(async () => { + fixture = await createFixture({ + future: { + v2_errorBoundary: true, + }, + files: { + "app/root.jsx": js` + import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; + + export default function Root() { + return ( + + + + + + +
+ +
+ + + + ); + } + `, + + "app/routes/parent.jsx": js` + import { + Link, + Outlet, + isRouteErrorResponse, + useLoaderData, + useRouteError, + } from "@remix-run/react"; + + export function loader() { + return "PARENT LOADER"; + } + + export default function Component() { + return ( +
+ +

{useLoaderData()}

+ +
+ ) + } + + export function ErrorBoundary() { + let error = useRouteError(); + return isRouteErrorResponse(error) ? +

{error.status + ' ' + error.data}

: +

{error.message}

; + } + `, + + "app/routes/parent/child-with-boundary.jsx": js` + import { + isRouteErrorResponse, + useLoaderData, + useLocation, + useRouteError, + } from "@remix-run/react"; + + export function loader({ request }) { + let errorType = new URL(request.url).searchParams.get('type'); + if (errorType === 'response') { + throw new Response('Loader Response', { status: 418 }); + } else if (errorType === 'error') { + throw new Error('Loader Error'); + } + return "CHILD LOADER"; + } + + export default function Component() {; + let data = useLoaderData(); + if (new URLSearchParams(useLocation().search).get('type') === "render") { + throw new Error("Render Error"); + } + return

{data}

; + } + + export function ErrorBoundary() { + let error = useRouteError(); + return isRouteErrorResponse(error) ? +

{error.status + ' ' + error.data}

: +

{error.message}

; + } + `, + + "app/routes/parent/child-without-boundary.jsx": js` + import { useLoaderData, useLocation } from "@remix-run/react"; + + export function loader({ request }) { + let errorType = new URL(request.url).searchParams.get('type'); + if (errorType === 'response') { + throw new Response('Loader Response', { status: 418 }); + } else if (errorType === 'error') { + throw new Error('Loader Error'); + } + return "CHILD LOADER"; + } + + export default function Component() {; + let data = useLoaderData(); + if (new URLSearchParams(useLocation().search).get('type') === "render") { + throw new Error("Render Error"); + } + return

{data}

; + } + `, + }, + }); + + appFixture = await createAppFixture(fixture, ServerMode.Development); + }); + + test.afterAll(() => { + appFixture.close(); + }); + + test.beforeEach(({ page }) => { + oldConsoleError = console.error; + console.error = () => {}; + }); + + test.afterEach(() => { + console.error = oldConsoleError; + }); + + test.describe("without JavaScript", () => { + test.use({ javaScriptEnabled: false }); + runBoundaryTests(); + }); + + test.describe("with JavaScript", () => { + test.use({ javaScriptEnabled: true }); + runBoundaryTests(); + }); + + function runBoundaryTests() { + // Shorthand util to wait for an element to appear before asserting it + async function waitForAndAssert( + page: Page, + app: PlaywrightFixture, + selector: string, + match: string + ) { + await page.waitForSelector(selector); + expect(await app.getHtml(selector)).toMatch(match); + } + + test("No errors", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/parent"); + await app.clickLink("/parent/child-with-boundary"); + await waitForAndAssert(page, app, "#child-data", "CHILD LOADER"); + }); + + test("Throwing a Response to own boundary", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/parent"); + await app.clickLink("/parent/child-with-boundary?type=response"); + await waitForAndAssert( + page, + app, + "#child-error-response", + "418 Loader Response" + ); + }); + + test("Throwing an Error to own boundary", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/parent"); + await app.clickLink("/parent/child-with-boundary?type=error"); + await waitForAndAssert(page, app, "#child-error", "Loader Error"); + }); + + test("Throwing a render error to own boundary", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/parent"); + await app.clickLink("/parent/child-with-boundary?type=render"); + await waitForAndAssert(page, app, "#child-error", "Render Error"); + }); + + test("Throwing a Response to parent boundary", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/parent"); + await app.clickLink("/parent/child-without-boundary?type=response"); + await waitForAndAssert( + page, + app, + "#parent-error-response", + "418 Loader Response" + ); + }); + + test("Throwing an Error to parent boundary", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/parent"); + await app.clickLink("/parent/child-without-boundary?type=error"); + await waitForAndAssert(page, app, "#parent-error", "Loader Error"); + }); + + test("Throwing a render error to parent boundary", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/parent"); + await app.clickLink("/parent/child-without-boundary?type=render"); + await waitForAndAssert(page, app, "#parent-error", "Render Error"); + }); + } +}); diff --git a/packages/remix-dev/config.ts b/packages/remix-dev/config.ts index 922a6247641..6dfbc487dd8 100644 --- a/packages/remix-dev/config.ts +++ b/packages/remix-dev/config.ts @@ -32,6 +32,7 @@ export type ServerModuleFormat = "esm" | "cjs"; export type ServerPlatform = "node" | "neutral"; interface FutureConfig { + v2_errorBoundary: boolean; v2_meta: boolean; } @@ -481,6 +482,7 @@ export async function readConfig( } let future = { + v2_errorBoundary: appConfig.future?.v2_errorBoundary === true, v2_meta: appConfig.future?.v2_meta === true, }; diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index ccb25b58306..f6c9c96f8e0 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -44,6 +44,7 @@ import { RemixRootDefaultErrorBoundary, RemixRootDefaultCatchBoundary, RemixCatchBoundary, + V2_RemixRootDefaultErrorBoundary, } from "./errorBoundaries"; import invariant from "./invariant"; import { @@ -140,7 +141,7 @@ export function RemixRoute({ id }: { id: string }) { } export function RemixRouteError({ id }: { id: string }) { - let { routeModules } = useRemixContext(); + let { future, routeModules } = useRemixContext(); // This checks prevent cryptic error messages such as: 'Cannot read properties of undefined (reading 'root')' invariant( @@ -152,14 +153,18 @@ export function RemixRouteError({ id }: { id: string }) { let error = useRouteError(); let { CatchBoundary, ErrorBoundary } = routeModules[id]; - // POC for potential v2 error boundary handling - // if (future.v2_errorBoundary) { - // // Provide defaults for the root route if they are not present - // if (id === "root") { - // ErrorBoundary ||= RemixRootDefaultNewErrorBoundary; - // } - // return - // } + if (future.v2_errorBoundary) { + // Provide defaults for the root route if they are not present + if (id === "root") { + ErrorBoundary ||= V2_RemixRootDefaultErrorBoundary; + } + if (ErrorBoundary) { + // TODO: Unsure if we can satisfy the typings here + // @ts-expect-error + return ; + } + throw error; + } // Provide defaults for the root route if they are not present if (id === "root") { diff --git a/packages/remix-react/entry.ts b/packages/remix-react/entry.ts index c062126e9bd..dc852f6e6f5 100644 --- a/packages/remix-react/entry.ts +++ b/packages/remix-react/entry.ts @@ -18,6 +18,7 @@ export interface EntryContext extends RemixContextObject { } export interface FutureConfig { + v2_errorBoundary: boolean; v2_meta: boolean; } diff --git a/packages/remix-react/errorBoundaries.tsx b/packages/remix-react/errorBoundaries.tsx index a91d96f67f0..d068d0ec92e 100644 --- a/packages/remix-react/errorBoundaries.tsx +++ b/packages/remix-react/errorBoundaries.tsx @@ -1,6 +1,6 @@ import React, { useContext } from "react"; import type { ErrorResponse } from "@remix-run/router"; -import type { Location } from "react-router-dom"; +import { isRouteErrorResponse, useRouteError } from "react-router-dom"; import type { CatchBoundaryComponent } from "./routeModules"; import type { ThrownResponse } from "./errors"; @@ -48,6 +48,23 @@ export function RemixRootDefaultErrorBoundary({ error }: { error: Error }) { ); } +export function V2_RemixRootDefaultErrorBoundary() { + let error = useRouteError(); + if (isRouteErrorResponse(error)) { + return ; + } else if (error instanceof Error) { + return ; + } else { + let errorString = + error == null + ? "Unknown Error" + : typeof error === "object" && "toString" in error + ? error.toString() + : JSON.stringify(error); + return ; + } +} + let RemixCatchContext = React.createContext( undefined ); @@ -89,6 +106,14 @@ export function RemixCatchBoundary({ */ export function RemixRootDefaultCatchBoundary() { let caught = useCatch(); + return ; +} + +function RemixRootDefaultCatchBoundaryImpl({ + caught, +}: { + caught: ThrownResponse; +}) { return ( diff --git a/packages/remix-react/index.tsx b/packages/remix-react/index.tsx index a571864c16f..64c326c3c82 100644 --- a/packages/remix-react/index.tsx +++ b/packages/remix-react/index.tsx @@ -12,6 +12,7 @@ export type { export { Form, Outlet, + isRouteErrorResponse, useBeforeUnload, useFormAction, useHref, @@ -24,6 +25,7 @@ export { useParams, useResolvedPath, useRevalidator, + useRouteError, useSearchParams, useSubmit, } from "react-router-dom"; diff --git a/packages/remix-react/routeModules.ts b/packages/remix-react/routeModules.ts index d7539a5e57b..dc7eb6c56a7 100644 --- a/packages/remix-react/routeModules.ts +++ b/packages/remix-react/routeModules.ts @@ -17,7 +17,7 @@ export interface RouteModules { export interface RouteModule { CatchBoundary?: CatchBoundaryComponent; - ErrorBoundary?: ErrorBoundaryComponent; + ErrorBoundary?: ErrorBoundaryComponent | V2_ErrorBoundaryComponent; default: RouteComponent; handle?: RouteHandle; links?: LinksFunction; @@ -43,6 +43,13 @@ export type CatchBoundaryComponent = ComponentType<{}>; */ export type ErrorBoundaryComponent = ComponentType<{ error: Error }>; +/** + * V2 version of the ErrorBoundary that eliminates the distinction between + * Error and Catch Boundaries and behaves like RR 6.4 errorElement and captures + * errors with useRouteError() + */ +export type V2_ErrorBoundaryComponent = ComponentType<{}>; + /** * A function that defines `` tags to be inserted into the `` of * the document on route transitions. diff --git a/packages/remix-server-runtime/entry.ts b/packages/remix-server-runtime/entry.ts index a97284b6916..be2d2056eb1 100644 --- a/packages/remix-server-runtime/entry.ts +++ b/packages/remix-server-runtime/entry.ts @@ -12,6 +12,7 @@ export interface EntryContext { } export interface FutureConfig { + v2_errorBoundary: boolean; v2_meta: boolean; } diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts index 449a685e4a5..39c26ca934d 100644 --- a/packages/remix-server-runtime/server.ts +++ b/packages/remix-server-runtime/server.ts @@ -230,7 +230,9 @@ async function handleDocumentRequestRR( } // Restructure context.errors to the right Catch/Error Boundary - differentiateCatchVersusErrorBoundaries(build, context); + if (build.future.v2_errorBoundary !== true) { + differentiateCatchVersusErrorBoundaries(build, context); + } let headers = getDocumentHeadersRR(build, context); @@ -270,7 +272,9 @@ async function handleDocumentRequestRR( ); // Restructure context.errors to the right Catch/Error Boundary - differentiateCatchVersusErrorBoundaries(build, context); + if (build.future.v2_errorBoundary !== true) { + differentiateCatchVersusErrorBoundaries(build, context); + } // Update entryContext for the second render pass entryContext = { diff --git a/packages/remix-testing/create-remix-stub.tsx b/packages/remix-testing/create-remix-stub.tsx index d2fb47495fc..b952795c791 100644 --- a/packages/remix-testing/create-remix-stub.tsx +++ b/packages/remix-testing/create-remix-stub.tsx @@ -145,7 +145,7 @@ function createRemixContext( let matches = matchRoutes(routes, currentLocation) || []; return { - // TODO: Check with Logan on how to handle the update heree + // TODO: Check with Logan on how to handle the update here // @ts-expect-error actionData: initialActionData, appState: { @@ -156,6 +156,7 @@ function createRemixContext( loaderBoundaryRouteId: null, }, future: { + v2_errorBoundary: false, v2_meta: false, ...future, }, From 13e5498dd44ef66f422431527fa3d8f9b979e9c9 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 19 Dec 2022 17:33:59 -0500 Subject: [PATCH 04/12] Adjust hasErrorBoundary logic --- packages/remix-react/browser.tsx | 3 ++- packages/remix-react/routes.tsx | 29 +++++++++++++++++++++---- packages/remix-react/server.tsx | 6 ++++- packages/remix-server-runtime/routes.ts | 15 ++++++++----- packages/remix-server-runtime/server.ts | 2 +- 5 files changed, 43 insertions(+), 12 deletions(-) diff --git a/packages/remix-react/browser.tsx b/packages/remix-react/browser.tsx index e2993283335..db53e8fe016 100644 --- a/packages/remix-react/browser.tsx +++ b/packages/remix-react/browser.tsx @@ -33,7 +33,8 @@ export function RemixBrowser(_props: RemixBrowserProps): ReactElement { if (!router) { let routes = createClientRoutes( window.__remixManifest.routes, - window.__remixRouteModules + window.__remixRouteModules, + window.__remixContext.future ); let hydrationData = window.__remixContext.state; diff --git a/packages/remix-react/routes.tsx b/packages/remix-react/routes.tsx index 15c237a7da3..196c678b447 100644 --- a/packages/remix-react/routes.tsx +++ b/packages/remix-react/routes.tsx @@ -13,6 +13,7 @@ import { fetchData, isCatchResponse, isRedirectResponse } from "./data"; import { prefetchStyleLinks } from "./links"; import invariant from "./invariant"; import { RemixRoute, RemixRouteError } from "./components"; +import { FutureConfig } from "./entry"; export interface RouteManifest { [routeId: string]: Route; @@ -41,13 +42,18 @@ export interface EntryRoute extends Route { export function createServerRoutes( manifest: RouteManifest, routeModules: RouteModules, + future: FutureConfig, parentId?: string ): DataRouteObject[] { return Object.values(manifest) .filter((route) => route.parentId === parentId) .map((route) => { let hasErrorBoundary = - route.id === "root" || route.hasErrorBoundary || route.hasCatchBoundary; + future.v2_errorBoundary === true + ? route.id === "root" || route.hasErrorBoundary + : route.id === "root" || + route.hasCatchBoundary || + route.hasErrorBoundary; let dataRoute: DataRouteObject = { caseSensitive: route.caseSensitive, element: , @@ -62,7 +68,12 @@ export function createServerRoutes( // since they're for a static render }; - let children = createServerRoutes(manifest, routeModules, route.id); + let children = createServerRoutes( + manifest, + routeModules, + future, + route.id + ); if (children.length > 0) dataRoute.children = children; return dataRoute; }); @@ -71,13 +82,18 @@ export function createServerRoutes( export function createClientRoutes( manifest: RouteManifest, routeModulesCache: RouteModules, + future: FutureConfig, parentId?: string ): DataRouteObject[] { return Object.values(manifest) .filter((entryRoute) => entryRoute.parentId === parentId) .map((route) => { let hasErrorBoundary = - route.id === "root" || route.hasErrorBoundary || route.hasCatchBoundary; + future.v2_errorBoundary === true + ? route.id === "root" || route.hasErrorBoundary + : route.id === "root" || + route.hasCatchBoundary || + route.hasErrorBoundary; let dataRoute: DataRouteObject = { caseSensitive: route.caseSensitive, @@ -95,7 +111,12 @@ export function createClientRoutes( action: createDataFunction(route, routeModulesCache, true), shouldRevalidate: createShouldRevalidate(route, routeModulesCache), }; - let children = createClientRoutes(manifest, routeModulesCache, route.id); + let children = createClientRoutes( + manifest, + routeModulesCache, + future, + route.id + ); if (children.length > 0) dataRoute.children = children; return dataRoute; }); diff --git a/packages/remix-react/server.tsx b/packages/remix-react/server.tsx index 2525ee24945..07b48c51733 100644 --- a/packages/remix-react/server.tsx +++ b/packages/remix-react/server.tsx @@ -25,7 +25,11 @@ export function RemixServer({ context, url }: RemixServerProps): ReactElement { } let { manifest, routeModules, serverHandoffString } = context; - let routes = createServerRoutes(manifest.routes, routeModules); + let routes = createServerRoutes( + manifest.routes, + routeModules, + context.future + ); let router = createStaticRouter(routes, context.staticHandlerContext); return ( diff --git a/packages/remix-server-runtime/routes.ts b/packages/remix-server-runtime/routes.ts index 19a48e589f4..b08d5d84281 100644 --- a/packages/remix-server-runtime/routes.ts +++ b/packages/remix-server-runtime/routes.ts @@ -5,6 +5,7 @@ import type { } from "@remix-run/router"; import { callRouteActionRR, callRouteLoaderRR } from "./data"; +import { FutureConfig } from "./entry"; import type { ServerRouteModule } from "./routeModules"; export interface RouteManifest { @@ -54,17 +55,21 @@ export function createRoutes( // createStaticHandler export function createStaticHandlerDataRoutes( manifest: ServerRouteManifest, + future: FutureConfig, parentId?: string ): AgnosticDataRouteObject[] { return Object.values(manifest) .filter((route) => route.parentId === parentId) .map((route) => { + let hasErrorBoundary = + future.v2_errorBoundary === true + ? route.id === "root" || route.module.ErrorBoundary != null + : route.id === "root" || + route.module.CatchBoundary != null || + route.module.ErrorBoundary != null; let commonRoute = { // Always include root due to default boundaries - hasErrorBoundary: - route.id === "root" || - route.module.CatchBoundary != null || - route.module.ErrorBoundary != null, + hasErrorBoundary, id: route.id, path: route.path, loader: route.module.loader @@ -97,7 +102,7 @@ export function createStaticHandlerDataRoutes( } : { caseSensitive: route.caseSensitive, - children: createStaticHandlerDataRoutes(manifest, route.id), + children: createStaticHandlerDataRoutes(manifest, future, route.id), ...commonRoute, }; }); diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts index 39c26ca934d..a75dca78049 100644 --- a/packages/remix-server-runtime/server.ts +++ b/packages/remix-server-runtime/server.ts @@ -34,7 +34,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( mode ) => { let routes = createRoutes(build.routes); - let dataRoutes = createStaticHandlerDataRoutes(build.routes); + let dataRoutes = createStaticHandlerDataRoutes(build.routes, build.future); let serverMode = isServerMode(mode) ? mode : ServerMode.Production; let staticHandler = createStaticHandler(dataRoutes); From 3c9db851ebd5b7f3af6e12ff925a2c594ef6049d Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 19 Dec 2022 17:43:12 -0500 Subject: [PATCH 05/12] fix TS error --- integration/helpers/create-fixture.ts | 2 +- packages/remix-react/routes.tsx | 2 +- packages/remix-server-runtime/routes.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/integration/helpers/create-fixture.ts b/integration/helpers/create-fixture.ts index d479dfb789a..204fa5686cc 100644 --- a/integration/helpers/create-fixture.ts +++ b/integration/helpers/create-fixture.ts @@ -8,7 +8,7 @@ import { sync as spawnSync } from "cross-spawn"; import type { JsonObject } from "type-fest"; import type { ServerMode } from "@remix-run/server-runtime/mode"; -import type { AppConfig } from "../../build/node_modules/@remix-run/dev"; +import type { AppConfig } from "../../build/node_modules/@remix-run/dev/dist/config"; import type { ServerBuild } from "../../build/node_modules/@remix-run/server-runtime"; import { createRequestHandler } from "../../build/node_modules/@remix-run/server-runtime"; import { createRequestHandler as createExpressHandler } from "../../build/node_modules/@remix-run/express"; diff --git a/packages/remix-react/routes.tsx b/packages/remix-react/routes.tsx index 196c678b447..0b342f808fa 100644 --- a/packages/remix-react/routes.tsx +++ b/packages/remix-react/routes.tsx @@ -10,10 +10,10 @@ import { redirect } from "react-router-dom"; import type { RouteModules } from "./routeModules"; import { loadRouteModule } from "./routeModules"; import { fetchData, isCatchResponse, isRedirectResponse } from "./data"; +import type { FutureConfig } from "./entry"; import { prefetchStyleLinks } from "./links"; import invariant from "./invariant"; import { RemixRoute, RemixRouteError } from "./components"; -import { FutureConfig } from "./entry"; export interface RouteManifest { [routeId: string]: Route; diff --git a/packages/remix-server-runtime/routes.ts b/packages/remix-server-runtime/routes.ts index b08d5d84281..6c63ae870b1 100644 --- a/packages/remix-server-runtime/routes.ts +++ b/packages/remix-server-runtime/routes.ts @@ -5,7 +5,7 @@ import type { } from "@remix-run/router"; import { callRouteActionRR, callRouteLoaderRR } from "./data"; -import { FutureConfig } from "./entry"; +import type { FutureConfig } from "./entry"; import type { ServerRouteModule } from "./routeModules"; export interface RouteManifest { From 8b5c813126dd6889f1490adf370808bd7051b88b Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 20 Dec 2022 10:51:31 -0500 Subject: [PATCH 06/12] fix TS error --- integration/helpers/create-fixture.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration/helpers/create-fixture.ts b/integration/helpers/create-fixture.ts index 204fa5686cc..4238d6a3b75 100644 --- a/integration/helpers/create-fixture.ts +++ b/integration/helpers/create-fixture.ts @@ -7,8 +7,8 @@ import stripIndent from "strip-indent"; import { sync as spawnSync } from "cross-spawn"; import type { JsonObject } from "type-fest"; import type { ServerMode } from "@remix-run/server-runtime/mode"; +import type { FutureConfig } from "@remix-run/server-runtime/entry"; -import type { AppConfig } from "../../build/node_modules/@remix-run/dev/dist/config"; import type { ServerBuild } from "../../build/node_modules/@remix-run/server-runtime"; import { createRequestHandler } from "../../build/node_modules/@remix-run/server-runtime"; import { createRequestHandler as createExpressHandler } from "../../build/node_modules/@remix-run/express"; @@ -21,7 +21,7 @@ interface FixtureInit { files?: { [filename: string]: string }; template?: "cf-template" | "deno-template" | "node-template"; setup?: "node" | "cloudflare"; - future?: AppConfig["future"]; + future?: Partial; } export type Fixture = Awaited>; From ffe39c135f8e7b7a4fc499991d594ca2b3d39e8f Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 20 Dec 2022 12:47:03 -0500 Subject: [PATCH 07/12] Update .changeset/odd-numbers-film.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sergio Xalambrí --- .changeset/odd-numbers-film.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/odd-numbers-film.md b/.changeset/odd-numbers-film.md index b3094e79309..bbb9f468f7a 100644 --- a/.changeset/odd-numbers-film.md +++ b/.changeset/odd-numbers-film.md @@ -3,7 +3,7 @@ "@remix-run/server-runtime": patch --- -Add `future.v2_errorBoundary` flag to opt-into v2 `ErrorBoundary` behavior. This removes the separate `CatchBoundary` and `ErrorBoundary` and consolidates them into a single `ErrorBoundary` following the logic used by `errorElement` in React You can then use `isRouteErrorResponse` to differentiate between thrown `Response`/`Error` instances. +Add `future.v2_errorBoundary` flag to opt-into v2 `ErrorBoundary` behavior. This removes the separate `CatchBoundary` and `ErrorBoundary` and consolidates them into a single `ErrorBoundary` following the logic used by `errorElement` in React Router. You can then use `isRouteErrorResponse` to differentiate between thrown `Response`/`Error` instances. ```jsx // Current (Remix v1 default) From ffc6ffee0e238b5e5699ecf7fa90fea6b962f6fd Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 20 Dec 2022 12:47:22 -0500 Subject: [PATCH 08/12] Update .changeset/odd-numbers-film.md Co-authored-by: Mehdi Achour --- .changeset/odd-numbers-film.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.changeset/odd-numbers-film.md b/.changeset/odd-numbers-film.md index bbb9f468f7a..7779f5c7ebe 100644 --- a/.changeset/odd-numbers-film.md +++ b/.changeset/odd-numbers-film.md @@ -18,6 +18,8 @@ export function ErrorBoundary({ error }) { // Using future.v2_errorBoundary +import { isRouteErrorResponse, useRouteError } from "@remix-run/react"; + export function ErrorBoundary() { let error = useRouteError(); From 6de4119dc3b80330fa0b113b4585298a38049b05 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 20 Dec 2022 15:36:36 -0500 Subject: [PATCH 09/12] Fix up unit tests that were not putting a future in the build --- packages/remix-dev/__tests__/readConfig-test.ts | 2 ++ packages/remix-server-runtime/__tests__/data-test.ts | 12 ++++++++---- .../remix-server-runtime/__tests__/handler-test.ts | 4 ++++ .../remix-server-runtime/__tests__/server-test.ts | 1 + packages/remix-server-runtime/__tests__/utils.ts | 1 + 5 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/remix-dev/__tests__/readConfig-test.ts b/packages/remix-dev/__tests__/readConfig-test.ts index a6cdea1f44e..c20081c5e06 100644 --- a/packages/remix-dev/__tests__/readConfig-test.ts +++ b/packages/remix-dev/__tests__/readConfig-test.ts @@ -23,6 +23,7 @@ describe("readConfig", () => { relativeAssetsBuildDirectory: expect.any(String), tsconfigPath: expect.any(String), future: { + v2_errorBoundary: expect.any(Boolean), v2_meta: expect.any(Boolean), }, }, @@ -36,6 +37,7 @@ describe("readConfig", () => { "entryClientFile": "entry.client.tsx", "entryServerFile": "entry.server.tsx", "future": Object { + "v2_errorBoundary": Any, "v2_meta": Any, }, "mdx": undefined, diff --git a/packages/remix-server-runtime/__tests__/data-test.ts b/packages/remix-server-runtime/__tests__/data-test.ts index fca71a1f9fe..5b64f3c7118 100644 --- a/packages/remix-server-runtime/__tests__/data-test.ts +++ b/packages/remix-server-runtime/__tests__/data-test.ts @@ -21,9 +21,10 @@ describe("loaders", () => { }, }, entry: { module: {} }, + future: {}, } as unknown as ServerBuild; - let handler = createRequestHandler(build, {}); + let handler = createRequestHandler(build); let request = new Request( "http://example.com/random?_data=routes/random&foo=bar", @@ -59,9 +60,10 @@ describe("loaders", () => { }, }, entry: { module: {} }, + future: {}, } as unknown as ServerBuild; - let handler = createRequestHandler(build, {}); + let handler = createRequestHandler(build); let request = new Request( "http://example.com/random?_data=routes/random&foo=bar", @@ -93,9 +95,10 @@ describe("loaders", () => { }, }, entry: { module: {} }, + future: {}, } as unknown as ServerBuild; - let handler = createRequestHandler(build, {}); + let handler = createRequestHandler(build); let request = new Request( "http://example.com/random?_data=routes/random&index&foo=bar", @@ -127,9 +130,10 @@ describe("loaders", () => { }, }, entry: { module: {} }, + future: {}, } as unknown as ServerBuild; - let handler = createRequestHandler(build, {}); + let handler = createRequestHandler(build); let request = new Request( "http://example.com/random?_data=routes/random&index&foo=bar&index=test", diff --git a/packages/remix-server-runtime/__tests__/handler-test.ts b/packages/remix-server-runtime/__tests__/handler-test.ts index bb676a8e9d3..8b0e05688ef 100644 --- a/packages/remix-server-runtime/__tests__/handler-test.ts +++ b/packages/remix-server-runtime/__tests__/handler-test.ts @@ -16,6 +16,10 @@ describe("createRequestHandler", () => { }, assets: {} as any, entry: { module: {} as any }, + future: { + v2_errorBoundary: false, + v2_meta: false, + }, }); let response = await handler( diff --git a/packages/remix-server-runtime/__tests__/server-test.ts b/packages/remix-server-runtime/__tests__/server-test.ts index 6834bd4b4bd..e4d1951728d 100644 --- a/packages/remix-server-runtime/__tests__/server-test.ts +++ b/packages/remix-server-runtime/__tests__/server-test.ts @@ -55,6 +55,7 @@ describe("server", () => { }, }, }, + future: {}, } as unknown as ServerBuild; describe("createRequestHandler", () => { diff --git a/packages/remix-server-runtime/__tests__/utils.ts b/packages/remix-server-runtime/__tests__/utils.ts index 9a75e5ce5c2..21223d7d5d0 100644 --- a/packages/remix-server-runtime/__tests__/utils.ts +++ b/packages/remix-server-runtime/__tests__/utils.ts @@ -80,6 +80,7 @@ export function mockServerBuild( }, {} ), + future: {}, }; } From 475a6b0722ca7368a028e1201abb6babae44ef5d Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 20 Dec 2022 15:40:46 -0500 Subject: [PATCH 10/12] Update changelog --- .changeset/odd-numbers-film.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.changeset/odd-numbers-film.md b/.changeset/odd-numbers-film.md index 7779f5c7ebe..1e129194f69 100644 --- a/.changeset/odd-numbers-film.md +++ b/.changeset/odd-numbers-film.md @@ -7,6 +7,8 @@ Add `future.v2_errorBoundary` flag to opt-into v2 `ErrorBoundary` behavior. Thi ```jsx // Current (Remix v1 default) +import { useCatch } from "@remix-run/react"; + export function CatchBoundary() { let caught = useCatch(); return

{caught.status} {caught.data}

; From 2cf1d0751082b199317529f600aee72a91ed0579 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 11 Jan 2023 17:40:23 -0500 Subject: [PATCH 11/12] Remove unneeded generic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michaël De Boey --- packages/remix-react/routeModules.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/remix-react/routeModules.ts b/packages/remix-react/routeModules.ts index 04f461536ad..454b33cc4c5 100644 --- a/packages/remix-react/routeModules.ts +++ b/packages/remix-react/routeModules.ts @@ -48,7 +48,7 @@ export type ErrorBoundaryComponent = ComponentType<{ error: Error }>; * Error and Catch Boundaries and behaves like RR 6.4 errorElement and captures * errors with useRouteError() */ -export type V2_ErrorBoundaryComponent = ComponentType<{}>; +export type V2_ErrorBoundaryComponent = ComponentType; /** * A function that defines `` tags to be inserted into the `` of From 9aa0dc1edc70ab219d70d5d0c8568886afc7eb32 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 11 Jan 2023 17:44:30 -0500 Subject: [PATCH 12/12] Add back in Remix wrapper Error boundary --- packages/remix-react/errorBoundaries.tsx | 65 +++++++++++++++++++++++- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/packages/remix-react/errorBoundaries.tsx b/packages/remix-react/errorBoundaries.tsx index d068d0ec92e..7b1419dfd55 100644 --- a/packages/remix-react/errorBoundaries.tsx +++ b/packages/remix-react/errorBoundaries.tsx @@ -1,10 +1,71 @@ import React, { useContext } from "react"; -import type { ErrorResponse } from "@remix-run/router"; +import type { ErrorResponse, Location } from "@remix-run/router"; import { isRouteErrorResponse, useRouteError } from "react-router-dom"; -import type { CatchBoundaryComponent } from "./routeModules"; +import type { + CatchBoundaryComponent, + ErrorBoundaryComponent, +} from "./routeModules"; import type { ThrownResponse } from "./errors"; +type RemixErrorBoundaryProps = React.PropsWithChildren<{ + location: Location; + component: ErrorBoundaryComponent; + error?: Error; +}>; + +type RemixErrorBoundaryState = { + error: null | Error; + location: Location; +}; + +export class RemixErrorBoundary extends React.Component< + RemixErrorBoundaryProps, + RemixErrorBoundaryState +> { + constructor(props: RemixErrorBoundaryProps) { + super(props); + + this.state = { error: props.error || null, location: props.location }; + } + + static getDerivedStateFromError(error: Error) { + return { error }; + } + + static getDerivedStateFromProps( + props: RemixErrorBoundaryProps, + state: RemixErrorBoundaryState + ) { + // When we get into an error state, the user will likely click "back" to the + // previous page that didn't have an error. Because this wraps the entire + // application (even the HTML!) that will have no effect--the error page + // continues to display. This gives us a mechanism to recover from the error + // when the location changes. + // + // Whether we're in an error state or not, we update the location in state + // so that when we are in an error state, it gets reset when a new location + // comes in and the user recovers from the error. + if (state.location !== props.location) { + return { error: props.error || null, location: props.location }; + } + + // If we're not changing locations, preserve the location but still surface + // any new errors that may come through. We retain the existing error, we do + // this because the error provided from the app state may be cleared without + // the location changing. + return { error: props.error || state.error, location: state.location }; + } + + render() { + if (this.state.error) { + return ; + } else { + return this.props.children; + } + } +} + /** * When app's don't provide a root level ErrorBoundary, we default to this. */