cashapp / multiplatform-paging

A library that packages AndroidX Paging for Kotlin/Multiplatform.
Apache License 2.0
557 stars 26 forks source link

NSInternalInconsistencyException in iOS when multiple update callbacks are triggered with single update #187

Open mukesh-mt opened 10 months ago

mukesh-mt commented 10 months ago

Scenario: Let's say I've loaded 3 pages with page size 20: (0, 20), (20, 20), (40, 20) And now some data got updated in the database and let's say the user was on the second page then we will receive the following event in the update callback.

onRemoved: 40, 20 onChanged: 20, 20 onRemoved: 0, 20

The issue is when we receive the first event and call collectionView.deleteItems for (40, 20), the number of items will mismatch with the data source as here collection view would expect 60-20 = 40 rows but only 20 rows are available. Also, we cannot batch these operations as they are triggered individually.

Please help how I can batch these operations so that I can use something like:

collectionView.performBatchUpdates {
  collectionView.deleteItems(at: indexPath)
  collectionView.reloadItems(at: indexPath)
  collectionView.deleteItems(at: indexPath)
} 
veyndan commented 10 months ago

That's a great question. We'd need some sort of callback telling us that the batch of item updates has completed, and from what I can tell, it's not currently possible do that with the existing AndroidX Paging API.

@claraf3 I know you've been looking at potentially changing these APIs. I think this'd be a really important addition to the API, because, as @mukesh-mt mentions, certain item updates will cause the app to crash on iOS. What are your thoughts on this?

claraf3 commented 10 months ago

If those callbacks were from a single event (i.e. REFRESH), then our addOnPagesUpdatedListener might be what you're looking for. It is a callback that triggers after an insert/drop event.

Otherwise, we currently don't have the infrastructure to definitively map multiple load events to a single user input. This was something we considered adding in the future (i.e. batching LoadStateUpdates or data updates). That work is beyond the scope of the current API changes, but I think it makes sense to factor that into our current work.

veyndan commented 10 months ago

I think we'll need the latter, i.e., "have the infrastructure to definitively map multiple load events to a single user input." The performBatchUpdates call that @mukesh-mt mentioned would exist within the library in PagingCollectionViewController, and it should work for any type of batched update.