You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
constComp1=()=>{useEffect(()=>alert(`Comp1 was rendered`));};constComp2=()=>{useEffect(()=>alert(`Comp2 was rendered`));};<Switch><Routepath='/1'><Comp1/></Route><Routepath='/2'><Comp2/></Route></Switch>
And I navigate as follows:
Navigate to /1.
This renders Comp1 in the first route and executes the effect: I see the alert "Comp1 was rendered".
Navigate on to /2.
This renders Comp2 in the second route and executes the effect: I see the alert "Comp2 was rendered".
I press the browser's native "back" button.
The URL in the browser immediately changes to /1.
Expected behavior:
The second route for /2 does not match anymore, so Comp2 in it won't render, the effect is not executed.
The first route for /1 now matches again. The effect for that Comp1 is executed, I see the alert "Comp1 was rendered".
Actual behavior:
The second route renders its children again, Comp2's effect gets executed, I see the alert "Comp2 was rendered".
Then the first route renders its children, Comp1's effect gets executed, I see the alert "Comp1 was rendered".
Note
This render of Comp1 should not happen, since the URL doesn't match its route anymore!
Cause analysis:
Analyzing this I saw that Switch loops over all its children to see which route matches. When one matches, it renders only that route (which is fine), but also (via cloneElement) passes down an extra prop match with information about the details of the match:
// we don't require an element to be of type Route,
// but we do require it to contain a truthy `path` prop.
// this allows to use different components that wrap Route
// inside of a switch, for example <AnimatedRoute />.
(match=matchRoute(
router.parser,
element.props.path,
location||originalLocation,
element.props.nest
))[0]
)
returncloneElement(element,{ match });
Presumably, this is done for performance reasons: Since Switch already determined that the Route in question does match, there is no reason to have Route determine that same fact a second time.
But I don't think this works when using the browser's back/forward buttons - the timing seems to be somehow different compared to using a <Link>:
When using a Link: The change seems to be controlled by React and processed "synchronously" and "in order" - "top down" if you will. The Switch gets rendered first, has a chance to react to the new location from the Link, and won't render the now-no-more-matching Route at all.
When using the browser's back button: The change is probably propagated "asynchronously" to all components, including the "old" Route which is subscribed to the location via useLocationFromRouter(). It re-renders, and it would not match anymore under the new location, but since it still has its match prop which it got passed down from Switch, it believes that it does still match and does not re-check if there is a new location. So it renders its children - which won't become visible anymore because Switch will unmount the Route soon after, but the Route's child component will execute any effects it has defined.
I don't really know how to circumvent this without breaking the performance/caching optimization that the match prop offers, but for my project this causes enough problems with browser back behavior that I'm going to change my Route implementation to something like this:
constRouteWithoutMatch=(props: RouteProps)=>{// @ts-ignore Do not let `Switch` pass a `match` prop to `Route`const{ match, ...restProps}=props;return<Route{...restProps}/>;};
Now the matching Route inside a Switch will have to do the matching again, but it does prevent the weird errors I'm seeing for unmounted components executing their effects (and with the wrong combination of params at that, because those come from the stale match prop).
NOTE THAT YOU HAVE TO USE THE "DEPLOYED" VERSION OF THE SANDBOX, not the internal preview browser. So when the server is running, click "Open externally" to open the real (https://<something>.csb.app/) URL and see the issue. It does not happen with the back button in Codesandbox's internal browser preview widget.
This also contains a path with a Route variant that leaves out the match prop, and you can see that everything is fine there.
Maybe someone has an idea how to fix this while keeping the performance optimization? Or maybe that optimization has little enough impact that it could be removed?
Tested with:
Suppose I have routing configured like this:
And I navigate as follows:
/1.Comp1in the first route and executes the effect: I see the alert "Comp1 was rendered"./2.Comp2in the second route and executes the effect: I see the alert "Comp2 was rendered"./1.Expected behavior:
/2does not match anymore, soComp2in it won't render, the effect is not executed./1now matches again. The effect for thatComp1is executed, I see the alert "Comp1 was rendered".Actual behavior:
Comp2's effect gets executed, I see the alert "Comp2 was rendered".Comp1's effect gets executed, I see the alert "Comp1 was rendered".Note
This render of
Comp1should not happen, since the URL doesn't match its route anymore!Cause analysis:
Analyzing this I saw that
Switchloops over all its children to see which route matches. When one matches, it renders only that route (which is fine), but also (viacloneElement) passes down an extra propmatchwith information about the details of the match:wouter/packages/wouter/src/index.js
Lines 335 to 348 in 2061eac
Presumably, this is done for performance reasons: Since
Switchalready determined that theRoutein question does match, there is no reason to haveRoutedetermine that same fact a second time.wouter/packages/wouter/src/index.js
Lines 244 to 247 in 2061eac
The content of that
matchvariable (or rather its first array element, thematchesboolean) decides whether theRoute's child is rendered or not:wouter/packages/wouter/src/index.js
Line 254 in 2061eac
But I don't think this works when using the browser's back/forward buttons - the timing seems to be somehow different compared to using a
<Link>:Link: The change seems to be controlled by React and processed "synchronously" and "in order" - "top down" if you will. TheSwitchgets rendered first, has a chance to react to the new location from theLink, and won't render the now-no-more-matchingRouteat all.Routewhich is subscribed to the location viauseLocationFromRouter(). It re-renders, and it would not match anymore under the new location, but since it still has itsmatchprop which it got passed down fromSwitch, it believes that it does still match and does not re-check if there is a new location. So it renders its children - which won't become visible anymore becauseSwitchwill unmount theRoutesoon after, but theRoute's child component will execute any effects it has defined.I don't really know how to circumvent this without breaking the performance/caching optimization that the
matchprop offers, but for my project this causes enough problems with browser back behavior that I'm going to change my Route implementation to something like this:Now the matching
Routeinside aSwitchwill have to do the matching again, but it does prevent the weird errors I'm seeing for unmounted components executing their effects (and with the wrong combination ofparamsat that, because those come from the stalematchprop).Codesandbox with repro
See here for an example where this happens: https://codesandbox.io/p/github/cataclyst/wouter-bug-repro/main
NOTE THAT YOU HAVE TO USE THE "DEPLOYED" VERSION OF THE SANDBOX, not the internal preview browser. So when the server is running, click "Open externally" to open the real (
https://<something>.csb.app/) URL and see the issue. It does not happen with the back button in Codesandbox's internal browser preview widget.This also contains a path with a
Routevariant that leaves out thematchprop, and you can see that everything is fine there.Maybe someone has an idea how to fix this while keeping the performance optimization? Or maybe that optimization has little enough impact that it could be removed?