DeclarativeHub / ReactiveKit

A Swift Reactive Programming Kit
MIT License
1.24k stars 115 forks source link

Crash when updating a binded collection or tableview #157

Closed sgousseauofficiel closed 7 years ago

sgousseauofficiel commented 7 years ago

Hello Serdan, hope you well.

I come to you with a question concerning a problem while updating a list.

Here is the steps: 1 - Instanciate your list controller and fill the list with a mutable observable array 2 - Do not show your view controller yet, but insert or remove an item in your array, or replace all. 3 - Crash in Collection or TableView Bond at insert line, it says that items before the update should be equal to items after update (in short)

What works: 1 - Instanciate your list controller and fill the list with a mutable observable array 2 - Show your view controller, then remove it but keep it alive 3 - Inserts / Deletion and everything works

Do you know what i'm doing wrong in the first case ?

sgousseauofficiel commented 7 years ago

Hello Serdan,

Here is a demo project containing 2 view controllers, DelegateViewController and ReactiveViewController.

Both shows items from array of a Manager class. I have written at the end of Manager class 2 functions, safeClear and unsafeClear. The first one will clear list by sending an empty array and the other one will clear the array by removing all items.

The unsafeClear function makes the app crash while using ReactiveKit, but not while using standard iOS delegation implementation.

The safeClear method is ok for both.

You can change from "DelegateViewController" to "ReactiveViewController" in the ContainerViewController class.

I believe that it's not a wanted behavior since its a different behavior from the standard framework. I did not go deeper, searching for inserts and removing problems reproduction and comparison because I'm sure it is the same source.

Hope it helps.

Here is the project: ListTest.zip

srdanrasic commented 7 years ago

Hi @sgousseauofficiel!

Sorry for the slow reply, but I finally got some time to investigate this.

The problem can occur when you make multiple changes to the observable array in one view update cycle. You are doing this:

self.clearData() // i.e. self.array.removeAll()
self.array.replace(with: arr)

Which in essence tries to update the collection view before the first update is over (collection view needs one view cycle to process the updates).

A way to fix it is to dispatch second call later.

self.clearData()
DispatchQueue.main.async {
    self.array.replace(with: arr)
}

Actually, in this example, first call is redundant because replace(with:) replaces whole array so there is no need to remove anything first, so you could just do:

self.array.replace(with: arr)

That being said, I'd still like to make sure this never happens. I'll see if I can make some improvements to Bond so that we don't have to worry about these things.

Thanks for the detailed issue report!

srdanrasic commented 7 years ago

This ticket should be in ReactiveKit/Bond repo so I'll close it here.