Doesn't seem to be working with route objects

[gaspardip]

If I understood correctly, I should be able to use my top level routes (app routes) as a param for the hook and correctly get all app breadcrumbs according to the routes definitions. The hook should walk down the tree and resolve children routes and their respective breadcrumbs in the process. I found that's not the case when doing the following: https://codesandbox.io/s/spring-fog-lbxg0g

In this sandbox, I want every route under /users/:userId to have a dynamic breadcrumb mapping to the user name, but I don't want to explicitly define each of those routes with the /users/:userId prefix (say /users/:userId/edit), so I take advantage of react-router and define a :userId route, then define some additional children routes like edit.

The only way I found to get this behavior is to explicitly define another routes array with a path that partially matches the one I'm interested in and pass that to the hook.

This is not ideal since I want to reuse my routes definitions in a couple of places in my app, including the app itself (useRoutes), a navbar, and the breadcrumbs trail. It also defeats the "encapsulation" of the UsersPage component since the breadcrumbs trail now needs to know about the :userId route at the top level, while react-router allows me to keep that something private to UsersPage.

Note that using { disableDefaults: true } generates no breadcrumbs at all in the first case, while it only generates the user name breadcrumb in the latter.

Am I missing something obvious here? or is my approach wrong?

[icd2k3]

There might be ways we can improve this hook to work better with * in react-router 🤔 ... as it stands in the current version, for this use case, you'd have to make your UserDetailBreadcrumb a bit smarter to be able to use the same route config object without any explicit definitions:

const routes = [
  {
    path: "/",
    element: <AppLayout />,
    children: [
      { index: true, element: <HomePage /> },
      {
        path: "users/*",
        element: <UsersPage />,
        breadcrumb: UserDetailBreadcrumb
      },
      { path: "*", element: <Navigate to="/" /> }
    ]
  }
] as BreadcrumbsRoute[];

export const UserDetailBreadcrumb: BreadcrumbComponentType<string> = ({
  match
}) => {
  console.log(match);

  if (!match.params["*"]) {
    return "Users";
  } else if (match.params["*"].includes("edit")) {
    return "Edit";
  }

  return <>{usersById[(match.params["*"] as unknown) as UserId]}</>;
};

Again, not the cleanest solution, but that will at least work with the current version of the hook. I'll have a think about ways this could potentially be made better. If the hook detects a * maybe we should return the following path sections as props to the component or something like that to make them easier to work with.

[gaspardip]

Thank you for the workaround, will try it out and report back.

[iba-1]

export const UserDetailBreadcrumb: BreadcrumbComponentType<string> = ({
  match
}) => {
  console.log(match);

  if (!match.params["*"]) {
    return "Users";
  } else if (match.params["*"].includes("edit")) {
    return "Edit";
  }

  return <>{usersById[(match.params["*"] as unknown) as UserId]}</>;
};

Writing this alone outputs:

Type '({ match }: BreadcrumbComponentProps) => "Users" | "Edit"' is not assignable to type 'BreadcrumbComponentType'. Type '({ match }: BreadcrumbComponentProps) => "Users" | "Edit"' is not assignable to type 'FunctionComponent<BreadcrumbComponentProps>'. Type 'string' is not assignable to type 'ReactElement<any, any>'. Type 'string' is not assignable to type 'ReactElement<any, any>'.

Are we missing something?

[gaspardip]

you could wrap the strings with a Fragment so ts doesn't complain

[icd2k3]

Thanks @gaspardip - yea types could be better for this

[Nodios]

@icd2k3 I too had the same "issue". I went on and extended the functionality a bit to enable route objects. I pushed the code here so you can see it.

It fits my usability perfectly, I don't know how it would fit a bigger picture.

[icd2k3]

Thanks @Nodios! I think that change might make sense as an optional param, but it unfortunately I don't think it solves @gaspardip's original issue (it might make it a bit nicer to work with, though)... I think to solve the core of this, we need to somehow know the nested route config within UsersPage. All the hook sees now is the root-level route config in App and it's unaware of the additional paths defined in UsersPage.

[icd2k3]

We might be able to do something like element()?.props in useReactRouterBreadcrumbs because when an element is returned via useRoutes it includes all the nested route data.

It might cause unexpected issues, so I'll have to investigate, but this could be a way where the hook could check the elements of a route object to make sure if any of them have nested routes (with breadcrumbs) inside.

This is within App in @gaspardip's example. As you can see in the deep object I can find the breadcrumb prop which is defined within UsersPage

[icd2k3]

Here's an experimental PR with support for nested route objects within element components... I'm not super comfortable with it (route.element?.type()?.props?.value?.matches[0]?.route?.children[0]?.breadcrumb lol), but it does seem to work

https://github.com/icd2k3/use-react-router-breadcrumbs/pull/69

I'll run some more tests on this approach later, but I really wish react-router-dom provided a way to retrieve the entire route config context including config set in child route elements

[Nodios]

@icd2k3 Did you consider lazy loaded routes? To be fair I did not test your changes yet, but I am unsure how it'll track nested route objects for the lazy loaded routes.

I assume you'll have to either register everything beforehand or have some kind of a mechanism in place that'll enable to you add configuration in a lazy manner.

[icd2k3]

@Nodios yea lazy loading will probably be a separate issue - I see in their example they wrap the element in a React.Suspense element, so we'd have to dig a layer deeper to get at the route config.

Of course, this hook will still generate default breadcrumbs for those lazy routes, and users can override their own in this hook's config, but it would be nicer to keep everything encapsulated like @gaspardip wants.

All of this would be pretty trivial if react-router provided a hook or some util to retrieve its internal state (I assume they collect a big state object of all the route config as it changes, they just don't expose it). Perhaps I'll propose something in their repo. It certainly would make it easier for plugins/hooks like this one to extend the functionality.

[icd2k3]

I updated the PR for support of nested lazy-loaded elements with their own route objects, but as you can see the code is pretty ugly to get into that config we need... will have to do a lot of different component-type testing to make sure there's no edges.

[gaspardip]

@icd2k3 thank you so much for taking the time to follow this up, it does indeed feel kinda hacky and as @Nodios pointed out, my real app also uses lazy loaded routes, so that would also need to be considered. react-router exports a couple of contexts that could, maybe, used for this purpose? I'm not too familiar with their codebase so I'm not sure.

[gaspardip]

I asked around on discord and someone shared this with me

React Router 6.4 makes it a bit easier, take a look: https://reactrouter.com/en/main/hooks/use-matches#breadcrumbs Previously it was very difficult because BrowserRouter couldn't statically determine which routes were going to be rendered for a given URL, since any route could itself render another .... But the DataBrowserRouter is designed so that it knows all possible routes statically, so you actually can do things like breadcrumbs

I still don't know if this covers nested lazy-loaded route trees

[icd2k3]

@gaspardip ohhh I haven't seen this before! Thanks for asking around, and bringing it to my attention!

This hook was created because (before 6.4) there wasn't really a straightforward way of implementing breadcrumbs with react-router. Since it's now "officially" supported in their docs, it might be time to suggest folks use their approach (if they're on 6.4+).

I should have some time to try it out and evaluate next week. I assume their example works with useRoutes and lazy-load too.

If it works well I'll probably change the docs here to suggest people use the 6.4 method instead.