Add optional batchUpdatesDidBegin block to UIKit extensions

[ahtierney]

This PR adds a hook to the UIKit extensions on batch updates did begin. This permits users to update their data model inside the performBatchUpdates block – a measure which can prevent crashes with dynamic and/or inconsistent collection layouts.

[osteslag]

Thanks for submitting this PR, @ahtierney. I have not experienced this error myself, and before adding more cruft to the method, I’m trying to understand…

1. In what situations could the layout be out of date?
2. Is it possible to detect this problem and thereby update the layout prior to calling this method?

If it can be detected, I’d rather add a comment about making sure the layout is up to date.

[ahtierney]

Thanks, @osteslag. I've attached a rather contrived example. Two cases I know of are:

• after reloadData has been previously called but the layout pass hasn't finished.
• when calling performBatchUpdates before the entire view hierarchy has finished laying out.
  I think in most cases it can be managed, though the later of the two has given me trouble when the view hierarchy is not wholly owned – ie. with a CollectionView nested inside a table.
  I appreciate your desire to keep the API clean – maybe a comment and exposing batchIndexPaths would make it easier for folks to opt into doing the updates manually if they find they need it?

https://github.com/PSPDFKit-labs/radar.apple.com/tree/master/28167779%20-%20CollectionViewBatchingIssue

[osteslag]

Thanks for info and pointer to radar, @ahtierney. I’ve been reading up on it and feel it’s the wrong approach to introduce this closure as a workaround for what is most likely a UICollectionView bug.

If your code provokes this inconsistent state, I recommend you call layoutIfNeeded() before updating your model and and then call update(with:in:completion:). This way we contain the bug and workaround in UIKit without involving Changeset.

Makes sense? Am I overlooking something?

[ahtierney]

For my particular case that early pass triggered by layoutIfNeeded results in an unwanted layout aberration, however I will say I'm not convinced that I couldn't achieve the right result without updating the datasource in performBatchUpdates. For the time being updating in performBatchUpdates seems like the right path – it is one of the two recommended solutions in the documentation – but I certainly respect that it may be too much of an edge case to pollute the helper api. Thanks for considering it (and for the great library!), @osteslag

[osteslag]

@ahtierney, I had missed that Apple itself recommends this approach, which changes my view on it. It’s less of a hack than I first thought.

However, I still think it’s an edge case, and with update(with:in:completion:) being a very simple wrapper around performUpdates(_:completion:), for now I prefer users write their own code outside of Changeset to update the view in conjunction with model changes for their particular needs.

Thanks for your valuable input.