Summary:
Using blocks can be nice, but here it causes a couple problems:
* The order of execution is a bit hard to follow. For example, in `-begin`, the very first thing we see defined is `executeCompletionBlocks` which is the last thing actually called. `IGListReloadTransaction` isn't too complex, but it gets even harder to follow in `IGListBatchUpdateTransaction`.
* It makes crash and assert reports hard to understand, because the stack includes mostly block names.
So lets turn blocks into methods.
Reviewed By: patters
Differential Revision: D23151918
fbshipit-source-id: 3ac4c77e9978c2700fd6744f084f624d27f0d300
Summary:
Lets move things to the right transaction:
* `performBatchUpdate` path to `IGListBatchUpdateTransaction`
* `reloadData` path to `IGListReloadTransaction`
Reviewed By: patters
Differential Revision: D23145770
fbshipit-source-id: e80fc05d2783e165354a147453083b449c92a61c
Summary: Now, lets move pending changes to a separate object `IGListUpdateTransactionBuilder`, which can we discarded once the update actually starts.
Reviewed By: patters
Differential Revision: D23145774
fbshipit-source-id: e12de178de33497f476972a9ad89ebb4e8d413ab
Summary:
Before adding `transactions`, lets clarify what information is collected before vs during the update. We can split apart `IGListBatchUpdates`.
* Before the update
* Collect the update blocks in `itemUpdateBlocks` which will be executed later during the update.
* Collect completion blocks in the same `completionBlocks` list as the other updates.
* During the update
* Collect item updates in `IGListItemUpdatesCollector`, like inserts and deletes.
* Collect new completion blocks (in case a section-controller schedules an update in -didUpdateToObject) in a separete list `inUpdateCompletionBlocks`
Reviewed By: patters
Differential Revision: D23145778
fbshipit-source-id: cae2c0689ac1ef0d93e3c04856b0495856e305b6
Summary: Lets test that the new updater (`IGListExperimentalAdapterUpdater`) can handle a missing section-controller. The old adapater `IGListAdapterUpdater` would crash.
Reviewed By: patters
Differential Revision: D23145773
fbshipit-source-id: 4589dcb11ddef5c7400947aea4b6827f1b2caaa8
Summary:
Lets implement `performExperimentalUpdateAnimated`. There's a couple of things happening:
* We're now using the `IGListTransitionData` container object
* We don't need `pendingTransitionToObjects` anymore because we query the `fromObjects` just before performing the diffing.
* We should update the unit tests to use `performExperimentalUpdateAnimated` since the regular `performUpdateAnimated` will now fail.
Reviewed By: patters
Differential Revision: D23145781
fbshipit-source-id: 0b896e918241e709c5eb23f56a6b7323521a2222
Summary:
There's a few issues with `IGListAdapterUpdater` which would be difficult to safely fix "in place", so lets create a duplicate updater that we can test separatly.
Ugh, copy paste?
* The alternative would be to change `IGListAdapterUpdater` directly and have lots of `if/else` branching. I've tried it and it's hard to follow and easy to break. By creating a separate class, we do run the risk of having 2 diverging updaters, but I think it's worth the risk since it rarely gets changed. I'll try to ship (or burn) this new updater quickly.
Why duplicate `IGListAdapterUpdater` rather then starting from scratch?
* I want to take advantage of the existing unit tests. I can make small incremental changes and make sure the tests still pass.
* There's a lot going on in `IGListAdapterUpdater` and it's easier to refactor it by moving things around, rather then writing it from nothing.
Why does `IGListAdapterUpdater` need a clean up?
* Check out the first diff in the stack or task T74605897.
Reviewed By: patters
Differential Revision: D23145777
fbshipit-source-id: 360e980a89a5681ce38d1b5f6f1ca035eb1eb195
Summary:
For diffing, `IGListAdapterUpdater` uses the full `fromObjects` instead of `validObjects`, which can be different if we're missing `IGListSectionController`. The diffing results won't match what the `IGListAdapter` tells the `UICollectionView`, so we can crash.
To fix this, lets try to generate the `IGListSectionController` before performing the diffing, and keep them in a container object (`IGListTransitionData`) alonside the `fromObjects` and `toObjects`. This might also unblock other cool features in the future, like giving `IGListSectionControllers` time for background work, but that's for another day.
Since this change is relatively large, lets create a temporary protocol `IGListUpdatingDelegateExperimental` which will have a slightly different API from `IGListUpdatingDelegate`. When/if the new implementation ships, we can merge both protocols.
Why not edit `IGListAdapterUpdater` directly?
* I don't want to mess up exiting updaters just yet.
Reviewed By: patters
Differential Revision: D23145776
fbshipit-source-id: 21642cafa64e2a26a173e2ba683ef5c6cede17d7
Summary: Create temporary protocol for `IGListAdapterUpdater` to test a different implementation. The idea is to ship (or burn) the new implementation before the next 5.0.0 version release.
Reviewed By: patters
Differential Revision: D23145772
fbshipit-source-id: ede312c4a1289a34f73c14415a9bca40033fe925
Summary: We pass `experiments` to places that don't need it, so lets clean that up.
Reviewed By: patters
Differential Revision: D23145782
fbshipit-source-id: affcfe0f38d1b8e544940af89e5692dbdc36dd76
Summary:
On `IGListAdapater` data update:
1) Pass the capacity count, so that arrays don't have to re-size.
2) Avoid using a set, so that we don't need to deal with hashes and equality. The updater should have dealt with duplicates already.
Reviewed By: patters
Differential Revision: D23145771
fbshipit-source-id: 2ed93231e15ddcd66cfe4d1f7384c563c77caa8e
Summary:
Starting this stack by bumping the next release version, because we're about to make breaking changes (plus there's already breaking changes since the last release)
# Project overview
## Stability improvements
* `[IGListAdapter setDataSource]` isn't safe
* We're changing the underlying data without telling the `UICollectionView`.
* Fix: Lets invalidate the `UICollectionView` data by changing its dataSource. More details in the diffs.
* `[IGListAdapter setCollectionView]` isn't safe
* This is synchronous, but we might have pending or on-going updates. The `UICollectionView` might get synced before the pending update actually start executing, so the diff results will be off.
* Fix: Lets wrap updates in a transaction that can be cancelled.
* Returning a nil `IGListSectionController` from `IGListAdapterDataSource` could crash
* The `IGListAdapterUpdater` will still perform the diffing assuming that all the objects will have a section, which isn't the case.
* Fix: Lets generate the `IGListSectionController` before the diffing.
* Multiple item update blocks can conflict and crash
* Because section-controllers can directly insert/delete/move `NSIndexPath`, we can run into conflicts when merging multiple batch updates together.
* Fix: Working on automatic conflict resolution.
* Item reloads aren't safe
* Lets say you have a section with items ["a", "b"] and need to update to items ["b-reloaded"]. You reload "b" (index 1) and delete "a" (index 0) in the same batch update block (or in separate blocks within the same overall update). This gets converted into 3 changes: delete 0, insert 1, delete 1 ... but we only have 1 item, so index 1 doesn't make sense anymore! This is because we convert reloads to insert/deletes without taking into account that inserts are relative the list after the update.
* Fix: Same as above, we need to convert the index to match the list after the update.
* Other
* Lets ask for the `fromObject` just before diffing, instead of asking when scheduling the update.
* If the `UICollectionView` section count doesn't match `fromObject`, lets fallback to a reload.
## Performance improvements
* Re-test background diffing
* `IGListExperimentBackgroundDiffing` and coalescing updates wasn't safe because of the issues mentioned above. The longer we wait, the more likely we'll end up in a race condition. Lets try re-testing with the stability improvements.
* Less array resizing and set hashing
* We can skip some hashing by using an `NSArray` instead of a `NSSet` in `updateObjects`. The adapter or updater should have dealt with uniqueness already.
* Avoid calling `[UICollectionView performBatchUpdate ...]`
* If there's no update, lets try skipping `performBatchUpdate`.
* Background work
* This is a larger project that will need to be borken out, but these improvements are required to make background work safe.
* Avoid double cell creation
* Still investigating why this is happening
## Other
* Transactions
* `IGListAdapterUpdater` is the workhorse of `IGListKit` and has become a bit hard to follow over the years. We want to break it apart into simpler, more manageable parts.
* Avoid blocks
* There's a lot of blocks flying around, making crash logs hard to read. Lets try to use methods/functions where possible.
Reviewed By: patters
Differential Revision: D23429237
fbshipit-source-id: ac200f139df05fefea2a83e1b942d58638ff9e15
Summary:
## Changes in this pull request
Update link of Ryan Nystrom's talk at try! Swift NYC.
### Checklist
- [x] All tests pass. Demo project builds and runs.
- [x] I added tests, an experiment, or detailed why my change isn't tested.
- [x] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
- [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md)
Pull Request resolved: https://github.com/Instagram/IGListKit/pull/1451
Reviewed By: candance
Differential Revision: D22950479
Pulled By: bdotdub
fbshipit-source-id: a6dcf76584122267ab2497bdbd226e66fb24d7f2
Summary:
Issue: Invalidating the layout of a `IGListSectionController` that's been removed from the map crashes, because we're passing `NSNotFound` to `[UICollectionView numberOfItemsInSection:...]`. This can easily happen if the section-controller invalidates its layout in a `performBatchAnimated` completion block, but the associated object was deleted during the update or after a full reload.
Fix: Like the rest of `IGListAdapter`, lets check for a valid `NSInteger` and bail early if we don't have it.
Reviewed By: mariajayne
Differential Revision: D22588010
fbshipit-source-id: bf0c26f313795bb10682ef6b660d6ea8a7ce9f1e
Summary: Wrap `[UICollectionView performBatchUpdates ...]` so that in case it crashes, the first app symbol will not be a block. A block name includes the line number, which means if you change the block line number, it will be categorized as a different crash. This makes tracking crashes across multiple app-versions a pain.
Differential Revision: D22398750
fbshipit-source-id: 90643e7eafe76b07e7022111f183e4734fd8a347
Summary: To prepare testing `Foundation` vs `IGListKit` diffing, lets measure how long it takes.
Reviewed By: Haud
Differential Revision: D22295546
fbshipit-source-id: 8023717f66ea68cbc24981272e8c134dd893274a
Summary: This header refers to `UICollectionViewCell` but does not import UIKit.
Reviewed By: candance
Differential Revision: D22340609
fbshipit-source-id: 8d03a48d363ecf3b00b29f89828d83ab44615fd4
Summary: The duration of an update depends a lot of if it's animated, so lets pass that info to the delegate.
Reviewed By: lorixx
Differential Revision: D22265405
fbshipit-source-id: 4d164447384b84eb6f41c7f8365d0b28670e1372
Summary: We call `[collectionView layoutIfNeeded]` before performing updates, but it's not clear why. It leads to some strange animation issues when performing updates while scrolling. So lets try to skip it?
Reviewed By: Haud
Differential Revision: D22244053
fbshipit-source-id: 0c01a73860ebc16f430fed04d5c29a5bde038286
Summary: This experiment shipped so we can remove it. We still need a way to disable it with `allowsReloadingOnTooManyUpdates` in case we don't want to resign the first responder on large updates.
Reviewed By: Haud
Differential Revision: D22219238
fbshipit-source-id: 98e78f3ed040809db6c4b4c8da8fd0e976aca0a2
Summary: If we fallback to `reloadData`, we don't queue the next update. That means if you call `performUpdate` multiple times quickly, there's a chance that the last update doesn't take effect.
Reviewed By: Haud
Differential Revision: D22219237
fbshipit-source-id: 167e3428859a0194f8f8c57624504edd531480df
Summary:
Issue: There's a couple of situations where `IGListAdapterUpdater` updates the `collectionView` without notifying the `IGListAdapterUpdater`.
* If we call `reloadDataFallback()` because of a missing window, there's no delegate calls.
* If we call `reloadDataFallback()` because of too many diff updates, we call `willPerformBatchUpdatesWithCollectionView` but not `didPerformBatchUpdates`.
Fix: Lets clean up the delegate calls
* If we `[UICollectioView performBatchUpdates:]` lets call `willPerformBatchUpdatesWithCollectionView` & `didPerformBatchUpdates`
* If we fallback to `[UICollectionView reloadData]` lets call `willReloadDataWithCollectionView` & `didReloadDataWithCollectionView`
* If we fallback to doing nothing, lets call `didFinishWithoutUpdates`
Reviewed By: Haud
Differential Revision: D22219236
fbshipit-source-id: 1afb4df8dbbaf4725424027bb52c1a5cc5100b30
Summary: Actually this won't work and causes even more crashes. For example, if we reload cell 0 and insert at 0, we end up with just a single delete & insert at 0, which causes an inconsistency exception. `IGListApplyUpdatesToCollectionView` already takes care of avoiding multiple reloads of the same `NSIndexPath` by using a `NSMutableSet` when collecting the `reloadDeletePaths`.
Reviewed By: apadalko
Differential Revision: D21822502
fbshipit-source-id: c2382951ffe013f82c1de2492286d2911f40444d
Summary: Ship this experiment. `IGListCollectionViewLayout` should get the section/index counts via `UICollectionView` to stay in sync, instead of the `dataSource`.
Reviewed By: apadalko
Differential Revision: D21789871
fbshipit-source-id: 9fe069bd793b36680cd8562e69c7367d69a11ec7
Summary:
= Context =
Thanks to the awesome Comparison View analysis, I am able to pinpoint where the scroll perf regression is for the single section inline update QE.
CV2 link: https://fburl.com/cv/fttkv99b
Also see from the graph that the these call stacks are pretty stat-sig,
https://pxl.cl/14D9M
And from the following stack stats, it's pretty clear that the heaviest one is calling from `-[IGListBindingSingleSectionController didUpdateToObject:]`, Approx. Frame Percentage is 24% to 1% compared to Control test group for my QE.
https://pxl.cl/14Db0
= Root cause =
Basically in my new QE, whenever IGListKit update, the -didUpdateToObject: is called on every single IGListSectionController, which in our case, we would call -[-configureCell:withViewModel:] all every single cells. And it's actually pretty expensive for any cell configuration:
https://pxl.cl/14DbZ
In control group, we never trigger any cell "Re-binding" thus no heavy impact. However the way control group works is, that we always trigger another cell creation and replace with the existing one. IGListKit would still check -isEqualToDiffable: to decide whether the current cell needs a reload, the logic is inside IGListAdapterUpdater.
= Solution =
It's easy, just do a simple `-isEqualtoDiffable:` check before updating the _item, inside `[-IGListBindingSingleSectionController didUpdateToObject:]` method.
Differential Revision: D20909174
fbshipit-source-id: 59d57fc8ddda7210cd1ae333942a345025cf1ee3
Summary: This allows us to potentially enable `-Wundef` in the future. Behavior is the same, undefined macros are false.
Differential Revision: D20906603
fbshipit-source-id: d5798861f0c9dfcd7a87936d575621d52c0ef7c7
Summary: We add a boolean isDisplayingCell method to check the existence of displayingCell in IGListBindingSingleSectionController
Reviewed By: chritto
Differential Revision: D20649871
fbshipit-source-id: f626cfdedff907604514c485d11362cd9ba12b99
Summary:
A common cause of `NSInternalInconsistencyException` is when a section controller tries to access a cell or modify the `UICollectionView` in `-didUpdateToObject`. If the collection view doesn't have data yet, this will kick off the regular calls (ex: `-numberOfSectionsInCollectionView`, `-numberOfItemsInSection`), which won't have the right information yet because the `IGListAdpater` is in the middle of updating the object list. Eventually, this can lead to a crash, which are hard to debug because the callstack doesn't include the call that caused the premature update.
Lets add an assert to catch when this happens.
Reviewed By: candance
Differential Revision: D20527372
fbshipit-source-id: 4ad0b2d89e5b126d6ae888a5c3b1d980945b7a4c
Summary:
Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.10.5 to 1.10.8.
<details>
<summary>Release notes</summary>
*Sourced from [nokogiri's releases](https://github.com/sparklemotion/nokogiri/releases).*
> ## 1.10.8 / 2020-02-10
>
> ### Security
>
> [MRI] Pulled in upstream patch from libxml that addresses CVE-2020-7595. Full details are available in [#1992](https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/1992). Note that this patch is not yet (as of 2020-02-10) in an upstream release of libxml.
>
>
>
> ## 1.10.7 / 2019-12-03
>
> ### Bug
>
> * [MRI] Ensure the patch applied in v1.10.6 works with GNU `patch`. [#1954](https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/1954)
>
>
>
> ## 1.10.6 / 2019-12-03
>
> ### Bug
>
> * [MRI] Fix FreeBSD installation of vendored libxml2. [#1941, [#1953](https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/1953)] (Thanks, [@​nurse](https://github.com/nurse)!)
>
>
</details>
<details>
<summary>Changelog</summary>
*Sourced from [nokogiri's changelog](https://github.com/sparklemotion/nokogiri/blob/master/CHANGELOG.md).*
> ## 1.10.8 / 2020-02-10
>
> ### Security
>
> [MRI] Pulled in upstream patch from libxml that addresses CVE-2020-7595. Full details are available in [#1992](https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/1992). Note that this patch is not yet (as of 2020-02-10) in an upstream release of libxml.
>
>
> ## 1.10.7 / 2019-12-03
>
> ### Fixed
>
> * [MRI] Ensure the patch applied in v1.10.6 works with GNU `patch`. [[#1954](https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/1954)]
>
>
> ## 1.10.6 / 2019-12-03
>
> ### Fixed
>
> * [MRI] Fix FreeBSD installation of vendored libxml2. [[#1941](https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/1941), [#1953](https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/1953)] (Thanks, [@​nurse](https://github.com/nurse)!)
</details>
<details>
<summary>Commits</summary>
- [`6ce10d1`](6ce10d15d7) version bump to v1.10.8
- [`2320f5b`](2320f5bd63) update CHANGELOG for v1.10.8
- [`4a77fdb`](4a77fdb789) remove patches from the hoe Manifest
- [`570b6cb`](570b6cbc5f) update to use rake-compiler ~1.1.0
- [`2cdb68e`](2cdb68e95a) backport libxml2 patch for CVE-2020-7595
- [`e6b3229`](e6b3229ec5) version bump to v1.10.7
- [`4f9d443`](4f9d443c2f) update CHANGELOG
- [`80e67ef`](80e67ef636) Fix the patch from [#1953](https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/1953) to work with both `git` and `patch`
- [`7cf1b85`](7cf1b85a5f) Fix typo in generated metadata
- [`d76180d`](d76180d0d2) add gem metadata
- Additional commits viewable in [compare view](https://github.com/sparklemotion/nokogiri/compare/v1.10.5...v1.10.8)
</details>
<br />
[](https://help.github.com/articles/configuring-automated-security-fixes)
Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `dependabot rebase` will rebase this PR
- `dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `dependabot merge` will merge this PR after your CI passes on it
- `dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `dependabot cancel merge` will cancel a previously requested merge and block automerging
- `dependabot reopen` will reopen this PR if it is closed
- `dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `dependabot use these labels` will set the current labels as the default for future PRs for this repo and language
- `dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language
- `dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language
- `dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language
You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/Instagram/IGListKit/network/alerts).
</details>
Pull Request resolved: https://github.com/Instagram/IGListKit/pull/1429
Differential Revision: D20124605
Pulled By: lorixx
fbshipit-source-id: 05e6117da1ff1eb92da518b0bf08bd7142a1f797
Summary:
## Changes in this pull request
Run ./script/setup.sh to regenerate some of the Example projects missing files, that are causing the build failure in travis CI. Let's see how it goes.
Issue fixed: #
### Checklist
- [ ] All tests pass. Demo project builds and runs.
- [ ] I added tests, an experiment, or detailed why my change isn't tested.
- [ ] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
- [ ] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md)
Pull Request resolved: https://github.com/Instagram/IGListKit/pull/1431
Reviewed By: bdotdub
Differential Revision: D20119287
Pulled By: lorixx
fbshipit-source-id: c9b22b513a248c6428f4730f620f8d72ff37a560
Summary:
**Context**
Recently an IGListKit experiment was introduced, `IGListExperimentPerformUpdatesWithoutDeferringCATransactionCommit`. When enabled, we opt to no longer defer a call to `[CATransaction commit]`, because it could feasibly end a different transaction than intended. In practice, this was leading to issues with `UIViewPropertyAnimator`, where deffered commits were ending in-progress animators.
**This Change**
The results we have seen from enabling this fix show no changes to performance and stability, so this seems safe to roll out. This change removes the experiment, and enables the new, non-deffered behavior.
Reviewed By: lorixx
Differential Revision: D20120169
fbshipit-source-id: 0473652020a250d67b02b860fb74c73e43615aef
Summary:
Pull Request resolved: https://github.com/Instagram/IGListKit/pull/1428
We kept getting an error as the following:
```
- ERROR | [OSX] unknown: Encountered an unknown error (The platform of the target `App` (macOS 10.11) is not compatible with `IGListSwiftKit (4.1.0)`, which does not support `macOS`.) during validation.
```
Looks like IGListSwiftKit did not support macos, wondering why this is the case here.
Potentially this error was introduced in this PR: https://github.com/Instagram/IGListKit/pull/1388
cc natestedman who might know the context here
The solution here is to lint the specs separately in travis, so that for IGListSwiftKit we only lint against ios and tvos. And the other podspecs will still lint against on all platform, including the macos.
Reviewed By: natestedman
Differential Revision: D20036436
fbshipit-source-id: 5406cef1825fc4c157fee9ca5856ace81df0508d
Summary:
We were using the iVar version of the `_displayDelegate`, but turned out a lot of product callsite just manually override the `displayDelegate` instead of use `self.displayDelegate = self`.
Let's use the self.displayDelegate instead.
Reviewed By: bdotdub
Differential Revision: D20037298
fbshipit-source-id: fa860adfa88cd088f39718279983acd32e90b478
Summary:
There is a KP in IGListBindingSingleSectionController that we need to override the `displayDelegate` in order to track the displayingCell for performant update, when the section controller is not visible.
With the newly refactoring in how IGListSectionController handles the displaying events, IGListBindingSingleSectionController can now override the willDisplay/endDisplay calls without manually adding itself as the `displayDelegate`, the behavior works as before.
Differential Revision: D19973670
fbshipit-source-id: 1f9fec1bf88aa8755c163c636ebb49b377e6873c
Summary:
Previously, IGListDisplayHandler took over the sectionController's displayDelegate and invoke directly.
We have requirement in other cases where we want to control the displayDelegate invokation, e.t. Subclass of IGListSectionController can listen to the willDisplay/endDisplay events and do other processing.
Moving the displayDelegate invokation inside IGListSectionController would allow us to have better control on the invokation timing.
Differential Revision: D19973669
fbshipit-source-id: 33d23bc7efb235d2e1ed99737200e56bb320b678
Summary:
When a section/cell is reloaded, UICollectionView actually calls:
1/ WillDisplay (new cell)
2/ DidEndDisplay (old cell)
So if we set `_displayingCell` in willDisplay call, then set it to nil in didEndDisplay, we would just have a nil cell. Thus we need to make sure the cell in `didEndDisplay` match with the previous `_displayCell` to avoid nil-ing ourselves out.
Reviewed By: alanzeino
Differential Revision: D19906734
fbshipit-source-id: 5217908ae99d488a2af5ff3f90ef8e021774acda
Summary:
Pull Request resolved: https://github.com/Instagram/IGListKit/pull/1418
This assert causes crashes on debug builds. While it's known that this algorithm is slow with a large number of items to work through, it doesn't seem to have been an issue for most when working with IGListKit. Removing this assertion to avoid crashes for others.
Reviewed By: bdotdub
Differential Revision: D19251924
fbshipit-source-id: c0e78d3963544b949bed823bdac9b39529ca5f9f
Summary:
Originally we want to update the cell when the SectionController is asked to update to a new value, that's pretty much what the experiment `singleItemSectionUpdates` is for inside `IGListAdapterUpdater`.
However, we should not request any cell during the initial setup, otherwise it would trigger cell to render in a wrong time, in particular we shouldn't call
```
UICollectionViewCell *cell = [self.collectionView cellForItemAtIndexPath:indexPath];
```
as the data source set up is not ready, and it can get crashy to compare the initial dataSource(nil) to the valid data source(IGListAdapter), which doesn't make sense.
So tl;dr is do not access cell during the initial data source setup. We might need to audit other callsite that might have violate this invariant.
Solution here is to rely on the willDisplay/endDisplay API from `IGListDisplayDelegate` so that we can set/unset the displaying cell here before we configure the cell with the updated data. This change is also safe as it's protected by `_enabledCellConfigurationDuringUpdate` which defaults to be NO.
= Facebook =
Also currently the `singleItemSectionUpdates` is only setup for Inbox thread-bump useage for now. So I added a `enabledCellConfigurationDuringUpdate` to properly gate it behind QE so that we have a safer exit here.
Reviewed By: maxolls
Differential Revision: D19332040
fbshipit-source-id: cbca7a63b0486c053ceabe759ce316f5d841fef5
Summary:
The `cellClass` parameter was still documented, but no longer present in the method signature.
Pull Request resolved: https://github.com/Instagram/IGListKit/pull/1412
Reviewed By: lorixx
Differential Revision: D19178421
Pulled By: iperry90
fbshipit-source-id: 17e69db2c5d52581bfd205b5557c6564cf0e5673
