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.

Sign up for GitHub

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

Jump to bottom

fix(remix-react): An error in MetaFunction or LinkFunction should render ErrorBoundary #3674

Closed
wants to merge 12 commits into from
Closed

fix(remix-react): An error in MetaFunction or LinkFunction should render ErrorBoundary #3674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b0fecce
test: add test for throwing an error in meta function and links function
mcansh Jul 7, 2022
d1adfe4
test: add test for not having an error boundary in app
mcansh Jul 8, 2022
23ec1e6
chore: indent or i'll go mad
mcansh Jul 8, 2022
bf294b9
test: update tests
mcansh Feb 3, 2023
aae7c22
make it mostly work
mcansh Feb 3, 2023
7fc50e0
chore: update pathname
mcansh Feb 3, 2023
3a0d5fb
chore: v2 meta docs exist
mcansh Feb 3, 2023
6f06210
chore: clean up getRenderableMatches
mcansh Feb 3, 2023
63989e8
chore: getRenderableMatches in headers
mcansh Feb 3, 2023
4d184e8
chore: indent
mcansh Feb 3, 2023
d819080
chore: clean up
mcansh Feb 3, 2023
6e57656
thanks, i hate it...
mcansh Feb 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
230 changes: 217 additions & 13 deletions integration/error-boundary-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ 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";
import { PlaywrightFixture, selectHtml } from "./helpers/playwright-fixture";

test.describe("ErrorBoundary", () => {
let fixture: Fixture;
Expand Down Expand Up @@ -90,18 +90,26 @@ test.describe("ErrorBoundary", () => {
</button>
</Form>

<Link to="${HAS_BOUNDARY_LOADER}">
${HAS_BOUNDARY_LOADER}
</Link>
<Link to="${NO_BOUNDARY_LOADER}">
${NO_BOUNDARY_LOADER}
</Link>
<Link to="${HAS_BOUNDARY_RENDER}">
${HAS_BOUNDARY_RENDER}
</Link>
<Link to="${NO_BOUNDARY_RENDER}">
${NO_BOUNDARY_RENDER}
</Link>
<p>
<Link to="${HAS_BOUNDARY_LOADER}">
${HAS_BOUNDARY_LOADER}
</Link>
</p>
<p>
<Link to="${NO_BOUNDARY_LOADER}">
${NO_BOUNDARY_LOADER}
</Link>
</p>
<p>
<Link to="${HAS_BOUNDARY_RENDER}">
${HAS_BOUNDARY_RENDER}
</Link>
</p>
<p>
<Link to="${NO_BOUNDARY_RENDER}">
${NO_BOUNDARY_RENDER}
</Link>
</p>
</div>
)
}
Expand Down Expand Up @@ -478,6 +486,8 @@ test.describe("ErrorBoundary", () => {
let NO_ROOT_BOUNDARY_ACTION = "/action-bad";
let NO_ROOT_BOUNDARY_LOADER_RETURN = "/loader-no-return";
let NO_ROOT_BOUNDARY_ACTION_RETURN = "/action-no-return";
let META_ERROR = "/meta-error";
let LINKS_ERROR = "/links-error";

test.beforeAll(async () => {
fixture = await createFixture({
Expand Down Expand Up @@ -517,6 +527,16 @@ test.describe("ErrorBoundary", () => {
Action no return
</button>
</Form>
<p>
<Link to="${META_ERROR}">
${META_ERROR}
</Link>
</p>
<p>
<Link to="${LINKS_ERROR}">
${LINKS_ERROR}
</Link>
</p>
</div>
)
}
Expand Down Expand Up @@ -579,6 +599,26 @@ test.describe("ErrorBoundary", () => {
)
}
`,

[`app/routes${LINKS_ERROR}.jsx`]: js`
export function links() {
throw new Error("Kaboom!")
}

export default function () {
return <div/>
}
`,

[`app/routes${META_ERROR}.jsx`]: js`
export function meta() {
throw new Error("Kaboom!")
}

export default function () {
return <div/>
}
`,
},
});
appFixture = await createAppFixture(fixture);
Expand Down Expand Up @@ -635,6 +675,170 @@ test.describe("ErrorBoundary", () => {
await page.waitForSelector(`text=${INTERNAL_ERROR_BOUNDARY_HEADING}`);
expect(await app.getHtml("h1")).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
});

test("ssr renders error boundary when meta throws", async () => {
let res = await fixture.requestDocument(META_ERROR);
expect(res.status).toBe(500);
expect(selectHtml(await res.text(), "h1")).toMatch(
INTERNAL_ERROR_BOUNDARY_HEADING
);
});

test("script transition renders error boundary when meta throws", async ({
page,
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
await app.clickLink(META_ERROR);
expect(await app.getHtml("h1")).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
});

test("ssr renders error boundary when links function throws", async () => {
let res = await fixture.requestDocument(LINKS_ERROR);
expect(res.status).toBe(500);
expect(selectHtml(await res.text(), "h1")).toMatch(
INTERNAL_ERROR_BOUNDARY_HEADING
);
});

test("script transition renders error boundary when links function throws", async ({
page,
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
await app.clickLink(LINKS_ERROR);
expect(await app.getHtml("h1")).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
});
});

test.describe("errors in meta and links don't result in internal server error", () => {
let ROOT_BOUNDARY_TEXT = "ROOT_BOUNDARY_TEXT";
let META_ERROR = "/meta-error";
let LINKS_ERROR = "/links-error";

test.beforeAll(async () => {
_consoleError = console.error;
console.error = () => {};
fixture = await createFixture({
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>
);
}

export function ErrorBoundary() {
return (
<html>
<head>
<Meta />
<Links />
</head>
<body>
<main>
<div id="root-boundary">${ROOT_BOUNDARY_TEXT}</div>
</main>
<Scripts />
</body>
</html>
)
}
`,

"app/routes/index.jsx": js`
import { Link, Form } from "@remix-run/react";
export default function () {
return (
<div>
<p>
<Link to="${META_ERROR}">
${META_ERROR}
</Link>
</p>
<p>
<Link to="${LINKS_ERROR}">
${LINKS_ERROR}
</Link>
</p>
</div>
)
}
`,

[`app/routes${LINKS_ERROR}.jsx`]: js`
export function links() {
throw new Error("Kaboom!")
}

export default function () {
return <div/>
}
`,

[`app/routes${META_ERROR}.jsx`]: js`
export function meta() {
throw new Error("Kaboom!")
}

export default function () {
return <div/>
}
`,
},
});
appFixture = await createAppFixture(fixture, ServerMode.Development);
});

test.afterAll(() => {
console.error = _consoleError;
appFixture.close();
});

test("ssr renders error boundary when meta throws", async () => {
let res = await fixture.requestDocument(META_ERROR);
expect(res.status).toBe(500);
expect(await res.text()).toMatch(ROOT_BOUNDARY_TEXT);
});

test("script transition renders error boundary when meta throws", async ({
page,
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
await app.clickLink(META_ERROR);
expect(await app.getHtml("main")).toMatch(ROOT_BOUNDARY_TEXT);
await page.reload();
expect(await app.getHtml("main")).toMatch(ROOT_BOUNDARY_TEXT);
});

test("ssr renders error boundary when links function throws", async () => {
let res = await fixture.requestDocument(LINKS_ERROR);
expect(res.status).toBe(500);
expect(await res.text()).toMatch(ROOT_BOUNDARY_TEXT);
});

test("script transition renders error boundary when links function throws", async ({
page,
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
await app.clickLink(LINKS_ERROR);
expect(await app.getHtml("main")).toMatch(ROOT_BOUNDARY_TEXT);
});
});
});

Expand Down
15 changes: 6 additions & 9 deletions integration/helpers/create-fixture.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import getPort from "get-port";
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 { 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";
Expand Down Expand Up @@ -92,7 +92,10 @@ export async function createFixture(init: FixtureInit) {
};
}

export async function createAppFixture(fixture: Fixture, mode?: ServerMode) {
export async function createAppFixture(
fixture: Fixture,
mode: ServerMode = ServerMode.Production
) {
let startAppServer = async (): Promise<{
port: number;
stop: VoidFunction;
Expand All @@ -102,13 +105,7 @@ export async function createAppFixture(fixture: Fixture, mode?: ServerMode) {
let app = express();
app.use(express.static(path.join(fixture.projectDir, "public")));

app.all(
"*",
createExpressHandler({
build: fixture.build,
mode: mode || "production",
})
);
app.all("*", createExpressHandler({ build: fixture.build, mode }));

let server = app.listen(port);

Expand Down
Loading