[Navigator] don't render twice for every navigation

[chirag04]

fixes #952

cc @hedgerwang @ericvicenti @ide

[ericvicenti]

This will fix that bug, but it will also prevent the current scene from getting re-rendered when the Navigator gets re-rendered. Is that the intention? I'm concerned because people probably rely on that behavior to some extent

[chirag04]

Yes. I think it is reasonable to not rely on navigator to re-render the current scene. isn't it?

Now that i think about it, is there a case where you would want navigator to re-render the current scene? I would listen to store change and re-render based on the state.

[hedgerwang]

So you'd get the same old copy of rendered scene even when this.state.presentedIndex had changed.
Some people may rely on using the the index to determined whether a scene is active or not and this change will break that.

If the scene can be expensive to render, the scene itself should use PureRenderMixin to protect itself from being overly rendered by the navigator.

[chirag04]

this.sate.presentedIndex is private so that shouldn't break anything ideally. I maybe wrong.
But i think replace will break where you would want to re-render the current scene?

Happy to close this.

[brentvatne]

I think the consensus here is that we should leave this behaviour as-is for now. Navigator is being worked on internally at fb right now so let's see how that shakes out and then maybe revisit this later on @chirag04

[chirag04]

👍

[shahankit]

@chirag04 I'm using react-native 0.13.2. I'm still facing the issue everything gets called for each element in routestack, my navigation bar get's rendered for all route in routes and so takes the properties of the first route. Did you solve the issue?

[adriansdev]

I see this issue with CameraRollView and Navigator. If you click an image and navigate to a new view, the roll is re-rendered, and again if you go back to the roll, which is not nice to look at for CameraRoll

The pending commit chirag04@31e2f40 by @chirag04 fixes the problem for me and makes transitions in general smoother

An option to disable re-render of an existing scene would be good. I don't expect the current scene to rerender as it moves to the next.

[chirag04]

@adriansprod There are side effects to that commit of mine. navigator replace will not work i guess.

Adding shouldComponentUpdate is the best way to avoid re-rendering your component.

[eysi09]

I'm having similar issues where the extra renderScene call slows down the animation to the point of being unusable, even though I first render an empty placeholder screen on the new scene.

I've tried doing something like this in my renderScene:

if (this._prevRouteID === route.__navigatorRouteID) { return null; }

However, that breaks the pop functionality of the Navigator, causing the scene I'm popping to to be mounted again. It's a scene that contains a ListView and I want to be able to navigate away from the scene and back again to the same position as one would do on e.g. the Facebook app. @chirag04 's commit solves this.

Additionally I need to be able to update the parent component to the Navigator to be able to change the navigationBar background color. @chirag04 's commit also allows me to do that without causing an extra renderScene (not even sure why).

So to sum up, for smooth animation and pop functionality that preserves the previous route (and as a bonus not having to keep track of when what should render) I think chirag04@31e2f40 get's the job done. Would be awesome if the side effects were fixed and the PR accepted.

[chirag04]

@ericvicenti @hedgerwang seems like a lot of people are running into this. Not re-rendering the previous scene on every transition would be awesome.

Wondering if i can do anything to still get this merged without the side-effects of breaking replace and any other.

[cldwalker]

I'm running into this on RN 0.20.0. I'm seeing the previous and the current route get passed to renderScene on every Navigator.push which is surprising and undocumented behavior. We should document renderScene is called twice if that is expected

@chirag04 For the suggested shouldComponentUpdate solution, are you suggesting a check of current pushed route == last of navigator.getCurrentRoutes()?

[geekvijay]

i am also facing the same issue with RN 0.41.2.
navigator.push() is causing renderScene() being called twice.

is there any solution to this ?