WIP: Auto diffing section controller

[rnystrom]

⚠️⚠️⚠️ Do Not Merge ⚠️⚠️⚠️

Started work on the plane to get this moving since #418 is up and ready to land. We'll likely need to spend some time fleshing out the API of this, and I think I'll split it up into a couple different PRs once ready for review. Putting this up now to get early feedback.

What is this?

This adds an auto-diffing section controller as outlined in #38. There are several key parts:

• Subclass a new section controller IGListAutoSectionController (naming wip)
• Connect a data source
• Implement the data source methods that do 3 things:
  • Given a top-level object, transform it into an array of diffable view models
  • Given a view model, return a cell
  • Given a view model, return a size for a cell
• A new protocol for the cell IGListBindable so that we can control when the cell is updated w/ the view model.
  • The most important part of this is that it unlocks moving and reloading a cell, which you can't do w/ UICollectionView

TODO

• Unit test reloadObjects:
• Add didSelectItemAtIndex: support
• Verify all headers use FB copyright
• Create PR for IGListAdapterUpdater changes
  • Include fix + tests for Bug when a section reload is queued while batch updating #288
• Create PR for new section controller w/ unit tests
• Create PR for example
• Add header docs

IGListAdapter

I had to make some changes to IGListAdapterUpdater in order for this to work. The biggest challenge is when the section controller receives a new object, it has to split it up into view models and then apply changes.

However, didUpdateToObject: will be called inside performBatchUpdates:, so we need to diff and fire off the changes on the collection view immediately. A few issues:

• You couldn't use -[IGListCollectionContext performBatchAnimated:updates:completion:] inside didUpdateToObject: and have it update immediately (async updates)
• There are other ways that didUpdateToObject: gets called, so we can't just insert/delete/move
• We're planning to remove non-batched updates entirely in RFC: Remove item-level updates from IGListCollectionContext, require them done in batch updates #392

Diffing top-level models

One super important piece to getting this right is having top-level (the ones given to IGListAdapter and handed to the SC in didUpdateToObject: always equal itself, even if its values change. I solved this in the unit tests w/ an object that implements diffing like:

@interface IGTestObject: NSObject <IGListDiffable>
@property NSString *key;
@end

@implementation IGTestObject
- (id)diffIdentifier {
  return self.key;
}

- (BOOL)isEqualToDiffableObject:(id)object {
  return [self.key isEqual:[object key]];
}
@end

If you use a traditional isEqualToDiffableObject: (where you check all props/values) you'll end up reloading the section, avoiding all the nice cell animations.

I started making a stock object that does this, something like IGListAutoObject, but its so simple that it's kind of annoying. It's essentially a model box that will probably end up more confusing than having to learn how to diff w/ the auto SC.

Would love thoughts here.

Selection

didSelectItemAtIndex: is totally broken. Options are:

• Add a new method on the data source protocol
  • Gross b/c that's another empty method for a lot of folks
• Add another delegate protocol you can optionally wire up

We've discussed splitting off didSelectItemAtIndex: into a selectionDelegate (like the display/scroll/etc delegate) in #184 and elsewhere. That might help, though the view models array is private, so we'd have to expose that or create another delegate API.

How can I help?

Since this is super early, please hold on nit picks, formatting, etc. I'll work on that as we go along. But there are some big things I'd love your thoughts on:

• The name IGListAutoSectionController. Anything better out there? I thought IGListDiffableSectionController but idk
• I'm using a data source to split the object into view models, return cells, and handle sizes. I started off passing the SC blocks on init that did the transform (sort of like IGListSingleSectionController). I thought the API was kind of gross though, and you have to subclass the SC anyways.
• IGListBindable is great, but you can't just use the view model to config the cell. For instance, how do you make the SC the cell delegate? That's why I'm leaving the method to still return a cell, but we will call bind before returning it. Is this ok?
• Sample project now or later?
• Let me know if you see some glaring Swift interop issues. I haven't tried it yet.
• Throw all the crazy test-case scenarios you got at me!

[zhubofei]

@rnystrom How about IGListReactSectionController 🤔? The one-way data flow behavior is similar to react.js

[Sherlouk]

Sounds amazing!

I'd say we should definitely try and create some examples with this, an easy way to show usage and will be easier to spot most/some Swift interop issues!

[rnystrom]

@zhubofei

How about IGListReactSectionController 🤔 The one-way data flow behavior is similar to react.js

That's so true! I worry about stepping on their toes though w/ naming. I should probably leave all the "react" stuff in their court. But I like where you're going w/ this.

@Sherlouk agreed! I'll probably include all of the changes in this PR so we can get a feel for things, but then split it up into 3 PRs (updater changes, new SC, example).

Btw forgot to mention that the updater changes should also fix #288.

[zhubofei]

@rnystrom I prefer IGListDiffableSectionController to IGListAutoSectionController. IMHO, Auto may be a little bit vague compared to what we already have i.e. Stack, Single. Or, can we use sth like IGListTemplateSectionController? b\c we have viewModel built into the SC.

[Adlai-Holler]

This is extremely exciting @rnystrom!

[Adlai-Holler]

Should we make this publicly readable? Or add a viewModelAtIndex: method to access this internal store? As a client it would be good to know what the state-of-the-world for this section controller is.

[rnystrom]

Yes! Absolutely we should. I can imagine plenty of cases where someone would need to grab the view model.

[Adlai-Holler]

Should these pass sender i.e. - (CGSize)autoSectionController:(IGListAutoSectionController *)sectionController sizeForViewModel:(id)viewModel? It's more formally correct but uglier.

[rnystrom]

Hrm, good point. I guess to be proper they probably should.

[Adlai-Holler]

Should we support formally reloading the item for an updated view-model, rather than only binding a new view model to the existing cell?

One idea is, we add a data source method like:
- (BOOL)shouldReloadCellForUpdatedViewModel:(id)viewModel atIndex:(NSInteger)index

And call it for each updated view model to decide whether to reload or just -bindToViewModel:.

[rnystrom]

What would the purpose be for actual reloading? Bear in mind that we don't actually call reloadItemsAtIndexPaths: b/c of UICollectionView issues, we convert it to a delete+insert under the hood (that's the whole thing about moving alongside reload/bind here).

[Adlai-Holler]

If the model changes in such a way that we need a new cell class to represent it, a reload (or delete/insert) is the only way to achieve the update. It's not a huge requirement, but I think it's worth considering at this early stage since it would affect the API bigly.

[rnystrom]

@Adlai-Holler interesting point. I could see that.

I'm leaning towards not supporting this yet and filling it in later if someone really-really needs it (who knows, we might run into it). I'd love to see some concrete use-cases before we design the API too.

Plus it shouldn't be breaking when we add it, only an enhancement.

bigly

🙊 😅

[rnystrom]

Some other naming ideas:

• IGListBindingSectionController (bindable, binder)
• IGListViewModelSectionController

[facebook-github-bot]

@rnystrom updated the pull request - view changes

[amonshiz]

IGListDiffingSectionController

• This new object is taking some actions on behalf of the developer and so the name should be active.
• The action that the object is taking is that of diffing up some view models.
• It is a section controller.

[facebook-github-bot]

@rnystrom updated the pull request - view changes

[rnystrom]

History got messed up, recreated and moving to #494