Skip to content

Commit

Permalink
fix: Remove the internal router singleton (#9227)
Browse files Browse the repository at this point in the history
* feat: remove singleton in favor of manual router creaton

* update examples

* fix browser/hash tests

* Fix memory tests

* Rename DataRouter -> RouterProvider

* rename in examples

* support multiple subscribers

* add changeset

* alias createRoutesFromElements

* Add consistent-type-imports eslint rule

* DataStaticRouter -> StaticRouterProvider

* Add PR number to changeset
  • Loading branch information
brophdawg11 committed Sep 8, 2022
1 parent 112c02c commit c17512d
Show file tree
Hide file tree
Showing 32 changed files with 712 additions and 560 deletions.
86 changes: 86 additions & 0 deletions .changeset/calm-lies-destroy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
---
"react-router": patch
"react-router-dom": patch
"@remix-run/router": patch
---

fix: remove internal router singleton (#9227)

This change removes the internal module-level `routerSingleton` we create and maintain inside our data routers since it was causing a number of headaches for non-simple use cases:

- Unit tests are a pain because you need to find a way to reset the singleton in-between tests
- Use use a `_resetModuleScope` singleton for our tests
- ...but this isn't exposed to users who may want to do their own tests around our router
- The JSX children `<Route>` objects cause non-intuitive behavior based on idiomatic react expectations
- Conditional runtime `<Route>`'s won't get picked up
- Adding new `<Route>`'s during local dev won't get picked up during HMR
- Using external state in your elements doesn't work as one might expect (see #9225)

Instead, we are going to lift the singleton out into user-land, so that they create the router singleton and manage it outside the react tree - which is what react 18 is encouraging with `useSyncExternalStore` anyways! This also means that since users create the router - there's no longer any difference in the rendering aspect for memory/browser/hash routers (which only impacts router/history creation) - so we can get rid of those and trim to a simple `RouterProvider`

```jsx
// Before
function App() {
<DataBrowserRouter>
<Route path="/" element={<Layout />}>
<Route index element={<Home />}>
</Route>
<DataBrowserRouter>
}

// After
let router = createBrowserRouter([{
path: "/",
element: <Layout />,
children: [{
index: true,
element: <Home />,
}]
}]);

function App() {
return <RouterProvider router={router} />
}
```

If folks still prefer the JSX notation, they can leverage `createRoutesFromElements` (aliased from `createRoutesFromChildren` since they are not "children" in this usage):

```jsx
let routes = createRoutesFromElements(
<Route path="/" element={<Layout />}>
<Route index element={<Home />}>
</Route>
);
let router = createBrowserRouter(routes);

function App() {
return <RouterProvider router={router} />
}
```

And now they can also hook into HMR correctly for router disposal:

```
if (import.meta.hot) {
import.meta.hot.dispose(() => router.dispose());
}
```

And finally since `<RouterProvider>` accepts a router, it makes unit testing easer since you can create a fresh router with each test.

**Removed APIs**

- `<DataMemoryRouter>`
- `<DataBrowserRouter>`
- `<DataHashRouter>`
- `<DataRouterProvider>`
- `<DataRouter>`

**Modified APIs**

- `createMemoryRouter`/`createBrowserRouter`/`createHashRouter` used to live in `@remix-run/router` to prevent devs from needing to create their own `history`. These are now moved to `react-router`/`react-router-dom` and handle the `RouteObject -> AgnosticRouteObject` conversion.

**Added APIs**

- `<RouterProvider>`
- `createRoutesFromElements` (alias of `createRoutesFromChildren`)
3 changes: 2 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{
"extends": ["react-app"],
"rules": {
"import/first": 0
"import/first": "off",
"@typescript-eslint/consistent-type-imports": "error"
},
"overrides": [
{
Expand Down
67 changes: 67 additions & 0 deletions examples/data-router/src/app.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import React from "react";
import {
createBrowserRouter,
createRoutesFromElements,
Route,
RouterProvider,
} from "react-router-dom";

import {
Fallback,
Layout,
homeLoader,
Home,
deferredLoader,
DeferredPage,
deferredChildLoader,
deferredChildAction,
DeferredChild,
todosAction,
todosLoader,
TodosList,
TodosBoundary,
todoLoader,
Todo,
sleep,
AwaitPage,
} from "./routes";
import "./index.css";

let router = createBrowserRouter(
createRoutesFromElements(
<Route path="/" element={<Layout />}>
<Route index loader={homeLoader} element={<Home />} />
<Route path="deferred" loader={deferredLoader} element={<DeferredPage />}>
<Route
path="child"
loader={deferredChildLoader}
action={deferredChildAction}
element={<DeferredChild />}
/>
</Route>
<Route id="await" path="await" element={<AwaitPage />} />
<Route
path="long-load"
loader={() => sleep(3000)}
element={<h1>👋</h1>}
/>
<Route
path="todos"
action={todosAction}
loader={todosLoader}
element={<TodosList />}
errorElement={<TodosBoundary />}
>
<Route path=":id" loader={todoLoader} element={<Todo />} />
</Route>
</Route>
)
);

if (import.meta.hot) {
import.meta.hot.dispose(() => router.dispose());
}

export default function App() {
return <RouterProvider router={router} fallbackElement={<Fallback />} />;
}
56 changes: 2 additions & 54 deletions examples/data-router/src/main.tsx
Original file line number Diff line number Diff line change
@@ -1,61 +1,9 @@
import React from "react";
import ReactDOM from "react-dom/client";
import { DataBrowserRouter, Route } from "react-router-dom";

import {
Fallback,
Layout,
homeLoader,
Home,
deferredLoader,
DeferredPage,
deferredChildLoader,
deferredChildAction,
DeferredChild,
todosAction,
todosLoader,
TodosList,
TodosBoundary,
todoLoader,
Todo,
sleep,
AwaitPage,
} from "./routes";
import "./index.css";
import App from "./app";

ReactDOM.createRoot(document.getElementById("root")).render(
<React.StrictMode>
<DataBrowserRouter fallbackElement={<Fallback />}>
<Route path="/" element={<Layout />}>
<Route index loader={homeLoader} element={<Home />} />
<Route
path="deferred"
loader={deferredLoader}
element={<DeferredPage />}
>
<Route
path="child"
loader={deferredChildLoader}
action={deferredChildAction}
element={<DeferredChild />}
/>
</Route>
<Route id="await" path="await" element={<AwaitPage />} />
<Route
path="long-load"
loader={() => sleep(3000)}
element={<h1>👋</h1>}
/>
<Route
path="todos"
action={todosAction}
loader={todosLoader}
element={<TodosList />}
errorElement={<TodosBoundary />}
>
<Route path=":id" loader={todoLoader} element={<Todo />} />
</Route>
</Route>
</DataBrowserRouter>
<App />
</React.StrictMode>
);
3 changes: 1 addition & 2 deletions examples/data-router/src/routes.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from "react";
import type { ActionFunctionArgs, LoaderFunctionArgs } from "react-router-dom";
import {
Await,
Form,
Expand All @@ -16,8 +17,6 @@ import {
useRouteError,
json,
useActionData,
ActionFunctionArgs,
LoaderFunctionArgs,
} from "react-router-dom";

import type { Todos } from "./todos";
Expand Down
42 changes: 42 additions & 0 deletions examples/error-boundaries/src/app.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import React from "react";
import { createBrowserRouter, Outlet, RouterProvider } from "react-router-dom";

import "./index.css";
import {
Fallback,
Layout,
RootErrorBoundary,
Project,
ProjectErrorBoundary,
projectLoader,
} from "./routes";

let router = createBrowserRouter([
{
path: "/",
element: <Layout />,
children: [
{
path: "",
element: <Outlet />,
errorElement: <RootErrorBoundary />,
children: [
{
path: "projects/:projectId",
element: <Project />,
errorElement: <ProjectErrorBoundary />,
loader: projectLoader,
},
],
},
],
},
]);

if (import.meta.hot) {
import.meta.hot.dispose(() => router.dispose());
}

export default function App() {
return <RouterProvider router={router} fallbackElement={<Fallback />} />;
}
28 changes: 2 additions & 26 deletions examples/error-boundaries/src/main.tsx
Original file line number Diff line number Diff line change
@@ -1,34 +1,10 @@
import React from "react";
import ReactDOM from "react-dom/client";

import { DataBrowserRouter, Outlet, Route } from "react-router-dom";
import "./index.css";
import {
Fallback,
Layout,
RootErrorBoundary,
Project,
ProjectErrorBoundary,
projectLoader,
} from "./routes";
import App from "./app";

ReactDOM.createRoot(document.getElementById("root")).render(
<React.StrictMode>
<DataBrowserRouter fallbackElement={<Fallback />}>
<Route path="/" element={<Layout />}>
<Route
path=""
element={<Outlet />}
errorElement={<RootErrorBoundary />}
>
<Route
path="projects/:projectId"
element={<Project />}
errorElement={<ProjectErrorBoundary />}
loader={projectLoader}
/>
</Route>
</Route>
</DataBrowserRouter>
<App />
</React.StrictMode>
);
2 changes: 1 addition & 1 deletion examples/error-boundaries/src/routes.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import React from "react";

import type { LoaderFunctionArgs } from "react-router-dom";
import {
isRouteErrorResponse,
json,
Link,
LoaderFunctionArgs,
Outlet,
useLoaderData,
useRouteError,
Expand Down
39 changes: 39 additions & 0 deletions examples/notes/src/app.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import "./index.css";
import { createBrowserRouter, RouterProvider } from "react-router-dom";

import Root, { loader as rootLoader } from "./routes/root";
import NewNote, { action as newNoteAction } from "./routes/new";
import Note, {
loader as noteLoader,
action as noteAction,
} from "./routes/note";

let router = createBrowserRouter([
{
path: "/",
element: <Root />,
loader: rootLoader,
children: [
{
path: "new",
element: <NewNote />,
action: newNoteAction,
},
{
path: "note/:noteId",
element: <Note />,
loader: noteLoader,
action: noteAction,
errorElement: <h2>Note not found</h2>,
},
],
},
]);

if (import.meta.hot) {
import.meta.hot.dispose(() => router.dispose());
}

export default function App() {
return <RouterProvider router={router} />;
}
Loading

0 comments on commit c17512d

Please sign in to comment.