fix: convert types in @remix-run/router to be framework-agnostic (#9141)

* chore: make router types fully framework agnostic * chore: re-export react-specific types from react-router * chore: re-export proper react-specific types from dom/native * Keep element/errorElement optional * add hasErrorBoundary to manual routes automatically * fix exports test and rename to enhanceManualRouteObjects * Bump bundle
remix-run · Aug 11, 2022 · 6df7745 · 6df7745
1 parent 6fef589
commit 6df7745
Show file tree

Hide file tree

Showing 15 changed files with 410 additions and 151 deletions.
diff --git a/package.json b/package.json
@@ -107,7 +107,7 @@
       "none": "12 kB"
     },
     "packages/react-router/dist/umd/react-router.production.min.js": {
-      "none": "13.5 kB"
+      "none": "14 kB"
     },
     "packages/react-router-dom/dist/react-router-dom.production.min.js": {
       "none": "10 kB"

diff --git a/packages/react-router-dom/__tests__/data-browser-router-test.tsx b/packages/react-router-dom/__tests__/data-browser-router-test.tsx
@@ -3313,6 +3313,94 @@ function testDomRouter(
           </div>"
         `);
       });
+
+      // This test ensures that when manual routes are used, we add hasErrorBoundary
+      it("renders navigation errors on leaf elements (when using manual route objects)", async () => {
+        let barDefer = createDeferred();
+
+        let routes = [
+          {
+            path: "/",
+            element: <Layout />,
+            children: [
+              {
+                path: "foo",
+                element: <h1>Foo</h1>,
+              },
+              {
+                path: "bar",
+                loader: () => barDefer.promise,
+                element: <Bar />,
+                errorElement: <BarError />,
+              },
+            ],
+          },
+        ];
+
+        let { container } = render(
+          <TestDataRouter window={getWindow("/foo")} routes={routes} />
+        );
+
+        function Layout() {
+          let navigation = useNavigation();
+          return (
+            <div>
+              <Link to="/bar">Link to Bar</Link>
+              <p>{navigation.state}</p>
+              <Outlet />
+            </div>
+          );
+        }
+
+        function Bar() {
+          let data = useLoaderData();
+          return <h1>Bar:{data?.message}</h1>;
+        }
+        function BarError() {
+          let error = useRouteError();
+          return <p>Bar Error:{error.message}</p>;
+        }
+
+        expect(getHtml(container)).toMatchInlineSnapshot(`
+          "<div>
+            <div>
+              <a
+                href=\\"/bar\\"
+              >
+                Link to Bar
+              </a>
+              <p>
+                idle
+              </p>
+              <h1>
+                Foo
+              </h1>
+            </div>
+          </div>"
+        `);
+
+        fireEvent.click(screen.getByText("Link to Bar"));
+        barDefer.reject(new Error("Kaboom!"));
+        await waitFor(() => screen.getByText("idle"));
+        expect(getHtml(container)).toMatchInlineSnapshot(`
+          "<div>
+            <div>
+              <a
+                href=\\"/bar\\"
+              >
+                Link to Bar
+              </a>
+              <p>
+                idle
+              </p>
+              <p>
+                Bar Error:
+                Kaboom!
+              </p>
+            </div>
+          </div>"
+        `);
+      });
     });
   });
 }

diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx
@@ -3,7 +3,12 @@
  * you'll need to update the rollup config for react-router-dom-v5-compat.
  */
 import * as React from "react";
-import { createRoutesFromChildren, NavigateOptions, To } from "react-router";
+import {
+  createRoutesFromChildren,
+  NavigateOptions,
+  RouteObject,
+  To,
+} from "react-router";
 import {
   Router,
   createPath,
@@ -17,6 +22,7 @@ import {
   UNSAFE_DataRouterContext as DataRouterContext,
   UNSAFE_DataRouterStateContext as DataRouterStateContext,
   UNSAFE_RouteContext as RouteContext,
+  UNSAFE_enhanceManualRouteObjects as enhanceManualRouteObjects,
 } from "react-router";
 import type {
   BrowserHistory,
@@ -27,7 +33,6 @@ import type {
   HashHistory,
   History,
   HydrationState,
-  RouteObject,
   Router as RemixRouter,
 } from "@remix-run/router";
 import {
@@ -72,6 +77,7 @@ export type {
   AwaitProps,
   DataMemoryRouterProps,
   DataRouteMatch,
+  DataRouteObject,
   Fetcher,
   Hash,
   IndexRouteProps,
@@ -171,6 +177,7 @@ export {
   UNSAFE_NavigationContext,
   UNSAFE_LocationContext,
   UNSAFE_RouteContext,
+  UNSAFE_enhanceManualRouteObjects,
 } from "react-router";
 //#endregion
 
@@ -219,7 +226,9 @@ export function DataBrowserRouter({
       basename,
       hydrationData: hydrationData || window.__staticRouterHydrationData,
       window: windowProp,
-      routes: routes || createRoutesFromChildren(children),
+      routes: routes
+        ? enhanceManualRouteObjects(routes)
+        : createRoutesFromChildren(children),
     }).initialize();
   }
   let router = routerSingleton;
@@ -257,7 +266,9 @@ export function DataHashRouter({
       basename,
       hydrationData: hydrationData || window.__staticRouterHydrationData,
       window: windowProp,
-      routes: routes || createRoutesFromChildren(children),
+      routes: routes
+        ? enhanceManualRouteObjects(routes)
+        : createRoutesFromChildren(children),
     }).initialize();
   }
   let router = routerSingleton;

diff --git a/packages/react-router-dom/server.tsx b/packages/react-router-dom/server.tsx
@@ -1,7 +1,6 @@
 import * as React from "react";
 import type {
   RevalidationState,
-  RouteObject,
   Router as RemixRouter,
   StaticHandlerContext,
 } from "@remix-run/router";
@@ -12,7 +11,7 @@ import {
   invariant,
   UNSAFE_convertRoutesToDataRoutes as convertRoutesToDataRoutes,
 } from "@remix-run/router";
-import type { Location, To } from "react-router-dom";
+import type { Location, RouteObject, To } from "react-router-dom";
 import {
   createPath,
   parsePath,

diff --git a/packages/react-router-native/index.tsx b/packages/react-router-native/index.tsx
@@ -28,6 +28,7 @@ export type {
   AwaitProps,
   DataMemoryRouterProps,
   DataRouteMatch,
+  DataRouteObject,
   Fetcher,
   Hash,
   IndexRouteProps,
@@ -127,6 +128,7 @@ export {
   UNSAFE_NavigationContext,
   UNSAFE_LocationContext,
   UNSAFE_RouteContext,
+  UNSAFE_enhanceManualRouteObjects,
 } from "react-router";
 
 ////////////////////////////////////////////////////////////////////////////////

diff --git a/packages/react-router/__tests__/createRoutesFromChildren-test.tsx b/packages/react-router/__tests__/createRoutesFromChildren-test.tsx
@@ -28,6 +28,7 @@ describe("creating routes from JSX", () => {
               </h1>,
               "errorElement": undefined,
               "handle": undefined,
+              "hasErrorBoundary": false,
               "id": "0-0",
               "index": undefined,
               "loader": undefined,
@@ -42,6 +43,7 @@ describe("creating routes from JSX", () => {
               </h1>,
               "errorElement": undefined,
               "handle": undefined,
+              "hasErrorBoundary": false,
               "id": "0-1",
               "index": undefined,
               "loader": undefined,
@@ -60,6 +62,7 @@ describe("creating routes from JSX", () => {
                   </h1>,
                   "errorElement": undefined,
                   "handle": undefined,
+                  "hasErrorBoundary": false,
                   "id": "0-2-0",
                   "index": true,
                   "loader": undefined,
@@ -74,6 +77,7 @@ describe("creating routes from JSX", () => {
                   </h1>,
                   "errorElement": undefined,
                   "handle": undefined,
+                  "hasErrorBoundary": false,
                   "id": "0-2-1",
                   "index": undefined,
                   "loader": undefined,
@@ -84,6 +88,7 @@ describe("creating routes from JSX", () => {
               "element": undefined,
               "errorElement": undefined,
               "handle": undefined,
+              "hasErrorBoundary": false,
               "id": "0-2",
               "index": undefined,
               "loader": undefined,
@@ -94,6 +99,7 @@ describe("creating routes from JSX", () => {
           "element": undefined,
           "errorElement": undefined,
           "handle": undefined,
+          "hasErrorBoundary": false,
           "id": "0",
           "index": undefined,
           "loader": undefined,
@@ -137,6 +143,7 @@ describe("creating routes from JSX", () => {
               </h1>,
               "errorElement": undefined,
               "handle": undefined,
+              "hasErrorBoundary": false,
               "id": "0-0",
               "index": undefined,
               "loader": [Function],
@@ -155,6 +162,7 @@ describe("creating routes from JSX", () => {
                   </h1>,
                   "errorElement": undefined,
                   "handle": undefined,
+                  "hasErrorBoundary": false,
                   "id": "0-1-0",
                   "index": true,
                   "loader": undefined,
@@ -165,6 +173,7 @@ describe("creating routes from JSX", () => {
               "element": undefined,
               "errorElement": undefined,
               "handle": undefined,
+              "hasErrorBoundary": false,
               "id": "0-1",
               "index": undefined,
               "loader": undefined,
@@ -177,6 +186,7 @@ describe("creating routes from JSX", () => {
             💥
           </h1>,
           "handle": undefined,
+          "hasErrorBoundary": true,
           "id": "0",
           "index": undefined,
           "loader": undefined,

diff --git a/packages/react-router/__tests__/data-memory-router-test.tsx b/packages/react-router/__tests__/data-memory-router-test.tsx
@@ -1478,6 +1478,93 @@ describe("<DataMemoryRouter>", () => {
       `);
     });
 
+    // This test ensures that when manual routes are used, we add hasErrorBoundary
+    it("renders navigation errors on leaf elements (when using manual route objects)", async () => {
+      let barDefer = createDeferred();
+
+      let routes = [
+        {
+          path: "/",
+          element: <Layout />,
+          children: [
+            {
+              path: "foo",
+              element: <h1>Foo</h1>,
+            },
+            {
+              path: "bar",
+              loader: () => barDefer.promise,
+              element: <Bar />,
+              errorElement: <BarError />,
+            },
+          ],
+        },
+      ];
+
+      let { container } = render(
+        <DataMemoryRouter routes={routes} initialEntries={["/foo"]} />
+      );
+
+      function Layout() {
+        let navigation = useNavigation();
+        return (
+          <div>
+            <MemoryNavigate to="/bar">Link to Bar</MemoryNavigate>
+            <p>{navigation.state}</p>
+            <Outlet />
+          </div>
+        );
+      }
+      function Bar() {
+        let data = useLoaderData();
+        return <h1>Bar:{data?.message}</h1>;
+      }
+      function BarError() {
+        let error = useRouteError();
+        return <p>Bar Error:{error.message}</p>;
+      }
+
+      expect(getHtml(container)).toMatchInlineSnapshot(`
+        "<div>
+          <div>
+            <a
+              href=\\"/bar\\"
+            >
+              Link to Bar
+            </a>
+            <p>
+              idle
+            </p>
+            <h1>
+              Foo
+            </h1>
+          </div>
+        </div>"
+      `);
+
+      fireEvent.click(screen.getByText("Link to Bar"));
+      barDefer.reject(new Error("Kaboom!"));
+      await waitFor(() => screen.getByText("Bar Error:Kaboom!"));
+      expect(getHtml(container)).toMatchInlineSnapshot(`
+        "<div>
+          <div>
+            <a
+              href=\\"/bar\\"
+            >
+              Link to Bar
+            </a>
+            <p>
+              idle
+            </p>
+            <p>
+              Bar Error:
+              Kaboom!
+            </p>
+          </div>
+        </div>"
+      `);
+    });
+
     it("handles render errors in parent errorElement", async () => {
       let { container } = render(
         <DataMemoryRouter