Fix console warning for routes that do not export a default component (…

…#7745)
remix-run · Oct 26, 2023 · 6743013 · 6743013
1 parent 1f29147
commit 6743013
Show file tree

Hide file tree

Showing 3 changed files with 155 additions and 15 deletions.
diff --git a/.changeset/empty-default-export.md b/.changeset/empty-default-export.md
@@ -0,0 +1,8 @@
+---
+"@remix-run/react": patch
+---
+
+Fix warning that could be logged when using route files with no `default` export
+ - It seems our compiler compiles these files to export an empty object as the `default` which we can then end up passing to `React.createElement`, triggering the console warning, but generally no UI issues
+ - By properly detecting these, we can correctly pass `Component: undefined` off to the React Router layer
+ - This is technically an potential issue in the compiler but it's an easy patch in the `@remix-run/react` layer and hopefully disappears in a Vite world
diff --git a/packages/remix-react/__tests__/components-test.tsx b/packages/remix-react/__tests__/components-test.tsx
@@ -1,10 +1,13 @@
+import { createStaticHandler } from "@remix-run/router";
+import { act, fireEvent, render } from "@testing-library/react";
 import * as React from "react";
-import { createMemoryRouter, RouterProvider } from "react-router-dom";
-import { fireEvent, render, act } from "@testing-library/react";
+import { createMemoryRouter, Outlet, RouterProvider } from "react-router-dom";
 
+import { RemixBrowser } from "../browser";
 import type { LiveReload as ActualLiveReload } from "../components";
 import { Link, NavLink, RemixContext } from "../components";
-
+import invariant from "../invariant";
+import { RemixServer } from "../server";
 import "@testing-library/jest-dom/extend-expect";
 
 // TODO: Every time we touch LiveReload (without changing the API) these tests
@@ -199,3 +202,126 @@ describe("<Link />", () => {
 describe("<NavLink />", () => {
   itPrefetchesPageLinks(NavLink);
 });
+
+describe("<RemixServer>", () => {
+  it("handles empty default export objects from the compiler", async () => {
+    let staticHandlerContext = await createStaticHandler([{ path: "/" }]).query(
+      new Request("http://localhost/")
+    );
+    invariant(
+      !(staticHandlerContext instanceof Response),
+      "Expected a context"
+    );
+
+    let context = {
+      manifest: {
+        routes: {
+          root: {
+            hasLoader: false,
+            hasAction: false,
+            hasErrorBoundary: false,
+            id: "root",
+            module: "root.js",
+            path: "/",
+          },
+          empty: {
+            hasLoader: false,
+            hasAction: false,
+            hasErrorBoundary: false,
+            id: "empty",
+            module: "empty.js",
+            index: true,
+            parentId: "root",
+          },
+        },
+        entry: { imports: [], module: "" },
+        url: "",
+        version: "",
+      },
+      routeModules: {
+        root: {
+          default: () => {
+            return (
+              <>
+                <h1>Root</h1>
+                <Outlet />
+              </>
+            );
+          },
+        },
+        empty: { default: {} },
+      },
+      staticHandlerContext,
+      future: {},
+    };
+
+    jest.spyOn(console, "warn").mockImplementation(() => {});
+    jest.spyOn(console, "error");
+
+    let { container } = render(
+      <RemixServer context={context} url="http://localhost/" />
+    );
+
+    expect(console.warn).toHaveBeenCalledWith(
+      'Matched leaf route at location "/" does not have an element or Component. This means it will render an <Outlet /> with a null value by default resulting in an "empty" page.'
+    );
+    expect(console.error).not.toHaveBeenCalled();
+    expect(container.innerHTML).toMatch("<h1>Root</h1>");
+  });
+});
+
+describe("<RemixBrowser>", () => {
+  it("handles empty default export objects from the compiler", () => {
+    window.__remixContext = {
+      url: "/",
+      state: {
+        loaderData: {},
+      },
+      future: {},
+    };
+    window.__remixRouteModules = {
+      root: {
+        default: () => {
+          return (
+            <>
+              <h1>Root</h1>
+              <Outlet />
+            </>
+          );
+        },
+      },
+      empty: { default: {} },
+    };
+    window.__remixManifest = {
+      routes: {
+        root: {
+          hasLoader: false,
+          hasAction: false,
+          hasErrorBoundary: false,
+          id: "root",
+          module: "root.js",
+          path: "/",
+        },
+        empty: {
+          hasLoader: false,
+          hasAction: false,
+          hasErrorBoundary: false,
+          id: "empty",
+          module: "empty.js",
+          index: true,
+          parentId: "root",
+        },
+      },
+      entry: { imports: [], module: "" },
+      url: "",
+      version: "",
+    };
+
+    jest.spyOn(console, "error");
+
+    let { container } = render(<RemixBrowser />);
+
+    expect(console.error).not.toHaveBeenCalled();
+    expect(container.innerHTML).toMatch("<h1>Root</h1>");
+  });
+});
diff --git a/packages/remix-react/routes.tsx b/packages/remix-react/routes.tsx
@@ -6,7 +6,7 @@ import type {
 } from "react-router-dom";
 import { redirect, useRouteError } from "react-router-dom";
 
-import type { RouteModules } from "./routeModules";
+import type { RouteModule, RouteModules } from "./routeModules";
 import { loadRouteModule } from "./routeModules";
 import {
   fetchData,
@@ -73,7 +73,7 @@ export function createServerRoutes(
     let routeModule = routeModules[route.id];
     let dataRoute: DataRouteObject = {
       caseSensitive: route.caseSensitive,
-      Component: routeModule.default,
+      Component: getRouteModuleComponent(routeModule),
       ErrorBoundary: routeModule.ErrorBoundary
         ? routeModule.ErrorBoundary
         : route.id === "root"
@@ -170,7 +170,7 @@ export function createClientRoutes(
       ...(routeModule
         ? // Use critical path modules directly
           {
-            Component: routeModule.default,
+            Component: getRouteModuleComponent(routeModule),
             ErrorBoundary: routeModule.ErrorBoundary
               ? routeModule.ErrorBoundary
               : route.id === "root"
@@ -244,17 +244,9 @@ async function loadRouteModuleWithBlockingLinks(
   let routeModule = await loadRouteModule(route, routeModules);
   await prefetchStyleLinks(route, routeModule);
 
-  // Resource routes are built with an empty object as the default export -
-  // ignore those when setting the Component
-  let defaultExportIsEmptyObject =
-    typeof routeModule.default === "object" &&
-    Object.keys(routeModule.default || {}).length === 0;
-
   // Include all `browserSafeRouteExports` fields
   return {
-    ...(routeModule.default != null && !defaultExportIsEmptyObject
-      ? { Component: routeModule.default }
-      : {}),
+    Component: getRouteModuleComponent(routeModule),
     ErrorBoundary: routeModule.ErrorBoundary,
     handle: routeModule.handle,
     links: routeModule.links,
@@ -299,3 +291,17 @@ function getRedirect(response: Response): Response {
   }
   return redirect(url, { status, headers });
 }
+
+// Our compiler generates the default export as `{}` when no default is provided,
+// which can lead us to trying to use that as a Component in RR and calling
+// createElement on it.  Patching here as a quick fix and hoping it's no longer
+// an issue in Vite.
+function getRouteModuleComponent(routeModule: RouteModule) {
+  if (routeModule.default == null) return undefined;
+  let isEmptyObject =
+    typeof routeModule.default === "object" &&
+    Object.keys(routeModule.default).length === 0;
+  if (!isEmptyObject) {
+    return routeModule.default;
+  }
+}