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

Update the reference to getStableId in VirtualRenderer when the DataProvider is changed #339

Closed

Conversation

DevopsElectricJukebox
Copy link

This is a bugfix for the case when the dataProvider is changed, and may provide entirely different data. The VirtualRenderer needs to update its own reference to getStableId to ensure that it uses the correct IDs for the components it creates.

dhmw and others added 2 commits April 30, 2019 10:17
Update the reference to getStableId in VirtualRenderer when the DataProvider is changed

// Future calls to get stable IDs must use the function
// provided by the new data provider.
this._fetchStableId = getStableId;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_fetchStableId always refers the the latest dataProvider. Check here: https://github.com/Flipkart/recyclerlistview/blob/master/src/core/RecyclerListView.tsx#L157 . What issue were you noticing?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in the function used to sync the virtual renderer with a new data provider. The virtual renderer keeps a reference to a getStableId function which must be updated when the data provider changes. If this is not done, then it will fail to correctly associate indexes with stableIds when the new data provider returns different stableIds for each index - e.g. if an item is inserted at the start of the data array.

I did not find any other place in the code where _fetchStableId could be updated when the data provider is changed, and this seems like the best place to do it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

You can test this bug by the following steps:

  1. Instantiate a RecyclerListView with a DataProvider containing 1 item.
  2. Change the DataProvider to one containing 3 items
  3. The RecyclerListView will crash due to trying to look up the 2 new items using the old DataProvider's getStableId function.

Thanks.

dhmw and others added 3 commits May 9, 2019 12:26
…class

adds new method clone() which subclasses must re-implement
…aHandling flag when cloned DataProvider was empty
@DevopsElectricJukebox
Copy link
Author

Hi,

I will close this PR since this is actually not a complete fix. The addition of _fetchStableId is in the wrong place, and needs to be called under different conditions. I will consolidate changes and open a new PR for you to review.

Thanks.

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

Successfully merging this pull request may close these issues.

3 participants