Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 7 commits into from
Aug 11, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
389d4ef
chore: make router types fully framework agnostic
brophdawg11 Aug 9, 2022
8639ac5
chore: re-export react-specific types from react-router
brophdawg11 Aug 9, 2022
e519220
chore: re-export proper react-specific types from dom/native
brophdawg11 Aug 9, 2022
0d09285
Keep element/errorElement optional
brophdawg11 Aug 10, 2022
11b9bb6
add hasErrorBoundary to manual routes automatically
brophdawg11 Aug 10, 2022
b45f4f5
fix exports test and rename to enhanceManualRouteObjects
brophdawg11 Aug 10, 2022
cf3d5c9
Bump bundle
brophdawg11 Aug 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
88 changes: 88 additions & 0 deletions packages/react-router-dom/__tests__/data-browser-router-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>"
`);
});
});
});
}
Expand Down
19 changes: 15 additions & 4 deletions packages/react-router-dom/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -27,7 +33,6 @@ import type {
HashHistory,
History,
HydrationState,
RouteObject,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

react-router-dom now re-exports from react-router instead of @remix-run-/router

Router as RemixRouter,
} from "@remix-run/router";
import {
Expand Down Expand Up @@ -72,6 +77,7 @@ export type {
AwaitProps,
DataMemoryRouterProps,
DataRouteMatch,
DataRouteObject,
Fetcher,
Hash,
IndexRouteProps,
Expand Down Expand Up @@ -171,6 +177,7 @@ export {
UNSAFE_NavigationContext,
UNSAFE_LocationContext,
UNSAFE_RouteContext,
UNSAFE_enhanceManualRouteObjects,
} from "react-router";
//#endregion

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions packages/react-router-dom/server.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as React from "react";
import type {
RevalidationState,
RouteObject,
Router as RemixRouter,
StaticHandlerContext,
} from "@remix-run/router";
Expand All @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions packages/react-router-native/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export type {
AwaitProps,
DataMemoryRouterProps,
DataRouteMatch,
DataRouteObject,
Fetcher,
Hash,
IndexRouteProps,
Expand Down Expand Up @@ -127,6 +128,7 @@ export {
UNSAFE_NavigationContext,
UNSAFE_LocationContext,
UNSAFE_RouteContext,
UNSAFE_enhanceManualRouteObjects,
} from "react-router";

////////////////////////////////////////////////////////////////////////////////
Expand Down
10 changes: 10 additions & 0 deletions packages/react-router/__tests__/createRoutesFromChildren-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ describe("creating routes from JSX", () => {
</h1>,
"errorElement": undefined,
"handle": undefined,
"hasErrorBoundary": false,
"id": "0-0",
"index": undefined,
"loader": undefined,
Expand All @@ -42,6 +43,7 @@ describe("creating routes from JSX", () => {
</h1>,
"errorElement": undefined,
"handle": undefined,
"hasErrorBoundary": false,
"id": "0-1",
"index": undefined,
"loader": undefined,
Expand All @@ -60,6 +62,7 @@ describe("creating routes from JSX", () => {
</h1>,
"errorElement": undefined,
"handle": undefined,
"hasErrorBoundary": false,
"id": "0-2-0",
"index": true,
"loader": undefined,
Expand All @@ -74,6 +77,7 @@ describe("creating routes from JSX", () => {
</h1>,
"errorElement": undefined,
"handle": undefined,
"hasErrorBoundary": false,
"id": "0-2-1",
"index": undefined,
"loader": undefined,
Expand All @@ -84,6 +88,7 @@ describe("creating routes from JSX", () => {
"element": undefined,
"errorElement": undefined,
"handle": undefined,
"hasErrorBoundary": false,
"id": "0-2",
"index": undefined,
"loader": undefined,
Expand All @@ -94,6 +99,7 @@ describe("creating routes from JSX", () => {
"element": undefined,
"errorElement": undefined,
"handle": undefined,
"hasErrorBoundary": false,
"id": "0",
"index": undefined,
"loader": undefined,
Expand Down Expand Up @@ -137,6 +143,7 @@ describe("creating routes from JSX", () => {
</h1>,
"errorElement": undefined,
"handle": undefined,
"hasErrorBoundary": false,
"id": "0-0",
"index": undefined,
"loader": [Function],
Expand All @@ -155,6 +162,7 @@ describe("creating routes from JSX", () => {
</h1>,
"errorElement": undefined,
"handle": undefined,
"hasErrorBoundary": false,
"id": "0-1-0",
"index": true,
"loader": undefined,
Expand All @@ -165,6 +173,7 @@ describe("creating routes from JSX", () => {
"element": undefined,
"errorElement": undefined,
"handle": undefined,
"hasErrorBoundary": false,
"id": "0-1",
"index": undefined,
"loader": undefined,
Expand All @@ -177,6 +186,7 @@ describe("creating routes from JSX", () => {
💥
</h1>,
"handle": undefined,
"hasErrorBoundary": true,
"id": "0",
"index": undefined,
"loader": undefined,
Expand Down
87 changes: 87 additions & 0 deletions packages/react-router/__tests__/data-memory-router-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading