Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NavigatorIOS] doesn't update when passProps update #795

Closed
lazywei opened this issue Apr 10, 2015 · 33 comments
Closed

[NavigatorIOS] doesn't update when passProps update #795

lazywei opened this issue Apr 10, 2015 · 33 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@lazywei
Copy link

lazywei commented Apr 10, 2015

I'm not sure if this is a feature or a bug:

When I change the value of passProps of a navigator, I expect it would re-render again, just like other react components do. However, it seems it will not do so.

For example,

          <NavigatorIOS
            style={{flex: 1}}
            initialRoute={{
              component: MyChatList,
              title: "My Chat List",
              passProps: {
                recentChats: this.state.ChatListData.recentChats,
                renderRow: this.renderRecentChat,
              },
            }} />

When the state ChatListData (which comes from flux) updates, the navigator will not update it self correspondly.

@bakso
Copy link

bakso commented Apr 10, 2015

I don't know

@ericvicenti
Copy link
Contributor

initialRoute only specifies the initial scene in the navigator.

The reason we don't do this is we don't want to re-render all of the scenes every time you re-render NavigatorIOS. This would make the app very slow once you have several scenes open. If your props are changing, you can call navigator.replace(newRoute), or use event emitters to communicate to inner scenes.

Navigator handles this case a bit better. When you re-render, it will call renderScene again for the currently-visible scenes (unless you are mid-transition).

I'm still trying to figure out the ideal pattern for re-rendering internal scenes. Feedback and ideas are welcome!

@lazywei
Copy link
Author

lazywei commented Apr 11, 2015

@ericvicenti Oh, I totally forgot I can use replace. I feel that is a little anti-react though. I mean, monitoring new props ourself. On the other hand, using event emitters is probably more like Flux, right? However, I'd personally prefer keep the states on the top-most components and let it pass props down, instead of let the navigator listen to the store directly.

For the Navigator, would it also be slow if I keep re-render (new props keep coming, for example)?

Thanks for your elaborating.

@mwilc0x
Copy link
Contributor

mwilc0x commented Apr 11, 2015

I too ran into this when I first started writing a RN app.

Is there any way that calling setState can update props to any potential children on the same route like with React?

e.g.

  _buttonPress() {

    this.setState({
      input: 'heyo'
    })

  },

  render() {
    return (
      <View>
         <ChildComponent input={this.state.input} /> // input prop change will be heard by Child
      </View>
    )
  }

I understand that on mobile you're probably not building as complex UI Component structures for a given single route. I'm not sure what the benefits/trade-offs is/are here.

@ericvicenti
Copy link
Contributor

@mjw56, That code should work, the input prop would be updated when setState is called. Are you having difficulties when putting ChildComponent inside a Navigator or NavigatorIOS?

@lazywei, I agree that it would be nicer to re-render the scene when the NavigatorIOS gets re-rendered. It would be great if somebody could implement Navigator's renderScene API for NavigatorIOS. That way the initial route would remain, but you could re-render the scene according to changing state outside the navigator.

When it comes to slowness from re-rendering in Navigator, it shouldn't be a problem because Navigator is careful to only re-render scenes that are visible. This would also be important to do if somebody adds the renderScene API to NavigatorIOS.

You might run into issues if you want to re-render aggressively, but you could always improve the shouldComponentUpdate logic in your scenes.

@ustun
Copy link

ustun commented Apr 14, 2015

It also doesn't update when navigationBarHidden is changed, see #846

@brentvatne brentvatne changed the title NavigatorIOS doesn't update when passProps update [NavigatorIOS] doesn't update when passProps update May 31, 2015
@mars
Copy link

mars commented Jun 6, 2015

Still an issue with React Native 0.6rc. Workaround is calling this.refs.navigator.replace(…) with the updated state in passProps.

@Iragne
Copy link
Contributor

Iragne commented Jun 24, 2015

may be this PR can solved your issue ?
#1733

@edo1493
Copy link
Contributor

edo1493 commented Jul 9, 2015

@mars: It's a Workaround if you haven't got anything in NavigatorIOS. What if you have pushed several routes?

I am trying to use resetTo() in order to reset the whole thing every time new props come in. (http://stackoverflow.com/questions/31320583/invariant-violation-calling-pop-to-route-for-a-route-that-doesnt-exist)

Any idea?

@mars
Copy link

mars commented Jul 9, 2015

@edo1493 I ended up using the more manual but less limiting React.Navigator and …navigator.immediatelyResetRouteStack(). See Navigator docs

@edo1493
Copy link
Contributor

edo1493 commented Jul 9, 2015

@ericvicenti have you seen this? Thanks.

@ghost
Copy link

ghost commented Aug 4, 2015

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.

@ericvicenti
Copy link
Contributor

Is this issue about Navigator or NavigatorIOS? I see references to both in here.

For clarification about the difference, check out @brentvatne's comparison

@mars
Copy link

mars commented Aug 5, 2015

@ericvicenti this issue is w/ NavigatorIOS

Using the Navigator module instead re-renders routes/scenes at the appropriate times.

@edo1493
Copy link
Contributor

edo1493 commented Aug 5, 2015

The issue is with NavigatorIOS.
Since I didn't have to re-render anything, I have solved with an EventEmitter that pass the updated props from componentWillReceiveProps.

@ericvicenti
Copy link
Contributor

Ok, that sounds like a reasonable workaround for now. We currently don't use NavigatorIOS at Fb, so its up to the community to get this one fixed.

@a2 a2 removed their assignment Aug 6, 2015
@chirag04
Copy link
Contributor

@ericvicenti @vjeux @brentvatne this may sound crazy but should react native move NavigatorIOS out of core?

It can live in userland and will be better supported i guess if it's in userland.

@aphillipo
Copy link

Yes. Please either remove it so people don't get confused or put a big red warning in the docs saying "this component breaks the underlying working of react". Wasted time until I found this bug.

@browniefed
Copy link
Contributor

@chirag04 there is already this #1519

svevang added a commit to jackpine/totem that referenced this issue Oct 8, 2015
My original plan was to filter global state down through the component
hierarchy, but given this bug:
facebook/react-native#795 It seems that we
need to inject state below the level of the ios navigator component.
@edo1493
Copy link
Contributor

edo1493 commented Oct 28, 2015

I know this is a bit off-topic, but is there a way to update the props of each component in the stackRoute of Navigator?

I am updating the state where Navigator is rendered, but the props don't seem to update. TabBar does this with StaticContainer, I am trying to find an Android workaround.

I think forceUpdate() just made my day. 😄

Update: which doesn't seem to really work 😢

@bokuweb
Copy link

bokuweb commented Oct 30, 2015

I use v0.13.1.
I create getter method to get updated props, and pass to NavigatorIOS.
It is easy to keep the states on the top-most components and let it pass props down.
In this case there will be a problem?

For example,

  getRecentChats() {
    return this.state.ChatListData.recentChats;
  }
  render() {
    return (
      <NavigatorIOS
        style={{flex: 1}}
        initialRoute={{
          component: MyChatList,
          title: "My Chat List",
          passProps: {
            recentChats: getRecentChats.bind(this),
            renderRow: this.renderRecentChat,
          },
       }} />
    );
}

ghost pushed a commit that referenced this issue Dec 9, 2015
Summary:
ping #795 (comment)
Closes #3087

Reviewed By: svcscm

Differential Revision: D2738947

Pulled By: mkonicek

fb-gh-sync-id: e29dfd933a8da42551576bdb1fb5c270039722ee
@maxkrieger
Copy link

I've been using replacePreviousAndPop({}) for more complex pagination scenarios and it has worked fine, albeit less cache-y than I'd hoped.

@katrinanova
Copy link
Contributor

@ericvicenti any idea why navigator.replace(newRoute) does't work in RN version 17 and 16? going back to version 14 now :(
replaceAtIndex method of NavigatorIOS seems to be identical.

@ericvicenti
Copy link
Contributor

It might be due to changes on the native side. I don't really work with NavigatorIOS, so I'm not sure

drtangible added a commit to bjornco/react-native that referenced this issue Jan 29, 2016
* Allows internal scenes to react to changes in their passed properties.

* By default, NavigatorIOS (and Navigator, to a lesser extent) doesn't play
  super nicely with flux's concept of unidirectional data flow. When a property
  in a route's `passProps` changes, the route's scene is not re-rendered.

* This patch allows route objects to optionally define their `passProps`
  property as a function that returns an hash of properties to use for
  rendering.

* When a route's `passProps` property is a function, it will be evaluated each
  time NavigatorIOS is rendered. If the properties have changed, the route's
  scene will be re-rendered with the new properties.

* Workaround for design decision described [here](facebook#795).

* Facebook is putting resources into figuring out a better process to accomplish single-directional
  data flow with navigation components & patterns. I think we'll eventually be able to take advantage
  of that work. GitHub repo for that project is [here](https://github.com/ericvicenti/navigation-rfc).
drtangible added a commit to bjornco/react-native that referenced this issue Feb 2, 2016
* Allows internal scenes to react to changes in their passed properties.

* By default, NavigatorIOS (and Navigator, to a lesser extent) doesn't play
  super nicely with flux's concept of unidirectional data flow. When a property
  in a route's `passProps` changes, the route's scene is not re-rendered.

* This patch allows route objects to optionally define their `passProps`
  property as a function that returns an hash of properties to use for
  rendering.

* When a route's `passProps` property is a function, it will be evaluated each
  time NavigatorIOS is rendered. If the properties have changed, the route's
  scene will be re-rendered with the new properties.

* Workaround for design decision described [here](facebook#795).

* Facebook is putting resources into figuring out a better process to accomplish single-directional
  data flow with navigation components & patterns. I think we'll eventually be able to take advantage
  of that work. GitHub repo for that project is [here](https://github.com/ericvicenti/navigation-rfc).
@hasen6
Copy link

hasen6 commented Feb 26, 2016

What happened with this? I'm also experiencing this error. Apparently we were going to see a response in 30 days but that was about 5 or 6 months ago.

@tkmoney
Copy link

tkmoney commented Mar 5, 2016

+1

1 similar comment
@sericaia
Copy link

👍

@ryanyu104
Copy link

+1

@psaitu
Copy link

psaitu commented Sep 18, 2016

+1

@aphillipo
Copy link

aphillipo commented Sep 19, 2016

Facebook don't support the native control (NavigatorIOS) - Navigator is their JS replacement and it works okay - try using that instead and it won't be full of bugs!

@Gagege
Copy link

Gagege commented Oct 6, 2016

I'm using version 0.34.0 and Navigator (not NavigatorIOS) and I'm having this exact problem. State changes in my parent screens get passed into child/sibling screens via passProps, but the child/sibling screens don't get re-rendered. I have to call this.forceUpdate() on my child/sibling screens after calling back to the parent screen. It works fine, but it's not very "Reacty".

@felipemullen
Copy link

I am also experiencing the issue described by @Gagege, which is very easy to reproduce. It is counter intuitive because with all react components, you expect that props passed to a child will automatically re-render the child when they are updated in the parent via setState().

The use of Navigator's passProps functionality breaks this pattern and makes it difficult to update any view that is a child of a navigator.

I am going to try using NavigationExperimental to see if it has the same issue

@lacker
Copy link
Contributor

lacker commented Oct 21, 2016

This issue seems like it is now a bunch of different issues combined. @felipemullen I suggest trying ExNavigation and complaining to @skevy if it is not good enough, rather than NavigationExperimental which we are figuring out how to combine with ExNavigation soon. I'm going to close this issue, and I suggest if people continue to have similar problems they open a new issue and be very clear about whether the problem applies to Navigator or NavigatorIOS because they are pretty different in terms of getting them fixed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests