diff --git a/.changeset/empty-default-export.md b/.changeset/empty-default-export.md new file mode 100644 index 00000000000..f19b394bb24 --- /dev/null +++ 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 index fa2d5523757..1d48cc703de 100644 --- 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("", () => { describe("", () => { itPrefetchesPageLinks(NavLink); }); + +describe("", () => { + 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 ( + <> +

Root

+ + + ); + }, + }, + empty: { default: {} }, + }, + staticHandlerContext, + future: {}, + }; + + jest.spyOn(console, "warn").mockImplementation(() => {}); + jest.spyOn(console, "error"); + + let { container } = render( + + ); + + expect(console.warn).toHaveBeenCalledWith( + 'Matched leaf route at location "/" does not have an element or Component. This means it will render an with a null value by default resulting in an "empty" page.' + ); + expect(console.error).not.toHaveBeenCalled(); + expect(container.innerHTML).toMatch("

Root

"); + }); +}); + +describe("", () => { + it("handles empty default export objects from the compiler", () => { + window.__remixContext = { + url: "/", + state: { + loaderData: {}, + }, + future: {}, + }; + window.__remixRouteModules = { + root: { + default: () => { + return ( + <> +

Root

+ + + ); + }, + }, + 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(); + + expect(console.error).not.toHaveBeenCalled(); + expect(container.innerHTML).toMatch("

Root

"); + }); +}); diff --git a/packages/remix-react/routes.tsx b/packages/remix-react/routes.tsx index 3aee8c494c0..9a82d73c1cd 100644 --- 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; + } +}