Skip to content

Commit

Permalink
Add future.v2_errorBoundary flag (#4918)
Browse files Browse the repository at this point in the history
- Add ability for integration tests to specify future flags
- Remove duplicate RemixErrorBoundary
- Add v2_errorBoundary future flag

Co-authored-by: Sergio Xalambrí <[email protected]>
Co-authored-by: Mehdi Achour <[email protected]>
Co-authored-by: Michaël De Boey <[email protected]>
  • Loading branch information
4 people authored Jan 13, 2023
1 parent 809f519 commit ef66307
Show file tree
Hide file tree
Showing 22 changed files with 415 additions and 47 deletions.
32 changes: 32 additions & 0 deletions .changeset/odd-numbers-film.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
"@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 Router. You can then use `isRouteErrorResponse` to differentiate between thrown `Response`/`Error` instances.

```jsx
// Current (Remix v1 default)
import { useCatch } from "@remix-run/react";

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
import { isRouteErrorResponse, useRouteError } from "@remix-run/react";

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 @@ -7,6 +7,7 @@ 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 { ServerBuild } from "../../build/node_modules/@remix-run/server-runtime";
import { createRequestHandler } from "../../build/node_modules/@remix-run/server-runtime";
Expand All @@ -20,6 +21,7 @@ interface FixtureInit {
files?: { [filename: string]: string };
template?: "cf-template" | "deno-template" | "node-template";
setup?: "node" | "cloudflare";
future?: Partial<FutureConfig>;
}

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);
}

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/__tests__/readConfig-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe("readConfig", () => {
unstable_cssModules: expect.any(Boolean),
unstable_cssSideEffectImports: expect.any(Boolean),
unstable_vanillaExtract: expect.any(Boolean),
v2_errorBoundary: expect.any(Boolean),
v2_meta: expect.any(Boolean),
v2_routeConvention: expect.any(Boolean),
},
Expand All @@ -43,6 +44,7 @@ describe("readConfig", () => {
"unstable_cssModules": Any<Boolean>,
"unstable_cssSideEffectImports": Any<Boolean>,
"unstable_vanillaExtract": Any<Boolean>,
"v2_errorBoundary": Any<Boolean>,
"v2_meta": Any<Boolean>,
"v2_routeConvention": Any<Boolean>,
},
Expand Down
2 changes: 2 additions & 0 deletions packages/remix-dev/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ interface FutureConfig {
unstable_cssModules: boolean;
unstable_cssSideEffectImports: boolean;
unstable_vanillaExtract: boolean;
v2_errorBoundary: boolean;
v2_meta: boolean;
v2_routeConvention: boolean;
}
Expand Down Expand Up @@ -495,6 +496,7 @@ export async function readConfig(
unstable_cssSideEffectImports:
appConfig.future?.unstable_cssSideEffectImports === true,
unstable_vanillaExtract: appConfig.future?.unstable_vanillaExtract === true,
v2_errorBoundary: appConfig.future?.v2_errorBoundary === true,
v2_meta: appConfig.future?.v2_meta === true,
v2_routeConvention: appConfig.future?.v2_routeConvention === 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 @@ -39,7 +39,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
Loading

0 comments on commit ef66307

Please sign in to comment.