[Bug]: errorElement bubbling doesnt work through <Routes> Elements

[MaxNoetzold]

What version of React Router are you using?

6.8.0

Steps to Reproduce

I built an example: https://codesandbox.io/s/react-router-dom-errorelement-not-bubbling-through-routes-tgcbk0?file=/src/App.js
Take a look at the last Element in App.js:

function ElementWithChildren() {
  return (
    <div>
      <Routes>
        {/* If you dont define errorElement here again it wont show the custom error page (of a parent route) */}
        <Route errorElement={<RouteError />}>
          <Route index element={<EmptyElement text={"1"} />} />
          <Route path="/one" element={<EmptyElement text={"1.1"} />} />
          <Route path="/two" element={<EmptyElementWithError />} />
        </Route>
      </Routes>
    </div>
  );
}

Expected Behavior

The way it was described in the documentation, I would expect an error to be forwarded to the root even through nested .

Put an errorElement at the top of your route tree and handle nearly every error in your app in one place.

Actual Behavior

The default errorElement gets triggered:

💿 Hey developer 👋
You can provide a way better UX than this when your app throws errors by providing your own errorElement props on <Route>

[woodreamz]

It seems to be the same issue as #9568.

Using <Routes /> with a data provider seems not really supported. I think the documentation should have a big warning on this one.

This thing made me crazy until I understand that... They are working on a feature to inject routes dynamically, but for now, you need to specify the errorElement on each Route :( There is no way to disable it.

[timdorr]

We have a note about this: https://reactrouter.com/en/main/components/routes

This is the same issue as #9568 and the comments there apply here.

[woodreamz]

@timdorr I know but it's more than a "Note" or maybe the note should explain the limitations.

Until react-router-dom allows to inject routes in the Data Router. You can wrap the Routes component to inject the errorElement, this is an example:

const Routes = ({ children }) => {
  return (
    <RRDRoutes>
      {React.Children.map(children, (child) => {
        // If the Route does not have the errorElement, we inject it.
        if (!child?.props?.errorElement) {
          return React.cloneElement(child, { errorElement: <ErrorElement /> });
        }
        return child;
      })}
    </RRDRoutes>
  );
};

It's only injecting the property at the top level of Route.

Wrapping a Route does not work for me because Routes checks the element type of its children.

[pleunv]

This really needs to be more prominently featured in the docs. I was aware that there was a difference between the new data routers and the previous Router components, but the fact that this creates such a big feature gap is really not obvious right now. They are effectively two different routers under the same package name, that you can but should not mix and match because you'll spend hours trying to figure out why things are not working the way they're seemingly advertised. I went through all changelogs, new docs, blog posts and videos and still seem to have missed half of these gotchas.

[brophdawg11]

I think there might be some confusion here that I'd like to attempt to clear up. And as always, docs certainly have room for improvement - we're open to docs PRs that would help clarify things from an application-developer point of view.

the fact that this creates such a big feature gap is really not obvious right now

We did recently add a section explicitly listing the "Data APIs" that only work when using a RouterProvider - does this help clarify? https://reactrouter.com/en/main/routers/picking-a-router#data-apis

They are effectively two different routers under the same package name

We see this as a feature, not a bug! Releasing RouterProvider in v7 or a separate package would have required full rewrites of your apps. Instead we released this as a minor/non-breaking release in v6 specifically to allow incremental upgrades of your current v6 apps. Maybe we need a more explicit docs section on this though (here's a blog post on our approach to major versions moving forward: https://remix.run/blog/future-flags).

In order to migrate incrementally from BrowserRouter to RouterProvider, you just need to use a single splat route at the root of your RouterProvider.

Current app using BrowserRouter:

function App() {
  return (
    <BrowserRouter>
      <Routes>
        <Route ... />
        <Route ... />
      </Routes>
    </BrowserRouter>
  );
}

New app using RouterProvider:

const router = createBrowserRouter([{ path: '*', element: <Layout /> }]);

// Layout is just your descendant Routes tree from your current app
function Layout() {
  return (
    <Routes>
      <Route ... />
      <Route ... />
    </Routes>
  );
}

// App is now a RouterProvider instead of a BrowserRouter
function App() {
  return <RouterProvider router={router} />
}

With those changes, your app should function the same. But now that you have the RouterProvider wired up, you can begin lifting route definitions from <Routes> up to createBrowserRouter() one-by-one and adding loader/action/errorElement/etc. and leveraging the rest of the Data APIs in those route components. This should make for a much easier migration than having to lift all of your routes up at once.

should not mix and match

This is not the intention and I hope the docs aren't implying this. We 100% want users to be mixing and matching these while they incrementally upgrade their apps. You can absolutely render descendant <Routes> trees inside of a RouterProvider- they just can't participate in the data API aspects because the router doesn't know about them ahead of the render cycle.

I hope this is useful and if you have any specific docs sections of concern or improvements, PRs are always welcome, or even a new issue with specifics would be helpful for us to look into. Thanks!

[jasikpark]

The migration guide from v5 to v6 I think could be clearer about which APIs are inaccessible w/o lifting the routes up to the main route config - it reads as a totally optional step atm, with a slight aside about "future data routing apis"

I think it might be helpful to have a page that lists what is incompatible / won't behave the way you might expect && to link to that from the migration guide - like this caveat about Routes

[pleunv]

Hi Matt! Really do appreciate the time you take to work through all these issues 🙂

I understand encouraging mixing and matching old and new style routers and I definitely favor an incremental upgrade path, however, I find the current approach can easily lead to problems:

• A component tree making use of data router capabilities can work in one place but not another due to introduction of a <Routes> somewhere in between. There are (as far as I'm aware) no guard rails in place to warn you of this which makes you distrust your code.
• The same Route components are being used for both data and non-data routers, meaning that e.g. the data API props are technically accessible on non-data router Routes although they will only work after all ancestor routes have been lifted to the top. After reading through the blog posts, migration guides, and some comments in other issues/threads that also suggest a similar incremental top-level data-router+wildcard approach without mentioning any caveats it's very easy to make the mistake of attempting to use some of these APIs. TypeScript doesn't complain but nothing will work as expected. This makes you distrust your types.

The mixing and matching is fine I suppose, but I think some sort of guard rail would be a welcome addition as right now this can create unnecessary friction, especially in big(ger) teams. This would at least already create some awareness about misuse of these new features, perhaps pointing to the new docs additions that clarify this a bit further. Personally I also feel like the types should reflect the APIs accordingly, for example through 2 different Route exports of which one has a narrowed down prop type that only enables the non-data props, with the guard rails giving guidance towards which one to use for your use-case. When introducing a secondary API there should be a clear boundary between the two, otherwise things just get overly complicated.

[brophdawg11]

That's fair. FWIW I don't believe there's a simple "make everyone happy" solution here. Originally we were only going to allow JSON route definitions for createBrowserRouter, which would have allowed us to better type-check the non-data <Route> components differently than the data-aware JSON route object.

But, then we'd have gotten a bunch of complaints from the other direction around "I need to change every route to JSON?!?". So createRoutesFromElements is the middleground/path-of-least-resistance there so you can literally copy/paste your route tree upwards and be done with it. And it didn't make sense to introduce a new <DataRoute> component because these aren't actually rendered elements so the usage of the JSX notation is already misleading and we didn't want to further that confusion.

We'll see what we can do to make this more explicit in the docs and migration guides for sure though - definitely appreciate the feedback 🙌