From c615f50c29e1e0d54ce5b9fb3289038a6d10af11 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 20 Apr 2023 17:53:17 -0400 Subject: [PATCH 1/2] Fix false-positive resource route identification if a route only exports a boundary --- .changeset/resource-route-boundary.md | 5 +++ integration/resource-routes-test.ts | 65 +++++++++++++++++++++++++++ packages/remix-react/components.tsx | 13 +++++- 3 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 .changeset/resource-route-boundary.md diff --git a/.changeset/resource-route-boundary.md b/.changeset/resource-route-boundary.md new file mode 100644 index 00000000000..945cf193289 --- /dev/null +++ b/.changeset/resource-route-boundary.md @@ -0,0 +1,5 @@ +--- +"@remix-run/react": patch +--- + +Fix false-positive resource route identification if a route only exports a boundary diff --git a/integration/resource-routes-test.ts b/integration/resource-routes-test.ts index 7a662245550..c4c0243e714 100644 --- a/integration/resource-routes-test.ts +++ b/integration/resource-routes-test.ts @@ -191,3 +191,68 @@ test.describe("loader in an app", async () => { ); }); }); + +test.describe("Development server", async () => { + let appFixture: AppFixture; + let fixture: Fixture; + let _consoleError: typeof console.error; + + test.beforeAll(async () => { + _consoleError = console.error; + console.error = () => {}; + + fixture = await createFixture( + { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + }, + files: { + "app/routes/_index.jsx": js` + import { Link } from "@remix-run/react"; + export default () => Child; + `, + "app/routes/_main.jsx": js` + import { useRouteError } from "@remix-run/react"; + export function ErrorBoundary() { + return
{useRouteError().message}
; + } + `, + "app/routes/_main.child.jsx": js` + export default function Component() { + throw new Error('Error from render') + } + `, + }, + }, + ServerMode.Development + ); + appFixture = await createAppFixture(fixture, ServerMode.Development); + }); + + test.afterAll(() => { + appFixture.close(); + console.error = _consoleError; + }); + + test.describe("with JavaScript", () => { + runTests(); + }); + + test.describe("without JavaScript", () => { + test.use({ javaScriptEnabled: false }); + runTests(); + }); + + function runTests() { + test.only("should not treat an ErrorBoundary-only route as a resource route", async ({ + page, + }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/child"); + let html = await app.getHtml(); + expect(html).not.toMatch("has no component"); + expect(html).toMatch("Error from render"); + }); + } +}); diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index cba0385c267..f844d9d6da0 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -22,6 +22,7 @@ import { Await as AwaitRR, Link as RouterLink, NavLink as RouterNavLink, + Outlet, UNSAFE_DataRouterContext as DataRouterContext, UNSAFE_DataRouterStateContext as DataRouterStateContext, isRouteErrorResponse, @@ -113,7 +114,7 @@ function useRemixContext(): RemixContextObject { // RemixRoute export function RemixRoute({ id }: { id: string }) { - let { routeModules } = useRemixContext(); + let { routeModules, future } = useRemixContext(); invariant( routeModules, @@ -121,7 +122,15 @@ export function RemixRoute({ id }: { id: string }) { "Check this link for more details:\nhttps://remix.run/pages/gotchas#server-code-in-client-bundles" ); - let { default: Component } = routeModules[id]; + let { default: Component, ErrorBoundary, CatchBoundary } = routeModules[id]; + + // Default Component to Outlet if we expose boundary UI components + if ( + !Component && + (ErrorBoundary || (!future.v2_errorBoundary && CatchBoundary)) + ) { + Component = Outlet; + } invariant( Component, From 99fe8f5b3ef3cc8923989d850bb56b72a2848730 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 20 Apr 2023 18:02:00 -0400 Subject: [PATCH 2/2] Remove focused test --- integration/resource-routes-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/resource-routes-test.ts b/integration/resource-routes-test.ts index c4c0243e714..5307d471113 100644 --- a/integration/resource-routes-test.ts +++ b/integration/resource-routes-test.ts @@ -245,7 +245,7 @@ test.describe("Development server", async () => { }); function runTests() { - test.only("should not treat an ErrorBoundary-only route as a resource route", async ({ + test("should not treat an ErrorBoundary-only route as a resource route", async ({ page, }) => { let app = new PlaywrightFixture(appFixture, page);