Skip to content

re-render when user react-native NavigationExperimental/NavigationLegacyNavigator.js #365

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

Closed
chenxiaohu opened this issue Apr 22, 2016 · 8 comments

Comments

@chenxiaohu
Copy link

When use NavigationExperimental/NavigationLegacyNavigator.js as navigator, all container components connected to redux store rendered by any action.
Old navigator has not this problem.

@gaearon
Copy link
Contributor

gaearon commented Apr 22, 2016

Can you provide an example that reproduces this?

@chenxiaohu
Copy link
Author

src.zip

this example toggle navigator can reproduces.

@gaearon
Copy link
Contributor

gaearon commented Apr 22, 2016

I don’t know React Native well. How do I launch this repo?

@chenxiaohu
Copy link
Author

Do you ever try any React Native project, even Get started project?

@gaearon
Copy link
Contributor

gaearon commented Apr 22, 2016

Yeah. But usually they come with Xcode projects, don’t they?

@gaearon
Copy link
Contributor

gaearon commented Apr 22, 2016

Okay, I got it to run. Can you please clarify which exactly actions you do not expect to cause components to re-render?

@gaearon
Copy link
Contributor

gaearon commented Apr 22, 2016

If I change renderList(list) in Comment.js to dispatch an action on click, I don’t get any log messages about re-renders:

  renderList(list) {
    let result = [];
    list.map(i => {
      result.push(<TouchableOpacity key={i.id} style={styles.row} onPress={() => {
        this.props.dispatch({
          type: 'test'
        })
      }}><Text>{i.title}</Text></TouchableOpacity>);
    });
    return result;
  }

Therefore, “any action” does not cause “all container components” to re-render, so it looks like there is no bug here.

When I run the app for the first time, I see this:

@Render Root.....
Post.js:58 @@Render PostList
App.js:34 REQUEST_POST_LIST
Post.js:58 @@Render PostList
App.js:34 RECEIVE_POST_LIST
Post.js:58 @@Render PostList

It looks correct to me: we first render the initial state, which is an empty screen, then we REQUEST_POST_LIST causes a re-render because there is a loading indicator now, and then RECEIVE_POST_LIST causes a re-render because the list is ready.

If I open a post’s comments, I see this:

@@Render CommentList for post: 4
App.js:34 REQUEST_COMMENT_LIST
Post.js:58 @@Render PostList
App.js:34 RECEIVE_COMMENT_LIST
Comment.js:63 @@Render CommentList for post: 4

The initial @@Render CommentList for post: 4 is caused by the empty list appearing. It looks fine to me. The second @@Render CommentList also makes sense to me here: the comments are loaded so it’s time to render the actual list.

The one that doesn’t quite make sense to me is REQUEST_COMMENT_LIST causing `@@Render PostList. Requesting comments shouldn’t re-render the posts.

I dug into why this happens, and it seems like returning a cached element in render() is not enough to prevent a re-render in some cases when there is an additional context between <Provider> and the connect()ed component because of this check in ReactReconciler.

I don’t fully understand in which cases this happens yet so I’ll file a separate issue for the future.
For now, you have two options to avoid this:

Option 1

Remove ownProps from your mapStateToProps function in Post.js.
This will turn on the optimizations earlier, and that code path will be avoided altogether.

function mapStateToProps(state) { // , ownProps
  let {list, isFetching} = state.postList || {isFetching:true};
  return {
    isFetching,
    list
  };
}

Option 2

Add a custom shouldComponentUpdate() function to Post.js because React Redux could not optimize it in this particular case, and you need to do it manually.

import shallowCompare from 'react-addons-shallow-compare'

class Post extends Component {
  shouldComponentUpdate(nextProps, nextState) {
    return shallowCompare(this, nextProps, nextState)
  }

I hope this helps!

@chenxiaohu
Copy link
Author

Great! your answer is prefect for me. thanks:)

foiseworth pushed a commit to foiseworth/react-redux that referenced this issue Jul 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants