remix-run / react-router

Declarative routing for React
https://reactrouter.com
MIT License
53.06k stars 10.3k forks source link

Make Routes component participate in data loading #9254

Closed MrHus closed 1 year ago

MrHus commented 2 years ago

What is the new or updated feature that you are suggesting?

Make the <Routes> component participate in data loading.

Why should this feature be included?

Currently I'm using the <Routes /> component as a way to split up my routes in multiple different files. This way my routes do not grow very large, and I keep things manageable.

Currently I'm looking at the new "loader" feature and how to integrate it in my application. But the loaders do not work for nested <Routes />. The documentation says:

"If you're using a data router like createBrowserRouter it is uncommon to use this component as it does not participate in data loading."

It would be nice to support <Routes /> in combination with the "createBrowserRouter" as to not get a GOD routing file containing all routes of a large application.

The only way forward I see now using the current API is not using "createRoutesFromElements" and importing "arrays" and merge them at the top, but this way I loose the "routes as jsx" features.

maxprilutskiy commented 2 years ago

I second this.

Unless I'm missing something, to use the new loader feature, createBrowserRouter suggests defining all the routes upfront and using that together with <Outlet />s.

However, if you think about it, it just undermines code-splitting techniques. Using routes-at-the-root solution would require either reference route components directly, which would disable code-splitting, or lazy loading routes as well as all of their child routes to make it work. For example, it would be simply impossible to make /users and /posts load lazily, while /user/:userId and /post/:postId in a regular way together with their respective parent routes.

When a component is required to know about not only its direct children, but also about children of its children, it just breaks incapsulation and becomes an unlikely scalable solution.

I understand react-router team might push the community towards using createBrowserRouter and routes-at-the-root approach to make it easier then to switch to remix in the future. And that could be really nice to have this option some day. But if this API breaks such a crucial thing as defined by the user code splitting, it just simply would not work.

Alternatively, what I thought of is that we could just start nesting prefixed RouterProviders throughout the app, to avoid defining all the routes at the root of the app; but I haven't tested this hypothesis, and it doesn't sound quite right.

brophdawg11 commented 2 years ago

The only way forward I see now using the current API is not using "createRoutesFromElements" and importing "arrays" and merge them at the top, but this way I loose the "routes as jsx" features.

@MrHus createRoutesFromElements simply transforms the JSX to the array notation - so there shouldn't be any reason you couldn't leverage that to import your sub-arrays (written in JSX) and merge them at the top?

@maxprilutskiy We will likely eventually add some "recommended" code splitting approaches to the docs, but you have various options available in the current setup. Another community member did a great write-up of some options here: https://dev.to/infoxicator/data-routers-code-splitting-23no.

As for making <Routes> participate in data loading, I don't think we've ruled it out completely, but it goes a bit against the ideas behind 6.4 - which is to decouple data fetching from rendering. Fetching inside the render lifecycle (i.e., when rendering descendant <Routes>) re-introduces the same waterfalls/spinners we're hoping the 6.4 APIs eliminate! Ryan gave a good talk on this at Reactathon: https://www.youtube.com/watch?v=95B8mnhzoCM.

Here's the main challenge with getting descendant Routes to participate in data-loading, as I see it:

  1. Assume your app defines <Route path="/users/*" loader={userLoader} element={<UsersApp/>}> in which UsersApp has descendant <Routes>
  2. Your user clicks <Link to="/users/1">
  3. At this point, all the router knows about is the /users/* route, so we go into a navigation.state="loading" state and run the singular loader we know about ahead of the render pass.
  4. Loading finishes and we update state.location="/users/1" and navigation.state="idle" and hand off to the render cycle.
  5. Uh-oh! During render we just found more routes with loaders - so we can't actually render since we've not yet got all of our data!
    1. What do we render? We don't have data so anything calling useLoaderData() inside your descendant Routes is going to break :/. So you need to now do data-checking inside your components and this breaks the happy-path guarantee of useLoaderData, and it becomes the same DX as if we were just fetching in components in 6.3 and before.
    2. Do we throw back out to a higher level suspense boundary? That breaks the current UI you were on prior to clicking the link though :/
    3. Do we go back into navigation.state=loading? That re-triggers the global loading indicators that were just removed because loading finished :/

So the idea of allowing descendant Routes (unknown to the router prior to render time) is a bit at odds with some of the major design goals of 6.4 (decouple fetching and rendering, reducing spinners, etc.)

We do agree that it's currently non-optimal for large apps to have to specify all of their routes up-front, and we have ideas in mind to alleviate that by adding some form of runtime route discovery, but I don't think that discovery will necessarily happen in the react render lifecycle - more likely it will happen through some other API we can tap into prior to rendering.

I'm going to leave this open for now but I suspect this will get converted into a Discussion in which we can continue to figure out the right API for the type of route discovery

gaspardip commented 2 years ago

So I wrote this on the remix discord a while ago but I'll leave it here so it doesn't get lost.

The current solution makes too many assumptions about the application and takes away too much control away from users in my opinion.

Kind of related to what @maxprilutskiy said, I think a good solution would be being able to declare multiple "sub" routers, then have a "root" router that composes all of them, very similar to what express.Router does, where you are able to declare a router for an entire route tree and apply middlewares and whatnot to it.

Why this is good:

Granted, the "root" router wouldn't be able statically determine the entire route tree but that doesn't matter because each "sub-router" would know the routes it cares about and the data they need to fetch in order to be rendered.

Here's a very rough usage as I imagine it:


// users/routes.tsx
import { UserLayout } from './UserLayout';
import { NewUser } from './NewUser';
import { UserDetails } from './UserDetails';
import { UsersList } from './UsersList';
import { EditUser } from './EditUser';

const usersRouter = createBrowserRouter([
  {
    path: '/',
    element: <UsersLayout />,
    children: [
      { index: true, element: <UsersList /> },
      { path: 'new', element: <NewUser /> },
      {
        path: ':userId/*',
        loader: ({ params }) =>
          fetch(`/api/users/${params.userId}`).then((res) => res.json()),
        children: [
          { index: true, element: <UserDetails /> },
          {
            path: 'edit',
            element: <EditUser />,
          },
        ],
      },
    ],
  },
]);

export default usersRouter;

// main.tsx
// lazy-load usersRouter, users-related components are not included in the initial bundle
const usersRouter = () => import('./users/routes.tsx')

const rootRouter = createBrowserRouter([
  {
    path: '/',
    element: <Layout />,
    loader: () => fetch(`/api/config`).then((res) => res.json()),
    children: [
      { index: true, element: <Home /> },
      // `router` should be able to accept lazy-loaded routers
      // `router` and `element` are mutually exclusive
      { path: 'users/*', router: usersRouter }, 
    ],
  },
  {
    path: '/login'',
    element: <Login />,
  }
]);
tlmader commented 2 years ago

We have an application that uses lazy-loaded descendant routes. I am running into this scenario when attempting to take advantage of the new useMatches hook for breadcrumbs.

brophdawg11 commented 1 year ago

I'm going to convert this to a discussion so it can go through our new Open Development process. Please upvote the new Proposal if you'd like to see this considered!