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

Confusion about rowHasChanged and ListView rendering #5754

Closed
janmonschke opened this issue Feb 4, 2016 · 3 comments
Closed

Confusion about rowHasChanged and ListView rendering #5754

janmonschke opened this issue Feb 4, 2016 · 3 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@janmonschke
Copy link
Contributor

Hey all,

While working on some performance improvements I found out that rowHasChanged was not called in any of our lists. These lists are infinite-scrolling lists, meaning that when you reach the end, it will fetch more items to display and then dump them into the list.

It seems like all entries in the list are updated whenever we change the DataSource's content. We do have a shouldComponentUpdate in place for all rows, but from how I understood rowHasChanged, these rows would not needed to be rendered again.

I created a simple example that shows how we are appending items to the list: https://gist.github.com/janmonschke/c9c84b6050683da0c64f

What we basically do is:

constructor() {
  // (...)
  this.ds = new ListView.DataSource({rowHasChanged: (r1, r2) => {
    console.log('rowHasChanged');
    return r1 !== r2;
  }});
  // (...)
}

onEndReached = () => {
  // append data to the end
  setData = [...setData, ...genRandomSet()];
  this.setState({
    dataSource: this.ds.cloneWithRows(setData)
  });
  console.log('onEndReached');
};

render() {
  return (
    <ListView
      dataSource={this.state.dataSource}
      renderRow={this.renderRow}
      onEndReached={this.onEndReached}
    />
  );
}

But rowHasChanged is never logged.

Am I using it somehow in a wrong way? Are there better ways to append to a ListView?

Same behaviour in 0.18.0 and 0.19.0

@facebook-github-bot
Copy link
Contributor

Hey janmonschke, thanks for reporting this issue!

React Native, as you've probably heard, is getting really popular and truth is we're getting a bit overwhelmed by the activity surrounding it. There are just too many issues for us to manage properly.

  • If you don't know how to do something or something is not working as you expect but not sure it's a bug, please ask on StackOverflow with the tag react-native or for more real time interactions, ask on Discord in the #react-native channel.
  • If this is a feature request or a bug that you would like to be fixed, please report it on Product Pains. It has a ranking feature that lets us focus on the most important issues the community is experiencing.
  • We welcome clear issues and PRs that are ready for in-depth discussion. Please provide screenshots where appropriate and always mention the version of React Native you're using. Thank you for your contributions!

@janmonschke
Copy link
Contributor Author

Okay, found the solution in: http://stackoverflow.com/a/33871118.

Apparently it's important to clone from the correct DataSource instance ;)

So the code from the gist should have been:

  setData = [...setData, ...genRandomSet()];
  this.setState({
    dataSource: this.state.dataSource.cloneWithRows(setData)
  });

This seems like an error that's easily sneaking into your codebase. I wonder if the correct usage of DataSource should be more emphasised in the documentation.

@ekryski
Copy link

ekryski commented Jun 23, 2016

Thanks @janmonschke! I agree, I had a bunch of views constantly re-rendering and I couldn't figure out why that was happening and why the rowHasChanged function was never getting called. 👊

@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 20, 2018
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

4 participants