React-native-router-flux: tracking React Native 0.24 compatibility/issues

[joenoon]

I'm opening this to track problems and gather information as I dig into this. If anyone has ideas or figures it out first, please update here. It looks like after upgrading to RN 0.24 there is an issue in DefaultRenderer not receiving the expected navigationState, and getting:

undefined is not an object (evaluating 'navigationState.index')

Logging the keys of navigationState inside DefaultRenderer, I see:

 [ 'layout',
  'navigationState',
  'onNavigate',
  'position',
  'scene',
  'scenes' ]

So it seems like its not getting the right value here for navigationState in 0.24. I did scan through the diff of NavigationExperimental on react-native between 0.22 and 0.24 but didn't see anything obvious.

I'll update here if I figure it out.

[joenoon]

I think I found a fix in the PR above

[sylvainbaronnet]

I upgraded RNRF to 3.22.22 and now the custom NavBar doesn't show up at all.

With RN 0.24.1

[sylvainbaronnet]

I tried previous versions of RNRF and the last version where the custom NavBar show up is the 3.22.8
With the 3.22.11 there's the bug. There's a change between these versions that introduce the bug.

(3.22.9 & 3.22.10 were not published to npm so I can't try them)

EDIT : with aksonov/react-native-router-flux#3.24 it works

[joenoon]

In case other people land here, as of today I'm using this combination successfully:

    "react-native": "0.24.1",
    "react-native-router-flux": "aksonov/react-native-router-flux#3.24"

[sylvainbaronnet]

There's still the problem with type="replace" but it's a RN bug

[jamesone]

@sylvainbaronnet Any update on a fix for this?

[aksonov]

@joenoon Could you check if it is possible to merge 3.24 with latest branch and upgrade it to usage with 0.26-rc? This way we will create new branch 3.26 and then will replace master when official 0.26 will be released.

[aksonov]

@sylvainbaronnet Maybe could you integrate 3.24 into master and then update imports?

[sylvainbaronnet]

Sorry I deleted my message because I realized imports were already updated in master.

[sylvainbaronnet]

I tried to merge 3.24 into master but there's too many conflict in src/NavBar.js and src/DefaultRenderer.js and I don't know the project enough to resolve them.

[aksonov]

Probably they are caused by eslint formatting done for master branch....

[aksonov]

Also I did some improvements for NavBar component.

[sylvainbaronnet]

yes esling formatting but not only. If you want to take a look :

https://gist.github.com/sylvainbaronnet/3dcd34832466abe7be012d9f67d824d5

https://gist.github.com/sylvainbaronnet/a2671d94e9f548aa169473e871d476b0

[joenoon]

@aksonov I attempted to update my app to 0.26-rc last night and decided it was too early for me. I made all of the required changes such as require react vs react-native, etc. But the real problem was downstream in other npm modules I relied on that had not yet done the same and probably wont for a while.

But the good news for me is the 3.24 branch works fine with react-native 0.25.1. I had branched to a 0.25-stable branch in my own fork thinking I would have to make some changes, but haven't needed to yet. However I remember there are still some issues with the 3.24 branch - I guess my app just doesnt run into them.

The one issue I do run into but not sure if its known is - when switching tabs the entire UI seems to re-render (loses scroll position etc). I've been meaning to try and track this down for a while - maybe I can figure it out this week.

@aksonov One thing I'm trying to wrap my head around is if we should be using NavigationStateUtils or not? Are we not using it on purpose or because it wasn't around initially?

[aksonov]

"The one issue I do run into but not sure if its known is - when switching tabs the entire UI seems to re-render (loses scroll position etc). I've been meaning to try and track this down for a while - maybe I can figure it out this week." - actually i've met this problem and it is very critical bug...

NavigationStateUtils seems necessary if user wants to use NavigationExperimental directly....

[mvaivre]

@aksonov Yep, I think I have this problem too: while switching (push) to a scene, the component associated to this scene is re-mounted, and I end up having several instance of the same component! If it's not a mistake on my side, then it's indeed critical. (3.24 branch with react-native 0.25.1).

[joenoon]

I've been trying to track down the re-mounting issue and so far am having no luck. From the basic logging I've added locally, it looks like previous components are unmounted as new ones are mounted when switching tabs - so its _not quite as bad_ as it could be.

I'm still trying today - does anyone know if this happens in older react-native + rnrf master combos?

When I follow the component lifecycles I get to https://github.com/aksonov/react-native-router-flux/blob/3.24/src/TabBar.js#L43 where _renderScene returns a <DefaultRenderer key={props.scene.navigationState.key} navigationState={props.scene.navigationState}/>. In a way, it seems to make sense its unmounting when switching tabs since this new key becomes the only "content" child. But at some point it worked... so still digging in.

[joenoon]

Its looking more like this is a problem in RN, so now I'm focusing efforts on getting my app to work with RN 0.26-stable branch which includes the commits that look relevant. https://github.com/ericvicenti/navigation-rfc/issues/69

Will update here as I know more

[joenoon]

Unfortunately the changes upstream in RN that I thought would fix the tab-switching issues actually didn't fix it. Now my app works with 0.26 though, which was quite a challenge in itself.

I've started a 0.26-wip branch: https://github.com/aksonov/react-native-router-flux/tree/0.26-wip

It brings back the old behavior where all tabs are loaded at the same time, however there is a bug I haven't been able to figure out yet related to the "selected" tab changing on load.

Basically I added a new TabbedView which is pretty much like the _old_ NavigationView from RN 0.22 . The NavigationView in RN > 0.22 is quite a bit different and I think the problem is its no longer the right view for implementing tabs. It only renders the current scene.

If anyone can help me try to figure out whats going on with the selected/index issues, that would be really helpful. It has something to do with Reducers.js#inject, but there is a lot of logic there that I haven't fully understood yet.

[jamesone]

@joenoon @aksonov - I'd find it a lot easier if the code were commented? I'd like to start contributing a bit, but the code is foreign to me! I'm not really sure what's doing what, I think comments would help developers new to the project.

[joenoon]

@jamesone I know what you mean. I think there is a lot going on - NavigationExperimental changes upstream, RNRF used to be its own thing from my understanding (2.x), but now more of an extension/default/implementation on top of NavigationExperimental (3.x). But its sometimes difficult to keep straight where NavigationExperimental ends and RNRF begins. But please feel free to ping me on gitter and I'll do my best to work through any questions.