useRouterMatch returns new object on each render

[bkazi]

Version

5.1.2

Test Case

https://codesandbox.io/s/eloquent-wave-vlixh

Steps to reproduce

CodeSanbox has a reproducible case

Expected Behavior

useRouteMatch hook returns a match object, when combined with a useEffect it is expected that the effect will run only when the window location or path parameters change

Actual Behavior

useRouteMatch hook returns a match object, when combined with a useEffect with the match as dependency the effect is triggered on every state change.
The codesandbox example performs a state update on a timeout and logs match whenever it is changed. The logs show that on every update match has changed even if the path has not. Memoizing the match object on window.location.pathname prevents this and has the expected behaviour

This line in the hook returns the result of matchPath which creates a new object every time it is executed.
Possible solution would be to do something like

return path ? 
  useMemo(() => matchPath(location.pathname, path), [location.pathname, path]) :
  match

[timdorr]

I think that makes sense. Can you submit a PR?

[pavelkuznetsov]

Hi! There was kind of similar question about new match object created on each render - #5099 .

The issue was closed because it is intentional, but now, with the useRouteMatch, the answer is that memoization could help.

@timdorr, is there any difference in these cases? Perhaps, Route could pass memoized match to its component too? It seems like it could help with kind of unnecessary re-rerenders described in the issue I linked above.

P.S. It could really help people like me, which migrating from RR3 to the last one and still use redux and react-redux. After migration we found out that our 'connected pure components' from react-redux connect HOC, which are passed to Route's component prop, aren't pure anymore.

[timdorr]

Yes, that was when using lifecycle methods and a class component. You could "memoize" by way of shouldComponentUpdate to prevent a render. With Hooks, you're already in a render cycle, so it's not possible to bail out. That's why useMemo/useCallback even exist, so you can safely memoize values directly.

[pavelkuznetsov]

Thanks for quick reply!

As i understood, you don't think that it makes sense if Route could memoize match object out of the box (like it was described for useRouteMatch) in order to recalculate and pass new match object to component props only when the location or path parameters have changed?

[LoiKos]

I face the same problem with useRouteMatch today what is the status of this issue ?

[lauterry]

same issue with useLocation

const location = useLocation();
const history = useHistory();
...

useEffect(() => {
	console.log(location)
        history.replace({ pathname: "/currentPath", state: { comeFromError: undefined } });
}, [location]);

Got infinite loop.

To detect location change with useEffect, I use as a workaround or solution :

const location = useLocation();
const history = useHistory();
...

useEffect(() => {
	console.log(location)
        history.replace({ pathname: "/currentPath", state: { comeFromError: undefined } });
}, [location.pathname]);

Is it the right way ?

[LoiKos]

I tried to reproduce infinite loop in my code for useLocation and i wasn't able to reproduce.

But my Location object didn't contain hash or search maybe your problem come from here but obviously if your looking for pathname changes you should definitevely put only this part in dependencies. No need to look for other changes if your not using them.

[lauterry]

@LoiKos Sorry, I have just noticed that my code was not complete. I was calling history.replace to reset the location state.

So I have updated my code. @LoiKos Can you managed to reproduce the infinite loop ?

[LoiKos]

Yes i can reproduce the infinite loop which is normal btw. You try to add a state with history.replace({ pathname: "/currentPath", state: { comeFromError: undefined } }); but as state is a part of location you create an infinite loop. If you only want to create the state the first time you came on /currentPath then you have to listen to location.pathname as you find.

In my opinion the best practice with hooks dependencies is to be as accurate as possible on which variable you want to listen to. That avoid unnecessary operation and bugs that come from an object getting updated but not the part you want to look after.

I think you also forget to add history in your dependencies

[bluefire2121]

Still looking for an update. Can this be memoized inside the package?

Scenario:
When using useRouteMatch() and a primative property of match.params, the property can't be passed into the useEffect() dependency array because it won't exist if there isn't a match.

Can't memoize the useRouteMatch() hook inside useMemo's callback either.

What's the solution?

Edit:
I've opted for using a regular expression on location.pathname instead.

[olejorgenb]

A bit off topic, but simply using useLocation inside a memo'ed component will cause a rerendering if any Route above the component rerenders: https://codesandbox.io/s/festive-greider-8did7

The location value isn't changed so it is trival to work-around, but quite a gothca IMO. facebook/react#15156 (comment) for a related discussion.

[richardscarrott]

If you want to perform side-effects on location change, then I think you can prob just rely on location:

const location = useLocation();
const match = useMatch();
useEffect(() => {
  // The location has changed
  nonIdempotentSideEffect(match.params.foo);
}, [location])

Or I think you could make match stable in user land like this:

const useStableRouteMatch = () => {
  const _match = useRouteMatch();
  const location = useLocation();
  const [match, setMatch] = useState(_match);
  useEffect(() => {
    setMatch(_match);
  }, [location]);
  return match;
};

// ...

const match = useStableRouteMatch();
useEffect(() => {
  // The location has changed
  nonIdempotentSideEffect(match.params.foo);
}, [match]);

However, I don't think using React.useMemo would be safe unless you were merely trying to improve render performance as there's no guarantee React won't just throw away the memo'd value, e.g. don't do this:

const useMemoRouteMatch = () => {
  const match = useRouteMatch();
  const location = useLocation();
  return useMemo(() => match, [location]);
}

// ...

const match = useMemoRouteMatch();
useEffect(() => {
  // The location has changed *or react has discarded the memo'd match value*
  nonIdempotentSideEffect(match.params.foo);
}, [match]);

[vladshcherbin]

Same for other hooks like useParams. It triggers a rerender on every route change, even if there are no params in url. So basically using hooks = trigger app rerender on every route change.

Maybe passing props you want to subscribe to into this hooks may help.

[andreybs11]

Nested routes trigger the infinite re-render loop. Would be great to memoize the hooks +1

[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.

[dtokarczyk]

+1

[Themonkey180]

I am still having this issue, is there a update on it?

[dwelle]

Unclear on what's the stance on <Router match> object memoization --- should we create a new issue for that?

(Haven't checked if v6 changed the behavior already.)

[Powersource]

In the meantime this history.listen function works pretty well for me https://github.com/ReactTraining/history/blob/v4/docs/GettingStarted.md#listening

[Powersource]

An example of how I use history.listen

function useListenHistory(fn) {
  const [state, setState] = useState(fn());
  const history = useHistory();

  useEffect(() => history.listen(() => {
    const newState = fn();

    if (newState !== state) {
      setState(newState);
    }
  }));

  return state;
}

export function useAtHome() {
  const getAtHome = () => window.location.pathname === '/';
  return useListenHistory(getAtHome);
}

[mafergus]

This is causing all kinds of unnecessary re-rendering, we've had to write our own set of wrapper functions to compensate

[jahglow]

An example of how I use history.listen

function useListenHistory(fn) {
  const [state, setState] = useState(fn());
  const history = useHistory();

  useEffect(() => history.listen(() => {
    const newState = fn();

    if (newState !== state) {
      setState(newState);
    }
  }));

  return state;
}

export function useAtHome() {
  const getAtHome = () => window.location.pathname === '/';
  return useListenHistory(getAtHome);
}

your example has zero memoization itself neither in getAtHome nor in useEffect and guess what, your useEffect is triggered on every render. Add deps to useEffect and wrap getAtHome with useCallback.

[jonkoops]

I've opened up a PR to start a discussion on how this issue could be resolved in #7822.

[zac-robinson]

Is there any update on the PR? I can see all the check have passed, but there looks like no activity after April

[chaance]

The update is that I'll be reviewing in the next week or two. This one is on the radar.

[Powersource]

fwiw I think this might have been a bit of an XY problem for me. I was blaming this library for rerendering too much (which might still be an issue that should be resolved), but I think that might have been such a big issue for me because when a certain component was rerendering for me, I had coded it poorly, and the component was throwing out a bunch of cache data (specifically i think i was throwing out an already initialized apolloClient). The real solution for me was probably just to stop reinitializing too many things on every render, instead of rerendering less.

[jonkoops]

I think this issue can be closed as the useRouteMatch() function no longer exists.

[SimenB]

It's just renamed to useMatch

react-router/packages/react-router/index.tsx

Line 409 in 3485453

export function useMatch<ParamKey extends string = string>(

[jonkoops]

Created a new PR (#8431) to resolve this issue.

[timdorr]

That's now released in 6.1.0! :)