fix: render root layout when error boundary is rendered

wixplosives · Nov 4, 2024 · 079161f · 079161f
1 parent be46b7a
commit 079161f
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 11 deletions.
diff --git a/packages/define-remix-app/src/manifest-to-router.tsx b/packages/define-remix-app/src/manifest-to-router.tsx
@@ -317,16 +317,18 @@ function nonMemoFileToRoute(
               const { moduleResults } = requireModule(filePath);
               const { results } = await moduleResults;
 
-              const moduleWithComp = results as {
+              const routeModule = results as {
                   ErrorBoundary?: React.ComponentType;
+                  Layout?: React.ComponentType;
               };
               return {
                   default: () => (
                       <ErrorPage
                           filePath={filePath}
-                          moduleWithComp={moduleWithComp}
+                          routeModule={routeModule}
                           onCaughtError={onCaughtError}
                           navigation={navigation}
+                          isRootFile={isRootFile}
                       />
                   ),
               };
@@ -410,24 +412,35 @@ function useDispatcher<T>(dispatcher: Dispatcher<T>, onChange?: (newValue: T) =>
 
 function ErrorPage({
     navigation,
-    moduleWithComp,
+    routeModule,
     filePath,
     onCaughtError,
+    isRootFile,
 }: {
     navigation: Navigation;
-    moduleWithComp: {
+    routeModule: {
         ErrorBoundary?: React.ComponentType;
+        Layout?: React.ComponentType<{ children?: React.ReactNode }>;
     };
     onCaughtError: ErrorReporter;
     filePath: string;
+    isRootFile: boolean;
 }) {
     navigation.setNavigateFunction(useNavigate());
     useEffect(() => {
         onCaughtError({ filePath, exportName: 'ErrorBoundary' });
     }, [filePath, onCaughtError]);
-    if (moduleWithComp.ErrorBoundary) {
-        return <moduleWithComp.ErrorBoundary />;
-    }
 
-    return <div>error boundary not found at {filePath}</div>;
+    const errorContent = routeModule.ErrorBoundary ? (
+        <routeModule.ErrorBoundary />
+    ) : (
+        <div>error boundary not found at {filePath}</div>
+    );
+
+    if (isRootFile && routeModule.Layout) {
+        const Layout = routeModule.Layout;
+        return <Layout>{errorContent}</Layout>;
+    } else {
+        return errorContent;
+    }
 }
diff --git a/packages/define-remix-app/test/define-remix.spec.ts b/packages/define-remix-app/test/define-remix.spec.ts
@@ -15,7 +15,13 @@ import {
     userApiConsumer,
     userApiPage,
 } from './test-cases/roots';
-import { pageSource, rootSource as originRootSource, expectRoute, expectLoaderData } from './test-cases/route-builder';
+import {
+    pageSource,
+    rootSource as originRootSource,
+    expectRoute,
+    expectLoaderData,
+    expectRootLayout,
+} from './test-cases/route-builder';
 import chai, { expect } from 'chai';
 import { IAppManifest, RouteInfo, RoutingPattern } from '@wixc3/app-core';
 import { ParentLayoutWithExtra, RouteExtraInfo, RouteModuleInfo } from '../src/remix-app-utils';
@@ -1135,8 +1141,8 @@ describe('define-remix', () => {
 
                 const { dispose, container } = await driver.render({ uri: '404' });
 
-                // ToDo: change to make sure layout is rendered around error
-                await expect(() => container.textContent)
+                await expectRootLayout(container);
+                await expect(() => container.textContent, 'root error boundary')
                     .retry()
                     .to.include('RootComponent error');
 

diff --git a/packages/define-remix-app/test/test-cases/route-builder.ts b/packages/define-remix-app/test/test-cases/route-builder.ts
@@ -182,6 +182,10 @@ export const expectRoute = async (root: HTMLElement, pageComponentName: string)
     const pageRootExpectedAttr = `[data-origin="${pageComponentName}-page-component"]`;
     await expect(() => root.querySelector(pageRootExpectedAttr), `in ${pageComponentName} page`).retry().to.exist;
 };
+export const expectRootLayout = async (root: HTMLElement) => {
+    const rootLayoutExpectedAttr = `[data-origin="root-layout"]`;
+    await expect(() => root.querySelector(rootLayoutExpectedAttr), `root layout rendered`).retry().to.exist;
+};
 export const expectRouteError = async (root: HTMLElement, pageComponentName: string) => {
     const pageRootExpectedAttr = `[data-origin="${pageComponentName}-page-error"]`;
     await expect(() => root.querySelector(pageRootExpectedAttr), `in ${pageComponentName} page error`).retry().to.exist;