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

[Bug]: Nested routing (<Routes> inside <RouterProvider />) #10317

Closed
AndrewWebDev opened this issue Apr 6, 2023 · 18 comments · Fixed by #10374
Closed

[Bug]: Nested routing (<Routes> inside <RouterProvider />) #10317

AndrewWebDev opened this issue Apr 6, 2023 · 18 comments · Fixed by #10374
Labels

Comments

@AndrewWebDev
Copy link

What version of React Router are you using?

6.10.0

Steps to Reproduce

Example:
I wrote a full example on Codesandbox for reproducing, please have a look:
https://codesandbox.io/s/react-router-v6-action-error-edbuhu?file=/src/business/business.js

image

Explanation:

I use <RouterProvider /> in the root,
but I have another router inside (for my header) where I show components (search, filtering, selectors) depending on routing.
Other words inside the root "Layout" component exist a "Header" component where exist nested routes <Routes><Route.../></Routes>

Steps for reproduce:
Press the button "Submit. Press and get an error." below on the page and get an error.

To fix the problem, comment in the opened file: (business/business.js file)
// export const APP_ENABLE_NESTED_ROUTES = true;

Conclusion:
Nested routes broke errorElement handling. Why?

Thanks for your attention,
Andrii Badekha

Expected Behavior

After pressing the button: "Submit. Press and get an error.",

Expect message on UI:
Exception properly handled in <PageError />

I expect that my "errorElement" will catch an error and process them, but instead, the react-router shows a general error, not my error handler component.

Actual Behavior

After pressing the button: "Submit. Press and get an error.",

Get the error:
Error: Could not find a matching route for the current errors: [object Object]

As a result, my errorElement was missed, I read additionally documentation and seems like it is unexpected behaviour, docs don't cover this case.

@timdorr
Copy link
Member

timdorr commented Apr 7, 2023

This is as expected. According to the docs, "If you're using a data router like createBrowserRouter it is uncommon to use this component as it does not participate in data loading.". This includes error handling.

The error you're getting is because <Routes> does not read the errorElement (or loader) prop from your <Route>s and is essentially passing null to the browser router. This is partially from some internal implementation details, but combining the two APIs and structures together is not recommended anyways.

@timdorr timdorr closed this as completed Apr 7, 2023
@AndrewWebDev
Copy link
Author

@timdorr Thanks for the clarification and quick response.

React-router notifies us if we try to use nested <RouterProvider router={createBrowserRouter(...)} />:
"You cannot render a <Router> inside another <Router>. You should never have more than one in your app.".

But, if we use instead nested <Routes> - we don't have this exception message.
I mean developers have the possibility of using nested <Routes>, it is not recommended, but possible with pitfalls.

I suppose the react-router team rethinked the router and concentrates on the new router v6 approach as it is recommended router now and further.

React-router v6 based on RouteObject[] a great and powerful config for routes and I try to use it not only as a core routing but as well as for nested routing.

For me would be a very suitable possibility if you can declare multiple route configs as many as you needed and create for example: core root routing, nested header routing, and nested footer routing

This would allows you to declare three RouteObject[] configs and use the same structure for declaring routes with loader, action, errorElement, element, etc.

As a conclusion:
createBrowserRouter() "is the recommended router for all React Router web projects" and allows "one in your app".

If we need something similar to "nested routes" we need to create our own "custom routes component" and according to current location load the proper "component".

@IanVS
Copy link

IanVS commented Apr 18, 2023

I think this is what I am also experiencing, though I'm not positive.

I read this guide to errorElement: https://reactrouter.com/en/main/route/error-element#bubbling, which says:

When a route does not have an errorElement, errors will bubble up through parent routes. This lets you get as granular or general as you like.

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

I can't find any caveats on that page about how the use of a Routes component will break this behavior. Is that something you'd be willing to include in the docs? I'm still a bit confused how the two are related.

@brophdawg11
Copy link
Contributor

I pulled this example down and I think there's an actual bug in here so I'm going to re-open this.

There's some confusion around the usage of <Routes> inside a RouterProvider and I tried to elaborate a bit in this comment, but generally speaking:

  • You can Render descendant <Routes> inside a <RouterProvider>
  • <Route>'s below a <Routes> component cannot participate in data loading, which includes loader/action/errorElement/handle/shouldRevalidate etc.
    • However, the presence of descendant <Routes> should not break error handling at the RouterProvider layer, which seems to be what is happening in this bug.

@brophdawg11
Copy link
Contributor

Related discussion with similar confusion: #10373

@brophdawg11 brophdawg11 added the awaiting release This issue have been fixed and will be released soon label Apr 21, 2023
@brophdawg11 brophdawg11 removed their assignment Apr 21, 2023
@brophdawg11
Copy link
Contributor

This is fixed by #10374 and will be available once React Router 6.11.0 is released.

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 6.11.0-pre.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@AndrewWebDev
Copy link
Author

AndrewWebDev commented Apr 26, 2023

Hi everyone, the issue has been solved, thank you
Modified example with "6.11.0-pre.1":
https://codesandbox.io/s/react-router-v6-action-error-forked-solved-7th46y?file=/src/business/business.js

@jasikpark
Copy link
Contributor

Hi everyone, the issue has been solved, thank you Modified example with "6.11.0-pre.0": codesandbox.io/s/react-router-v6-action-error-forked-solved-7th46y?file=/src/business/business.js

wait... is that allowing an errorElement to be used on a Routes Route? I thought that wasn't permitted?

@jasikpark
Copy link
Contributor

ah... the relative routes building is different when you're in a Routes - is it only relative to statically defined routes?

@AndrewWebDev
Copy link
Author

@jasikpark Nice catch, I believe "errorElement" here is then an exception as a rule
https://reactrouter.com/en/main/route/route

Let's have a look at an example from docs:
image

And what are the types for <Route />:
image

What I am thinking is that "<RouterProvider />" should be the only one on the page and <Route /> should work with "createRoutesFromElements" and mainly created for migration from v5 to v6

But if we trying create our own nested routes like this one:
image

We got typescript IntelliSense for allowed properties, but it's the wrong properties,
because our <Route /> is not inside the function "createRoutesFromElements" and we got the wrong IntelliSense and a lot of them shouldn't work

I think that "errorElement" is somehow connected with internal realization and now it works and doesn't matter where is <Route /> declared

As mentioned @timdorr "The error you're getting is because does not read the errorElement (or loader) prop from your s and is essentially passing null to the browser router. This is partially from some internal implementation details, but combining the two APIs and structures together is not recommended anyways."

I believe we can get a lot of different issues here because "combining the two APIs and structures together is not recommended anyways" and this a reason why I conclude "If we need something similar to "nested routes" we need to create our own "custom routes component" and according to current location load the proper "component"." because if it's not recommended officially we can't use it for production

@brophdawg11
Copy link
Contributor

because if it's not recommended officially we can't use it for production

I think this needs some clarification, as this is not an accurate statement. I've tried to clarify some of the confusion here and here.

To try to summarize:

  • If you are starting a brand new <RouterProvider> application, then yes, we generally wouldn't advise the usage of nested <Routes> since you lose out on the new data APIs.
  • If you are migrating a BrowserRouter application to use <RouterProvider> then you can and should keep using your existing nested <Routes> during the migration which allows you to migrate routes one by one up to the root createBrowserRouter. The end goal would be eventually getting rid of all of the nested <Routes>. But it is absolutely expected that <RouterProvider> + <Routes> can ship to production.

We are working on better docs for this BrowserRouter -> RouterProvider migration to try to clear up some of the common misconceptions.

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 6.11.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@brophdawg11 brophdawg11 removed the awaiting release This issue have been fixed and will be released soon label Apr 28, 2023
@qwertie
Copy link

qwertie commented Sep 17, 2023

<Routes> does not read the errorElement (or loader) prop

Uh, so when using createBrowserRouter, how do I define routes inside a child component such that they DO support loader and errorElement?

(Or is the new system simply incompatible with encapsulation?)

@AndrewWebDev
Copy link
Author

@qwertie Hello, I believe loader and errorElement should work only for createBrowserRouter according to the docs

Here is the link where on v6.10 errorElement works for and seems like it was fixed, it does not work anymore:
https://codesandbox.io/s/react-router-v6-action-error-forked-solved-forked-9f2pr3?file=/src/routes/RoutesNested.js

image

@brophdawg11
Copy link
Contributor

The "Data APIs" do not (and can not) work in BrowserRouter/<Routes> because they rely on the decoupling of route-definition/data-fetching and rendering. Here's a list of the APIs that only work in RouterProvider: https://reactrouter.com/en/main/routers/picking-a-router#data-apis.

If you want error boundaries in Routes, you can just use them directly in your routes to capture rendering errors in children routes, there's no need to try to abstract them through the router:

<BrowserRouter>
  <Routes>
    <Route element={<MyErrorBoundary><Outlet/></MyErrorBoundary>}>
      ...
    </Route>
  </Routes>
</BrowserRouter>

https://codesandbox.io/s/wizardly-feynman-mxvpnt?file=/src/index.js

@jasikpark
Copy link
Contributor

Is https://reactrouter.com/en/main/route/lazy the main way to allow splitting routes into separate files?

@brophdawg11
Copy link
Contributor

For RouterProvider, yes.

Let's please stop using this closed issue as a Q&A forum and open new Q&A discussions for questions moving forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants