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

Add future.v2_errorBoundary flag #4918

Merged
merged 13 commits into from
Jan 13, 2023
28 changes: 28 additions & 0 deletions .changeset/odd-numbers-film.md
Original file line number Diff line number Diff line change
@@ -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.
brophdawg11 marked this conversation as resolved.
Show resolved Hide resolved

```jsx
// Current (Remix v1 default)
export function CatchBoundary() {
let caught = useCatch();
return <p>{caught.status} {caught.data}</p>;
}

export function ErrorBoundary({ error }) {
return <p>{error.message}</p>;
}


// Using future.v2_errorBoundary
brophdawg11 marked this conversation as resolved.
Show resolved Hide resolved
export function ErrorBoundary() {
let error = useRouteError();

return isRouteErrorResponse(error) ?
<p>{error.status} {error.data}</p> :
<p>{error.message}</p>;
}
```
237 changes: 237 additions & 0 deletions integration/error-boundary-v2-test.ts
Original file line number Diff line number Diff line change
@@ -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 (
<html lang="en">
<head>
<Meta />
<Links />
</head>
<body>
<main>
<Outlet />
</main>
<Scripts />
</body>
</html>
);
}
`,

"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 (
<div>
<nav>
<ul>
<li><Link to="/parent/child-with-boundary">Link</Link></li>
<li><Link to="/parent/child-with-boundary?type=error">Link</Link></li>
<li><Link to="/parent/child-with-boundary?type=response">Link</Link></li>
<li><Link to="/parent/child-with-boundary?type=render">Link</Link></li>
<li><Link to="/parent/child-without-boundary?type=error">Link</Link></li>
<li><Link to="/parent/child-without-boundary?type=response">Link</Link></li>
<li><Link to="/parent/child-without-boundary?type=render">Link</Link></li>
</ul>
</nav>
<p id="parent-data">{useLoaderData()}</p>
<Outlet />
</div>
)
}

export function ErrorBoundary() {
let error = useRouteError();
return isRouteErrorResponse(error) ?
<p id="parent-error-response">{error.status + ' ' + error.data}</p> :
<p id="parent-error">{error.message}</p>;
}
`,

"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 <p id="child-data">{data}</p>;
}

export function ErrorBoundary() {
let error = useRouteError();
return isRouteErrorResponse(error) ?
<p id="child-error-response">{error.status + ' ' + error.data}</p> :
<p id="child-error">{error.message}</p>;
}
`,

"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 <p id="child-data">{data}</p>;
}
`,
},
});

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");
});
}
});
18 changes: 18 additions & 0 deletions integration/helpers/create-fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/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";
Expand All @@ -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<ReturnType<typeof createFixture>>;
Expand Down Expand Up @@ -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);
}
Comment on lines +180 to +193
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you pass a future config into createFixtureProject we'll write it into the remix config here


await writeTestFiles(init, projectDir);
build(projectDir, init.buildStdio, init.sourcemap);

Expand Down
4 changes: 4 additions & 0 deletions integration/helpers/node-template/remix.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {},
};
2 changes: 2 additions & 0 deletions packages/remix-dev/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export type ServerModuleFormat = "esm" | "cjs";
export type ServerPlatform = "node" | "neutral";

interface FutureConfig {
v2_errorBoundary: boolean;
v2_meta: boolean;
}

Expand Down Expand Up @@ -481,6 +482,7 @@ export async function readConfig(
}

let future = {
v2_errorBoundary: appConfig.future?.v2_errorBoundary === true,
v2_meta: appConfig.future?.v2_meta === true,
};

Expand Down
3 changes: 2 additions & 1 deletion packages/remix-react/browser.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down