Nested RouterNode's do not react when matching same route

[thupi]

Hi,

I have been using this router in a production application for a while now and is very happy with the result.

Today when i was working on some new features i discovered a bug og rather a issue with the way routeNode's with mobx.

When a route matches the same route even though the route changed(params change) the child RouteNode is not updated beacause the interception point is the parrent route due to the param change and the parrent router matches the same route so the child Router do not get rerendered.

See this reproduction: https://stackblitz.com/edit/react-azsnhb :-)

1. Click on "Page 1 / Article"
2. Click on "Page 2 / Info"

The unexpected behavior is that the nested router statys at article even though the route is updated to the info page. :-)

[LeonardoGentile]

Hello @thupi I'm looking into this, but I still can't figure out what's going on.

Your Navigation is:

 "app.page.article", params:{id:1} -> "app.page.info", params:{id:2}

In this case the intersection is the "app" node, that is the AppRouter component, so this should re-render and in turn all its children should be re-rendered, so also Page -> PageRouter -> Info and it shouldn't matter that PageRouter is not the intersection node because at this time the parent (AppRouter) should have forced the re-rendering of all its children.

But instead this weird behaviour happens:

• AppRouter re-render with the correct route injected (OK)
• Page re-renders (OK)
• PageRouter re-render BUT for some reason it has the old route, so it will select the wrong component (Wrong)
• Article re-render instead of Info because of previous point (Wrong)

I suspect that something in react or mobx has changed about timings and internal priorities, most probably setState (used here)

Let me know if you can spot anything that might help figuring out the problem, thanks!

[LeonardoGentile]

Ok, this seems to be a bug on my side.

The logic to handle when a routeNode should re-render is this:

@computed get isIntersection() {
  return this.routerStore.intersectionNode === this.nodeName;
}

componentDidMount() {
  this.autorunDisposer = autorun(() => {
    // Change state only if this is the correct "transition node" for the current transition
    // This will re-render this component and so the wrapped RouteSegment component
    if (this.isIntersection) {
      this.setState({
        route: this.routerStore.route
      });
    }
  });
}

Now when navigating from /a/1/b to /a/2/c it fails.

Similar packages, like react-router5 don't seems to have this extra check and they just re-render all the time.

Removing the extra verification if (this.isIntersection) should fix the problem. I wanted to prevent this but now this seems a bit buggy.

Anybody has a better idea to achieve selective re-rendering (only when needed and for the right components)?

[thupi]

@LeonardoGentile Couldn't it be fixed if you also check if the segment has been changed from the previous route to the current route inside the computed function? :-)

The might result in the all routeNodes checks if there child segment has bee changed and re-render if so. :-) ?

[LeonardoGentile]

I think one option could be:

@computed get shouldUpdate() {
  const isIntersection = this.routerStore.intersectionNode === this.nodeName;
  const toActivate = this.routerStore.toActivate.find((item, index, array) => {
    return item === this.nodeName
  });
  return isIntersection || toActivate
}

componentDidMount() {
  this.autorunDisposer = autorun(() => {
    // Change state only if this is the correct "transition node" for the current transition
    // Or the segment represented by the node is one of those that needs to be activated
    // This will re-render this component and so the wrapped RouteSegment component
    if (this.shouldUpdate) {
      this.setState({
        route: this.routerStore.route
      });
    }
  });
}

[carlosagsmendes]

Hi @LeonardoGentile, is there a workaround until this issue gets fixed?

[LeonardoGentile]

Hello, no sorry. I've replicated the issue also in the react-router5 package and awaiting the router5 creator for an answer because I'm still not sure if this is an implementation error on my side, a misconfiguration or just something router5 can't do.

[carlosagsmendes]

Thanks for the heads up Leonardo.

[LeonardoGentile]

I've got a potential solution here.

Is anybody available to review this?

[thupi]

@LeonardoGentile That seems to be a good solution for now :-)

I am convinced that this should solve the problem :-) Do you want me to test it in one of my projects :-) ?

[LeonardoGentile]

@thupi yes if you could please. It's not published on npm so you can clone or download it, then cd into it and activate the issue6 branch and npm link. Finally cd into your project where you use this package and npm link react-mobx-router5.

I know this should fix the original problem but could you please double check it doesn't trigger any extra unwanted updates?

Thanks!

[thupi]

@LeonardoGentile I tried to take a look at but i discovered that the projects i work with where we are using the packages has been redesigned in the meantime. It would need to recreate the sample i made for this issue in order to test that behavior :-)

I think I will be able to find the time to recreate the sample in the coming weekend :-)

However, since the solution is implemented in router5 you might not need the same implementation in this package right :-) ?

[LeonardoGentile]

:tada: This issue has been resolved in version 6.1.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[LeonardoGentile]

@thupi if you have the time I've found another solution, it's version 6.1.0 and it relies on router5@6