mcudich / HeckelDiff

A fast Swift diffing library.
MIT License
167 stars 28 forks source link

Failure in UITableView/UICollectionView applyDiff when data has a row that is deleted and a row that is updated #9

Open SuperTango opened 7 years ago

SuperTango commented 7 years ago

If a diff contains a row that is to be deleted, and a row that is to be updated that has the same indexPath as the deleted row (before it was deleted), there is a crash.

For example:

struct SessionCacheDatum: Equatable, Hashable {
    let sessionId: String
    let sessionDetail: String

    static func ==(lhs: SessionCacheDatum, rhs: SessionCacheDatum) -> Bool {
        return lhs.sessionId == rhs.sessionId && lhs.sessionDetail == rhs.sessionDetail
    }

    var hashValue: Int {
        return sessionId.hashValue
    }
}

        a1.append (SessionCacheDatum(sessionId: "session1", sessionDetail: "detail1"))
        a1.append (SessionCacheDatum(sessionId: "session2", sessionDetail: "detail2"))
        a2.append (SessionCacheDatum(sessionId: "session2", sessionDetail: "detail3"))

If we do a

    self.tableView.applyDiff(a1, a2, inSection: 0, withAnimation: UITableViewRowAnimation.fade)

it will crash with an internal inconsistency exception.

a full repo showing the issue can be found here: https://github.com/playlist/HeckelDiffUpdateFail

The cause is that the row updates are processed in relation to the indexes BEFORE the update. The diff() returns the indexes after.

from https://developer.apple.com/library/content/documentation/UserExperience/Conceptual/TableView_iPhone/ManageInsertDeleteRow/ManageInsertDeleteRow.html:

Ordering of Operations and Index Paths ... It (the UITableView) defers any insertions of rows or sections until after it has handled the deletions of rows or sections. The table view behaves the same way with reloading methods called inside an update block—the reload takes place with respect to the indexes of rows and sections before the animation block is executed. This behavior happens regardless of the ordering of the insertion, deletion, and reloading method calls.

I put together a sledge-hammer type fix which is somewhat inelegant in my mind. Basically, in the UITableView.applyDiff and UICollectionView.applyDiff I made two update blocks. The first one does the inserts, deletes, and moves. The second one does the updates.

I think a better fix would be to leave the single update block, but that would require the Diff API to change to return both the "before" and "after" indices of the update (or maybe create an internal/private API that the UITableView/UICollectionView extensions use. But this works and I didn't want to try to change the API.

ghost commented 6 years ago

Good spot. A similar project called 'Dwifft' seems to have addressed this issue.

https://github.com/jflinter/Dwifft/blob/b123166a32c4985749c5363c8b322e255a04e271/Dwifft/Dwifft.swift#L175