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.

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

Layout Routes without a path or index throw an error with useBreadcrumbs #34

Closed
iainsimmons opened this issue Nov 10, 2021 · 3 comments · Fixed by #35
Closed

Layout Routes without a path or index throw an error with useBreadcrumbs #34

iainsimmons opened this issue Nov 10, 2021 · 3 comments · Fixed by #35
Labels
bug Something isn't working v6 support

Comments

@iainsimmons
Copy link

I was attempting to use Layout routes so I didn't have to import and wrap each route component into the same Layout component, but using a route object with breadcrumb properties and useRoutes, I get the following:

Unhandled Runtime Error
Error: useBreadcrumbs: `path` or `index` must be provided in every route object

app.js:

export let routes = [
  {
    element: <Layout />,
    breadcrumb: null,
    children: [
      {
        path: 'home',
        element: <Home />,
        breadcrumb: 'Home',
      },
      {
        path: 'about',
        element: <About />,
        breadcrumb: 'About Us',
      },
    ],
  },
];

export default function App() {
  let appRoutes = useRoutes(routes);
  return appRoutes;
}

components/breadcrumbs.js

import { NavLink } from 'react-router-dom';
import useBreadcrumbs from 'use-react-router-breadcrumbs';
import { routes } from '../app';

export default function Breadcrumbs() {
  const breadcrumbs = useBreadcrumbs(routes);
  return (
    <nav className="breadcrumbs" aria-label="Breadcrumbs">
      {breadcrumbs.map(({ match, breadcrumb }) => (
        <span key={match.pathname}>
          <NavLink to={match.pathname}>{breadcrumb}</NavLink>
        </span>
      ))}
    </nav>
  );
}
@icd2k3 icd2k3 added bug Something isn't working v6 support labels Nov 10, 2021
@icd2k3
Copy link
Owner

icd2k3 commented Nov 10, 2021

Thanks for reporting @iainsimmons - I'm still getting up to speed with the v6 changes

The PageLayout route is admittedly weird. We call it a layout route because it doesn't participate in the matching at all (though its children do). It only exists to make wrapping multiple child routes in the same layout simpler. If we didn't allow this then you'd have to handle layouts in two different ways: sometimes your routes do it for you, sometimes you do it manually with lots of layout component repetition throughout your app:

So in this case it sounds like we expect use-react-router-breadcrumbs not to require a path or index if children are present (meaning we can assume it's a LayoutRoute) - does that sound right?

@iainsimmons
Copy link
Author

Thanks for reporting @iainsimmons - I'm still getting up to speed with the v6 changes

The PageLayout route is admittedly weird. We call it a layout route because it doesn't participate in the matching at all (though its children do). It only exists to make wrapping multiple child routes in the same layout simpler. If we didn't allow this then you'd have to handle layouts in two different ways: sometimes your routes do it for you, sometimes you do it manually with lots of layout component repetition throughout your app:

So in this case it sounds like we expect use-react-router-breadcrumbs not to require a path or index if children are present (meaning we can assume it's a LayoutRoute) - does that sound right?

Yes, that looks like it would work. Here's the source code for those routes and the props it will accept/expect:
https://github.com/remix-run/react-router/blob/main/packages/react-router/index.tsx#L205-L236

@icd2k3
Copy link
Owner

icd2k3 commented Nov 11, 2021

@iainsimmons give 3.0.1 a shot, it should permit Layout Routes now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v6 support
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants