duplicate rendering

[pzmosquito]

all the components including node components are rendered twice. I tried the example project https://github.com/LeonardoGentile/react-mobx-router5-example, same result.

Here's how: put console.log in the render() method on both Home.jsx and Main.jsx in the example project, I get the log message twice.

react@16.0.0 mobx@3.3.1 mobx-react@4.3.3 router5@5.4.0 mobx-router5@3.0.0 react-mobx-router5@5.0.0

[LeonardoGentile]

Hello @pzmosquito sorry but I can't reproduce. I've updated the example project to the dependencies versions you specified but it worked fine for me.

From time to time this might happen only on the first page load, when you first launch your application, was that your case?

Otherwise what navigation exactly triggered the multiple rendering?

[pzmosquito]

yes, it happens on first launch. Even though it's not causing any issue for now, I think it's still necessary to figure out why and get it fixed in case it does cause problems when application gets more complicated.

[LeonardoGentile]

Take a look from this line up to the render body. That is where it relies the logic telling routeNode when it should re-render.

Can you spot any bug? Btw take a look at this long discussion https://github.com/mobxjs/mobx-react/issues/230 talking exactly about this problem.

Let me know if you can see a better way to handle this: to re-render only when strictly necessary.

[LeonardoGentile]

Sorry I haven't touched that code in a while. After having a look now I remember why this is happening and I can confirm this is happening only when the app is first launched.

This is the relevant code deciding when the routeNode components should re-render:

// Compute a new observable used by autorun
@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
      });
    }
  });
}

componentWillUnmount() {
  this.autorunDisposer();
}

render() {
  const {route} = this.state;
  const plainRoute = toJS(route); // convert to plain object
  return createElement(RouteComponent, {...this.props, [storeName]: this.props[storeName], route, plainRoute});
}

When the application starts and the components wrapped with routeNode are mounted their render method will be called for the first time. After the component has mounted the autorun is registered so that from now on the component will re-render on the condition this.routerStore.intersectionNode === this.nodeName => so that's the second time the render method runs.

After the first mount this should not happen again.

Anyway this shouldn't be a big deal because react still will manage to update the DOM only if things have really changed, so calling render multiple times shouldn't be that expensive (the whole point of react).

[pzmosquito]

The long discussion in mobxjs/mobx-react#230 makes me think if MobX and router5 is a good combination.

[LeonardoGentile]

@pzmosquito apart from the double rendering on the first page load I am not having any particular problem with this package. I'm using it for building a complex app.

Keep in mind that being this a plugin and not an app itself it is normal that it might fall a bit outside of the standard way how mobx-react is normally used, but it my opinion I've found a good solution (the above logic).

If you use another router5 plugin (for example the react-router5) I am not even sure if there is this extra check I've implemented (only render when necessary). That might not be super optimized but it will still work because as I've told you react will take care of updating the DOM only when necessary.

I think this issue can be marked as solved (regarding the multiple render call). If you have other question please open another issue

[aap82]

I think using reaction instead of autorun to track this.isIntersection should solve this issue because:

Unlike autorun the side effect won't be run directly when created, but only after the data expression returns a new value for the first time.