remix-run / react-router

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

[Bug]: useNavigate causes component to rerender #11844

Open jacobjuul opened 1 month ago

jacobjuul commented 1 month ago

What version of React Router are you using?

6.25.1

Steps to Reproduce

I encountered an issue where the useNavigate hook causes unnecessary re-renders in sibling components when the router state is updated. This is particularly problematic in scenarios where one sibling component updates the router state (e.g., search parameters), causing other sibling components that use useNavigate to re-render even though they do not depend on the updated state.

  1. Define two sibling components, Child1 and Child2.
  2. In Child1, use the useSearchParams hook to update the search parameters.
  3. In Child2, use the useNavigate hook.
  4. Update the search parameters in Child1 and observe that Child2 re-renders even though it does not depend on the updated search parameters.
  5. Remove useNavigate from Child2 and observe that it no longers rerenders

Expected Behavior

Use of the hook useNavigate should not cause the component to rerender.

Actual Behavior

using useNavigate in a component causes rerenders when other parts of the app (even siblings), update the router state

jacobjuul commented 1 month ago

Here is a create-react-app that shows the issue https://github.com/jacobjuul/useNavigation/blob/master/src/App.js

pbpeterson commented 1 month ago

I think this is caused by the fact that the useSearchParams hook uses useNavigate under the hood (and this hook, in turn, relies on RouteContext). This means that when you use these hooks, they reference the same state. Here's an illustration:

useNavigate -> RouteContext (its states) useSearchParams -> useNavigate -> RouteContext (its states)

Any state change within the RouterContext will trigger a re-render in the components that are observing those states via hooks.

maheshramaiah commented 3 weeks ago

We are observing same issue with useLocation hook as well. If a route component uses useLocation hook and after it unmounts, it still listens to the page URL changes, causing the unmounted component to execute unnecessarily. Any solution for this to be fixed ?

pbpeterson commented 1 week ago

We are observing same issue with useLocation hook as well. If a route component uses useLocation hook and after it unmounts, it still listens to the page URL changes, causing the unmounted component to execute unnecessarily. Any solution for this to be fixed ?

This also happens for the same reason as the other issue mentioned above.

All pages are wrapped inside various Contexts: Route Context, Navigation Context , Location Context(which is the case here), and when you execute the useLocation hook, you are subscribing to all state changes within that context.

Your Remix application is structured something like this:

<RouteContext>
   <NavigationContext>
      <LocationContext>
          {children}
      <LocationContext />
   </NavigationContext>
</RouteContext>

To be clear: it’s not exactly like this. My point is that every time you use a hook like useLocation, useNavigate, or useSearchParams, you are subscribing your component to the states inside these contexts.

If something changes inside these contexts, the component will re-render. This is not a bug.

jacobjuul commented 1 week ago

I would argue that it is a bug that useNavigate causes rerenders. I agree that it's not a bug that useLocation does. But useNavigate doesn't expose any state (afaik).

pbpeterson commented 1 week ago

I would argue that it is a bug that useNavigate causes rerenders. I agree that it's not a bug that useLocation does. But useNavigate doesn't expose any state (afaik).

This is the implementation of useNavigate: https://github.com/remix-run/react-router/blob/main/packages/react-router/lib/hooks.tsx#L181

RouteContext returns isDataRoute, and you perform the following check: if it's true, you call useNavigateStable; if it's not, you call useNavigateUnstable.

Let's start with the success case:

https://github.com/remix-run/react-router/blob/main/packages/react-router/lib/hooks.tsx#L1086

You are calling useDataRouterContext with "useNavigate" as an argument.

https://github.com/remix-run/react-router/blob/main/packages/react-router/lib/hooks.tsx#L868

Here, you're using the context of DataRouterContext. Let's take a look at its provider:

https://github.com/remix-run/react-router/blob/7372affd445eaa16d7866bc97ef14cb61361bff5/packages/react-router-dom/index.tsx#L726

This is already in react-router-dom, but check the line below DataRouterStateContext — it receives state, and its type is RouterState.

https://github.com/remix-run/react-router/blob/7372affd445eaa16d7866bc97ef14cb61361bff5/packages/router/router.ts#L289

The reason is that when you're invoking the useNavigation hook, you're subscribing your component to all these states. If one state changes in RouterProvider, it will re-render. That's not a bug.