Performance concerns on RouterContext with useHistory.

[esealcode]

I noticed an important slowdown of my application (running in development build but i think it's a good practice to optimize from it) when route change occurred. At first i thought it was some deep bug between the Redux dispatch lifecycle with react-redux clashing with the one of react-router since i was dispatching an action at the same time as changing the route but recently i switched to using hooks from the new react-router API and inspected what they were actually doing, leading me to notice that it just uses useContext from react API and extract the right property from it.

https://github.com/ReactTraining/react-router/blob/ea44618e68f6a112e48404b2ea0da3e207daf4f0/packages/react-router/modules/hooks.js#L17

The problem is that whenever the react-router context value is updated on a route change, the useHistory hook causes a re-render of the component which is using it since the new context value is just a new object containing the newly computed values along with the history prop.

<RouterContext.Provider
        children={this.props.children || null}
        value={{
          history: this.props.history,
          location: this.state.location,
          match: Router.computeRootMatch(this.state.location.pathname),
          staticContext: this.props.staticContext
        }}
      />

Since it should not be a problem in most applications, my use case was one where the re-render caused at every route changes were a major performance issue. I use useHistory (mostly for calling navigation methods) in a lot of components which stay mounted even when a route change since my application is mostly modal-based. This means that at every route change, there is every components using useHistory which re-render, and even while using memo in some of them the performance are not that great.

Since the history prop is not likely to change (or even probably never changes since it's a mutable object, but still not a hundred percent sure) i decided to implement a custom context which will act as a "tank-and-proxy" of the react-router context for the history props:

/* I put all the code in one place here for readability purpose */

import * as React from 'react'
import { render } from 'react-dom'
import { useHistory } from 'react-router-dom'

const CustomHistoryContext = React.createContext(null)

function CustomHistoryProvider(props) {
    // Tank the updates from react-router context and get the history props
    const history = useHistory()

    // Provide only the history object
    return (
        <CustomHistoryContext.Provider value={ history }>
            { props.children }
        </CustomHistoryContext.Provider>
    )
}

render(
<BrowserRouter>
    <CustomHistoryProvider>
        <App />
    </CustomHistoryProvider>
</BrowserRouter>
, document.getElementById('app'))

and i define my own useHistory hooks like so:

import * as React from 'react'
import { useContext } from 'react'

import { CustomHistoryContext } from '.../CustomHistoryContext '

export function customUseHistory() {
    // Listen from our CustomHistoryContext
    return useContext(CustomHistoryContext)
}

And as a test component:

import * as React from 'react'
import { memo, useRef } from 'react'
import { useHistory } from 'react-router-dom'

import { customUseHistory } from '.../customUseHistory'

// This component will render once, and probably never again even when route changes.
const ComponentWhichCustomUseHistory = memo(
    props => {
        const history = customUseHistory()
        const lastHistory = useRef(history)

        console.log("#debug customUseHistory test component re-rendered: ", history === lastHistory.current, history)

        lastHistory.current = history

        return null
    }
)

// This component will render at every route changes.
const ComponentWhichUseHistory = memo(
    props => {
        const history = useHistory()
        const lastHistory = useRef(history)

        console.log("#debug useHistory test component re-rendered: ", history === lastHistory.current, history)

        lastHistory.current = history

        return null
    }
)

By using my custom useHistory, performances were back to normal on route changes.

I'm wondering if this would be possible to implement this behavior of "standalone" history context in the current codebase. It's probably not possible from the withRouter HOC since it involves a lot of things outside of the history props (and moving the history prop to a withHistory HOC would break every apps using withRouter to access the history), but it would be probably a lot easier to do it from the useHistory hook.

Also i assume that my custom implementation is safe from a lifecycle point of view but it can be a wrong assumption since i'm not that familiar with the react-router codebase, so correct me if i'm wrong.

Keep up the good work 👍

[rob2d]

Looking into this briefly (not to familiar with the codebase yet): it seems it would be beneficial to use React 16 as a peer dep now.

I notice that in the context file it is pulling from that is created, it's simply creating the context without regard to Provider -- also using an older polyfill for context. If we bump to React 16, we can just useMemo for the necessary properties, and then pass in a wrapper component ContextProvider similar to your approach. That would solve the issue since useContext only re-renders when the context provided with value changes.

As a side effect, this would also cut bundle size just slightly which is nice as react-router is such a core dependency nowadays for non-trivial routing, and not have to rely on a non standard way of implementing a context for react-router.

If I have some time later on in the day today, would definitely like to try to open a PR on that.

[MeiKatz]

@rob2d The story goes like this: we are still supporting React versions without support for React.createContext. Therefore we can support some more React versions. If you use a React version with support for React.createContext we are using this internal. But yes, maybe we could (should?) add mini-create-react-context as a peer dependency.

Now for the actual problem: we pass all of our api details via the react context. Therefore any change will cause a re-rendering and the use of useMemo will not help. The only way I can think about this would be to use a standalone context for the history object, as mentioned by @esealcode.

[narkai]

In reference to #7000
I was able to prevent history.push() from rerendering the whole app by encapsulating the call further down in a component. The small lag still occurs and is perceptible because of animation and probably wouldn't be noticed without it.
As useHistory will change according to the roadmap, maybe we'll wait for useNavigate() to come.

[MeiKatz]

Maybe one of the three options might help you guys: facebook/react#15156 (comment) (option 1 is kind of the solution used by @esealcode)

[narkai]

I tried to split contexts like @esealcode and to completely disconnect url change via history.push() from any other components / render. It basically just pushes a new random url by pressing a key, with its own context, and the animation slows down exactly at this moment. It seems that the only action of mutating history causes a framerate drop (animation are made via react-spring / request animation frame).

[rob2d]

@rob2d The story goes like this: we are still supporting React versions without support for React.createContext. Therefore we can support some more React versions. If you use a React version with support for React.createContext we are using this internal. But yes, maybe we could (should?) add mini-create-react-context as a peer dependency.

Now for the actual problem: we pass all of our api details via the react context. Therefore any change will cause a re-rendering and the use of useMemo will not help. The only way I can think about this would be to use a standalone context for the history object, as mentioned by @esealcode.

I can see the reasoning for maintaining backwards compat and usually pretty sensible. Just IMHO but it seems like supporting React < 16 in new releases might be a little bit overkill when increasing package size/lib complexity for everyone else. There's been a warning that context has been experimental since pre 16 I believe (which spans back > 2 years), and official Context API was released about 20 months ago initially in prod [also trust me I understand that time flies; it's hard to realize it's already been that long!].

For the main issue, completely makes sense. It could work to provide context a bit separate as that is sort of already provided optionally via 3rd party, so it's not entirely out-there to decouple that from the rest as that idea is somewhat consistent when considering the possible use cases of history itself. Aside from this, functionality with history could be deriven and expanded upon this way and provides some more flexibility (obviously a little off scope of primary issue though but fun to think about).

[narkai]

Using the history package directly proved to be a good solution for my use case, which only requires to read/write from the url.

[ricokahler]

The post @MeiKatz shared is great. I think a potential solution is to have the <Router /> component inject 2-3 contexts, possibly one for each hook react-router ships but maybe also something simpler.

I think the simplest solution is to ship the current context as-is + a history context but maybe there's a compelling reason to break the current context apart more.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
You can add the fresh label to prevent me from taking any action.

[afreix]

Can we keep this issue open?

[andrewleader]

Yeah this is kinda annoying. I've switched to using the history library so that my components don't re-render on every browser url change.

[m-sterspace]

Yeah, is there an update on this? Every time a low level component tries to 'useHistory' it causes the entire tree to rerender which is super slow.

At present I'm at somewhat of a loss for what to do aside from either writing my own useHistory hook or switch to a redux based router that allows history changes by dispatching actions....

[esealcode]

@m-sterspace Yep, there is a PR awaiting to be merged which fixes the issue by separating the history object to its own context. (see: #7103)

I'm not sure of when we can expect it to be merged though.

[esealcode]

#7103 got merged, so i think we can close this issue now.

Thank you @illuminist for the PR and all for your feedbacks :)