Revision of UIKit Overlay's API

[jenox]

I would like to revise Differ's public API in terms of its UIKit overlay. The changes I would like to make are the following:

1. Optional completion handlers for methods on UITableView

The UICollectionView already has this parameter and it seems reasonable to support this on UITableView as well. The intuitive approach to implement this would be using performBatchUpdates(_:completion:) instead of beginUpdates()/endUpdates(). This method, however, is only available on iOS 11+.

Alternatively, we could keep the old methods and add extra @available(iOS 11.0, *) methods with completion handlers.

Another idea would be to hook into CATransaction, wrapping beginUpdates()/endUpdates() in CATransaction.begin()/CATransaction.commit() and registering the completion handler using CATransaction.setCompletionBlock(_:). I highly doubt that this would have the same semantics as using performBatchUpdates(_:completion:)'s completion handler.

Personally, I would make this a major version bump (see other changes below), drop support for iOS 10 and earlier, and simply use the modern performBatchUpdates(_:completion:) API.

The update block passed to performBatchUpdates(_:completion:) is also optional, meaning it is implicitly escaping, requiring us to annotate things like indexPathTransform as @escaping as well. I don't think that's a huge deal, though. I'm not sure if the block even escapes; we could try using withoutActuallyEscaping(_:do) if there are strong reasons to not annotate the closures as @escaping, though I'd try to avoid that if possible in case the block actually does escape.

2. Mandatory updateData parameter for UITableView

Again, this is a feature we already have for UICollectionView, and we should port it to UITableView as well. Currently, it is very easy to use the Differ/UITableView/UICollectionView APIs incorrectly, as #62 shows. Adding this parameter alone is not going to solve these issues, but it at least tells users of the library at which point they are supposed to update the data source.

Unfortunately, the intuitive way of using Differ calls things like animateRowChanges(oldData:newData:) when the underlying data has already changed, e.g., from inside a didSet property observer, which is incorrect. This causes crashes when a UITableView/UICollectionView has not queried its data source or the cache has been invalidated prior to scheduling an animation. In these cases, the UITableView/UICollectionView query its data twice, before and after the changes specified by the diff, but ends up querying the new data twice, thus crashing with an NSInternalInconsistencyException. Problems like this may be anchored deeply within an app's architecture and it might not be feasible to call Differ before the data changes and update the data source only once Differ tells you to.

We could try to prevent the crashes by checking if a diff is compatible with the current state of a UITableView/UICollectionView before applying it, and calling reloadData() instead if it is not. I'm not sure how this interacts with nested calls to performBatchUpdates(_:completion:) and will need to investigate a bit more. If you have any other ideas to address this issue, please let me know.

Even with this safeguard in place, I believe we should make the parameter mandatory to force users to think about when they update the data source. This way, users would need to opt in to using the APIs incorrectly.

Personally, I have no experience with making breaking changes in a library. Do you think we should keep the old signatures around and annotate them as @available(*, deprecated, message: "") to make migration to 2.0 easier?

3. Optional animated: Bool parameter for UITableView and UICollectionView.

Sometimes, we may want to update the UITableView's data without animation, for example, if the UITableView is currently offscreen or if the user has the "Reduce Motion" accessibility feature enabled. In these cases, updating the relevant parts of the UITableView via performBatchUpdates(_:completion:) is much more efficient than simply calling reloadData(), which causes all cells to be reloaded. reloadData() also has the nasty side effect of cancelling touches if the user is currently interacting with the cells.

If animated is false, we could simply wrap the call to performBatchUpdates(_:completion:) in UIView.performWithoutAnimation(_:).

---

In summary, here are the changes I would like to make to the UIKit overlay. Analogous changes to the animateRowChanges, animateRowAndSectionChanges, animateItemChanges, and animateItemAndSectionChanges families of methods are intentionally left out here. Having the methods begin with "animated" and then having an animated: Bool parameter might be a bit strange — maybe we should rename the methods to "applyRowChanges" or "transitionRows" or something else?

extension UITableView {
    func apply(
        _ diff: ExtendedDiff,
        deletionAnimation: DiffRowAnimation = .automatic,
        insertionAnimation: DiffRowAnimation = .automatic,
-       indexPathTransform: (IndexPath) -> IndexPath = { $0 },
+       indexPathTransform: @escaping (IndexPath) -> IndexPath = { $0 },
+       updateData: () -> Void,
+       animated: Bool = true,
+       completion: ((_ finished: Bool) -> Void)? = nil,
    )

    func apply(
        _ diff: NestedExtendedDiff,
        rowDeletionAnimation: DiffRowAnimation = .automatic,
        rowInsertionAnimation: DiffRowAnimation = .automatic,
        sectionDeletionAnimation: DiffRowAnimation = .automatic,
        sectionInsertionAnimation: DiffRowAnimation = .automatic,
-       indexPathTransform: (IndexPath) -> IndexPath,
+       indexPathTransform: @escaping (IndexPath) -> IndexPath,
-       sectionTransform: (Int) -> Int,
+       sectionTransform: @escaping (Int) -> Int,
+       updateData: () -> Void,
+       animated: Bool = true,
+       completion: ((_ finished: Bool) -> Void)? = nil,
    )
}

extension UICollectionView {
    func apply(
        _ diff: ExtendedDiff,
        updateData: () -> Void,
+       animated: Bool = true,
        completion: ((Bool) -> Void)? = nil,
        indexPathTransform: @escaping (IndexPath) -> IndexPath = { $0 },
    )

    func apply(
        _ diff: NestedExtendedDiff,
        indexPathTransform: @escaping (IndexPath) -> IndexPath = { $0 },
        sectionTransform: @escaping (Int) -> Int = { $0 },
        updateData: () -> Void,
+       animated: Bool = true,
        completion: ((Bool) -> Void)? = nil,
    )
}