[v6] Split location from navigation to prevent unnecessary re-rendering

[KingSora]

Good day!

The following is an idea, which would increase the performance of any app which is using react-router. Please keep in mind, that some of the following assumptions could be wrong. This is also based on the v6 code.

Currently we have one combined context called LocationContext, it stores:

interface LocationContextObject {
  action?: Action;
  location?: Location;
  navigator?: Navigator;
  static: boolean;
}

The ContextProvider is the <Router> component, which is also assigning those values here.

After taking a look at any Router implementation (MemoryRouter, BrowserRouter or HashRouter) I was seeing the same pattern, where the context updates:

let [state, setState] = React.useState({
  action: history.action,
  location: history.location
});

React.useLayoutEffect(() => history.listen(setState), [history]);

The context will update each time you navigate, but as you can see only the action and the location update, the navigator remains unchanged. This is important, because many hooks are depending on the LocationContext but are only using the navigator: useNavigate, useBlocker, useHref, and possibly more.

Yes I know, they're using the useInRouterContext hook which is depending on the location, but this is only used to print error / warning messages and thus could be changed.

If there would be one context for location (and action) and one for navigation, then components which would use hooks where only the navigation is needed, would not update unnecessarily if the location changes, since navigation is static.

Actual Behavior

I noticed this behavior while experimenting with the router. Here is the Codesandbox example I did: https://codesandbox.io/s/charming-hooks-yotlj?file=/src/Admin.js

As you can see, the "renders" stay the same, no matter how many times you change the route. But as soon as I start using the useNavigate hook, they increase with each navigation: https://codesandbox.io/s/peaceful-faraday-32xke?file=/src/Admin.js

Thanks for reading!

[myNameIsDu]

@KingSora I thought it was a great idea, but no one seemed to be paying attention to it,Can I try to implement this optimization and come up with a PR

[KingSora]

@myNameIsDu I believe the people behind react-router also have other projects and responsibilities, so I would give them more time to respond. If you like.. feel free to create a PR whenever you like 😃

[gitcatrat]

It could help a bit in some architectures and please correct me if I'm wrong about anything here but it's common to have routing at the top of your app. This means that most of your app re-renders on any location change anyway and micro-managing renders on "leaf components" doesn't make any sense and/or even hurts overall performance.

So what you described occurs pretty much only in "global" components that are outside the routes.

All that being said, I have nothing against the idea.

[KingSora]

@gitcatrat It all depends how you structure your app, if you have a ton of memoized components and "just" a subnavigation, its possible to increase the performance quite a bit. 😄 (by just navigation inside the subnavigation)

[myNameIsDu]

@chaance Why can't my PR get merged？:sob:

[Lure5134]

I would like to see a usePathname() that just return the pathname as a string😄

[timdorr]

Fixed by #7936

[KingSora]

Thanks for implementing to everyone involved! 😃