Summary: Adds an init override if the generic type is a Swift value conforming to `ListIdentifiable`.
Reviewed By: natestedman
Differential Revision: D27013270
fbshipit-source-id: 44f5c45e6396643d251c23bfedb6cd0a482e3913
Summary:
Silences warning:
```
Source/IGListSwiftKit/IGListCollectionContext+Refinements.swift:90:71: warning: string interpolation produces a debug description for an optional value; did you mean to make this explicit?
fatalError("A nib named \"\(nibName)\" was not found in \(bundle)")
^~~~~~
Source/IGListSwiftKit/IGListCollectionContext+Refinements.swift:90:71: note: use 'String(describing:)' to silence this warning
fatalError("A nib named \"\(nibName)\" was not found in \(bundle)")
^~~~~~
String(describing: )
Source/IGListSwiftKit/IGListCollectionContext+Refinements.swift:90:71: note: provide a default value to avoid this warning
fatalError("A nib named \"\(nibName)\" was not found in \(bundle)")
^~~~~~
```
Reviewed By: natestedman
Differential Revision: D26320097
fbshipit-source-id: a5fa87fbe2cdf2ad8b55f985a904cf0d387debad
Summary:
Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.10.8 to 1.11.1.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a href="https://github.com/sparklemotion/nokogiri/releases">nokogiri's releases</a>.</em></p>
<blockquote>
<h2>v1.11.1 / 2021-01-06</h2>
<h3>Fixed</h3>
<ul>
<li>[CRuby] If <code>libxml-ruby</code> is loaded before <code>nokogiri</code>, the SAX and Push parsers no longer call <code>libxml-ruby</code>'s handlers. Instead, they defensively override the libxml2 global handler before parsing. [<a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2168">#2168</a>]</li>
</ul>
<h3>SHA-256 Checksums of published gems</h3>
<pre><code>a41091292992cb99be1b53927e1de4abe5912742ded956b0ba3383ce4f29711c nokogiri-1.11.1-arm64-darwin.gem
d44fccb8475394eb71f29dfa7bb3ac32ee50795972c4557ffe54122ce486479d nokogiri-1.11.1-java.gem
f760285e3db732ee0d6e06370f89407f656d5181a55329271760e82658b4c3fc nokogiri-1.11.1-x64-mingw32.gem
dd48343bc4628936d371ba7256c4f74513b6fa642e553ad7401ce0d9b8d26e1f nokogiri-1.11.1-x86-linux.gem
7f49138821d714fe2c5d040dda4af24199ae207960bf6aad4a61483f896bb046 nokogiri-1.11.1-x86-mingw32.gem
5c26111f7f26831508cc5234e273afd93f43fbbfd0dcae5394490038b88d28e7 nokogiri-1.11.1-x86_64-darwin.gem
c3617c0680af1dd9fda5c0fd7d72a0da68b422c0c0b4cebcd7c45ff5082ea6d2 nokogiri-1.11.1-x86_64-linux.gem
42c2a54dd3ef03ef2543177bee3b5308313214e99f0d1aa85f984324329e5caa nokogiri-1.11.1.gem
</code></pre>
<h2>v1.11.0 / 2021-01-03</h2>
<h3>Notes</h3>
<h4>Faster, more reliable installation: Native Gems for Linux and OSX/Darwin</h4>
<p>"Native gems" contain pre-compiled libraries for a specific machine architecture. On supported platforms, this removes the need for compiling the C extension and the packaged libraries. This results in <strong>much faster installation</strong> and <strong>more reliable installation</strong>, which as you probably know are the biggest headaches for Nokogiri users.</p>
<p>We've been shipping native Windows gems since 2009, but starting in v1.11.0 we are also shipping native gems for these platforms:</p>
<ul>
<li>Linux: <code>x86-linux</code> and <code>x86_64-linux</code> -- including musl platforms like alpine</li>
<li>OSX/Darwin: <code>x86_64-darwin</code> and <code>arm64-darwin</code></li>
</ul>
<p>We'd appreciate your thoughts and feedback on this work at <a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2075">#2075</a>.</p>
<h3>Dependencies</h3>
<h4>Ruby</h4>
<p>This release introduces support for Ruby 2.7 and 3.0 in the precompiled native gems.</p>
<p>This release ends support for:</p>
<ul>
<li>Ruby 2.3, for which <a href="https://www.ruby-lang.org/en/news/2019/03/31/support-of-ruby-2-3-has-ended/">official support ended on 2019-03-31</a> [<a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/1886">#1886</a>] (Thanks <a href="https://github.com/ashmaroli"><code>ashmaroli</code></a>!)</li>
<li>Ruby 2.4, for which <a href="https://www.ruby-lang.org/en/news/2020/04/05/support-of-ruby-2-4-has-ended/">official support ended on 2020-04-05</a></li>
<li>JRuby 9.1, which is the Ruby 2.3-compatible release.</li>
</ul>
<h4>Gems</h4>
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/sparklemotion/nokogiri/blob/main/CHANGELOG.md">nokogiri's changelog</a>.</em></p>
<blockquote>
<h2>v1.11.1 / 2021-01-06</h2>
<h3>Fixed</h3>
<ul>
<li>[CRuby] If <code>libxml-ruby</code> is loaded before <code>nokogiri</code>, the SAX and Push parsers no longer call <code>libxml-ruby</code>'s handlers. Instead, they defensively override the libxml2 global handler before parsing. [<a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2168">#2168</a>]</li>
</ul>
<h2>v1.11.0 / 2021-01-03</h2>
<h3>Notes</h3>
<h4>Faster, more reliable installation: Native Gems for Linux and OSX/Darwin</h4>
<p>"Native gems" contain pre-compiled libraries for a specific machine architecture. On supported platforms, this removes the need for compiling the C extension and the packaged libraries. This results in <strong>much faster installation</strong> and <strong>more reliable installation</strong>, which as you probably know are the biggest headaches for Nokogiri users.</p>
<p>We've been shipping native Windows gems since 2009, but starting in v1.11.0 we are also shipping native gems for these platforms:</p>
<ul>
<li>Linux: <code>x86-linux</code> and <code>x86_64-linux</code> -- including musl platforms like alpine</li>
<li>OSX/Darwin: <code>x86_64-darwin</code> and <code>arm64-darwin</code></li>
</ul>
<p>We'd appreciate your thoughts and feedback on this work at <a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2075">#2075</a>.</p>
<h3>Dependencies</h3>
<h4>Ruby</h4>
<p>This release introduces support for Ruby 2.7 and 3.0 in the precompiled native gems.</p>
<p>This release ends support for:</p>
<ul>
<li>Ruby 2.3, for which <a href="https://www.ruby-lang.org/en/news/2019/03/31/support-of-ruby-2-3-has-ended/">official support ended on 2019-03-31</a> [<a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/1886">#1886</a>] (Thanks <a href="https://github.com/ashmaroli"><code>ashmaroli</code></a>!)</li>
<li>Ruby 2.4, for which <a href="https://www.ruby-lang.org/en/news/2020/04/05/support-of-ruby-2-4-has-ended/">official support ended on 2020-04-05</a></li>
<li>JRuby 9.1, which is the Ruby 2.3-compatible release.</li>
</ul>
<h4>Gems</h4>
<ul>
<li>Explicitly add racc as a runtime dependency. [<a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/1988">#1988</a>] (Thanks, <a href="https://github.com/voxik"><code>voxik</code></a>!)</li>
<li>[MRI] Upgrade mini_portile2 dependency from <code>~> 2.4.0</code> to <code>~> 2.5.0</code> [<a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2005">#2005</a>] (Thanks, <a href="https://github.com/alejandroperea"><code>alejandroperea</code></a>!)</li>
</ul>
<h3>Security</h3>
<p>See note below about CVE-2020-26247 in the "Changed" subsection entitled "XML::Schema parsing treats input as untrusted by default".</p>
<h3>Added</h3>
<ul>
<li>Add Node methods for manipulating "keyword attributes" (for example, <code>class</code> and <code>rel</code>): <code>#kwattr_values</code>, <code>#kwattr_add</code>, <code>#kwattr_append</code>, and <code>#kwattr_remove</code>. [<a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2000">#2000</a>]</li>
</ul>
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="7be6f04aa2"><code>7be6f04</code></a> version bump to v1.11.1</li>
<li><a href="aa0c399195"><code>aa0c399</code></a> dev: overhaul .gitignore</li>
<li><a href="3d90c6d1bc"><code>3d90c6d</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2169">#2169</a> from sparklemotion/2168-active-support-test-failure</li>
<li><a href="bbf850c629"><code>bbf850c</code></a> changelog: update for <a href="https://github-redirect.dependabot.com/sparklemotion/nokogiri/issues/2168">#2168</a></li>
<li><a href="ee697726dc"><code>ee69772</code></a> ci: another valgrind suppression</li>
<li><a href="f9a2c4e050"><code>f9a2c4e</code></a> fix: restore proper error handling in the SAX push parser</li>
<li><a href="35aa88b75e"><code>35aa88b</code></a> fix(cruby): reset libxml2's error handler in sax and push parsers</li>
<li><a href="07459fd0e0"><code>07459fd</code></a> fix(test): clobber libxml2's global error handler before every test</li>
<li><a href="b682ac5afe"><code>b682ac5</code></a> ci: ensure all tests are running <code>setup</code></li>
<li><a href="007662fc21"><code>007662f</code></a> github: update "installation difficulty" issue template</li>
<li>Additional commits viewable in <a href="https://github.com/sparklemotion/nokogiri/compare/v1.10.8...v1.11.1">compare view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
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/1496
Reviewed By: lorixx
Differential Revision: D26201659
Pulled By: iperry90
fbshipit-source-id: 6e7bc3c0b97119a54e0fa3de806793d4286999e7
Summary: This API is dependent on dynamic types right now, making it awkward and unsafe to use in Swift. By providing a generic wrapper, we can make it safer to use.
Reviewed By: joetam
Differential Revision: D26057302
fbshipit-source-id: ec5d5ed202900f6171761214900fde1f9615e7f5
Summary: We don't need these synthesize anymore.
Reviewed By: joetam
Differential Revision: D26052888
fbshipit-source-id: eedb9acea68846af1a23fea15a44f94351eb999a
Summary: While building the example project, I noticed this warning
Reviewed By: lorixx
Differential Revision: D25962436
fbshipit-source-id: c686e07ac82551f658b40a72280dac29a026cb62
Summary:
`IGListExperimentBackgroundDiffing` has been running on Instagram for a couple months now, and we're seeing nice scroll performance improvements (less frame drops) without increasing crashes. Lets move the experiment to a real property!
Should it be enabled by default?
* I'm leaning toward no, but I could be convinced otherwise. This optimization is good for large/complex applications with lots of diffing, but I'm not sure the more common apps would need it initially. It does make updates more complicated and the order of operation will be a bit different. For example, we've seen some places call `-performUpdatesAnimated` and almost immediately expect the `IGListAdapter` data to be updated, leading to missing views. This issue can still happen without background diffing, but less often.
Reviewed By: joetam
Differential Revision: D25884786
fbshipit-source-id: 3c8755a774f63868b7dfbc8e7a2e5c012a9e7e27
Summary:
Originally, `allowsBackgroundReloading` was added to improve performance, but ironically, it's causing lots of performance issues among other issues.
* Performance: Looking back, it's not too surprising that it causes perf issues. We're falling back to a full `-reloadData` if the view is not in the window, which can happen pretty often. For example, if a view-controller is within a `UINavigationController` stack but not on top, or within a `UITabBarController`. Because a full `-reloadData` will re-query the cells and re-create the entire layout, it's going to be more expensive than an incremental update via `-performUpdatesAnimated`. The proof is in the data and we have a few examples where this flag was the cause of significant UI stalls.
* Bugs: Because we might reload cells often, it can create strange animation artifacts. Specifically, it was breaking the `UIView` snapshots just before a transition, like the new zoom animator.
Overall, we ended disabling this feature and I think most apps will be in the same boat.
But what if this flag does improve my app's performance?
* File an issue and lets chat! I'd be curious to understand why that's the case. If a full `-reloadData` is more performant than an incremental `-performUpdatesAnimated`, than something odd is happening and I don't think this flag is the right solution.
Reviewed By: joetam
Differential Revision: D25884777
fbshipit-source-id: c4626a52082ef4c7b7300b21077529f26c551e70
Summary: Now that the experimental updater is done, we can remove this protocol.
Reviewed By: iperry90
Differential Revision: D25884776
fbshipit-source-id: e7ce962f166aecf73ca1e8fdfa41404bc794696e
Summary:
Lets clean up `-performExperimentalUpdateAnimated`
* Remove the "experimental"
* Re-order the params to match the other method
* Rename dataBlock -> sectionDataBlock to make it clear we mean sections
* Rename applyDataBlock -> applySectionDataBlock
Reviewed By: Haud
Differential Revision: D25884784
fbshipit-source-id: e24c54b43c08c02538c83ba044b1a547cd0f38ae
Summary:
Now that the new updater has shipped, lets update `IGListUpdatingDelegate` with the new methods:
* `-performExperimentalUpdateAnimated` is the new section update method (renaming coming in the next diff)
* `-performDataSourceChange` lets us safely update the `UICollectionView` dataSource
Also, something I've been wanting to do for a long time, lets group related methods in `IGListUpdatingDelegate.h`.
Reviewed By: Haud
Differential Revision: D25884780
fbshipit-source-id: 5d9201ace8bf6b281d71ff03463cb7c911e7f967
Summary:
It's time to ship the new updater! `IGListExperimentalAdapterUpdater` has been running on Instagram for a couple months with better performance and stability. In this diff, we're renaming `IGListExperimentalAdapterUpdater` (and related classes) to `IGListAdapterUpdater`.
Here's a recap:
## Stability
* `[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.
* `[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.
* Other improvements
* 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
* 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.
* Unblocks background layout calculation
* This is a larger project, but these improvements are required to make background work safe.
* Only create the `backgroundView` if needed (although this doesn't really require the new updater)
## 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: Haud, lorixx
Differential Revision: D25884782
fbshipit-source-id: 1357fa23513a239051d5b1766823effa3199f656
Summary: The current `IGListAdapterUpdater` implements `-debugDescriptionLines` which is used by `IGListDebugger` to dump information about all the apaters, so lets also implement it for `IGListExperimentalAdapterUpdater`.
Reviewed By: lorixx
Differential Revision: D25884781
fbshipit-source-id: 86b227258f033f0079058af04915171d6a149241
Summary: This makes sure the `UICollectionView` section count matches what we expect before applying a diff results, if not, we fallback to a `reloadData`. This doesn't decrease the crash rate significantly, but it's nice to have a last line of defense. We do tradeoff crashes for performance issues, but it's better that an app works at all and we'll see the issue via asserts.
Reviewed By: lorixx
Differential Revision: D25884778
fbshipit-source-id: 5011d0907ce0f971ea3a0bf95c1549d52f615982
Summary: This experiment shows a slight decrease in frame-drops on features that have more expensive hashing, so lets ship!
Reviewed By: lorixx
Differential Revision: D25884785
fbshipit-source-id: a5e33abe6f129166ab9b75de80636db801599b74
Summary:
We had shipped the IGListBindingSingleSectionController and have been using it for direct inbox/message/reaction list for a while now and it's quite stable for the inline update infra, there is no need to keep this boolean around anymore.
Let's cleanup for good!
Differential Revision: D25871674
fbshipit-source-id: b6cc9de9308d2d3feb7e354040c1b49ce588a4ec
Summary:
## Changes in this pull request
Add `shouldSelectItemAtIndex:` to `IGListSectionController`.
Issue fixed: https://github.com/Instagram/IGListKit/issues/1033
### 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/1479
Reviewed By: DimaVartanian
Differential Revision: D25562797
Pulled By: lorixx
fbshipit-source-id: 39e398914e35f2cff090c9b8bd9f32a345bc5d6f
Summary:
## Changes in this pull request
Made some translation improvements for Chinese version README.
### 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/1480
Reviewed By: DimaVartanian
Differential Revision: D25562751
Pulled By: lorixx
fbshipit-source-id: 7a089ec4452c5517e0cdfe9ed3156f09554dbcfd
Summary: `NSIndexPath` is in UIKit, so we should be importing its header.
Reviewed By: lorixx
Differential Revision: D25334717
fbshipit-source-id: 5a134a0ddbe88fddb33adfa182d7aaf437bc47d0
Summary:
Trying to improve my previous QE regression on all the perf: https://www.internalfb.com/intern/qe2/ig_ios_listkit_improvements_universe/ig_ios_listkit_improvements_skip_view_section_controller_map/review
My theory is that it's incresing the load in the -visibleObjects call, then we call -[UICollectionView cellForIndexPath:] which is expensive in the `_sectionControllerForCell:`
Thus my new approach is to optimize this -visibleObjects entirely and instead of dealing with visible cells, we can use the `indexPathsForVisibleItems` which can give us the indexpath instead and make it faster. It would be O(N) instead of O(N^2) compared to before.
Reviewed By: calimarkus
Differential Revision: D24663555
fbshipit-source-id: 69fcf2d0b44f8bc06351c92f96c3cbe227c22479
Summary:
Another source-of-truth inconsistency issue.
I kept getting this assertion T65423827 task.
It turned out the the root cause is that we use the `_viewSectionControllerMap` to keep track of the mapping between view and section controller.
Which is dangerous.
Because the UICV's source of truth is the `sectionMap`, now we are adding another source of truth between view <-> sectionController to be `_viewSectionControllerMap`, this is just waiting for bugs to happen.
We should always refers to sectionMap as the source of truth for everything here: since we can get the section index, we can replace all the `-sectionControllerForView` to use `-sectionControllerForSection:` instead, which is safer.
Multiple source of truth is the bug for this unexpected assertion.
Once we can cleanup this, then we dont need to use that weird: `- (void)mapView:(UICollectionReusableView *)view toSectionController:(IGListSectionController *)sectionController` API which we might forget to map, that can cause bug.
Reviewed By: elfredpagan
Differential Revision: D24173911
fbshipit-source-id: 6138cafc0b382fc569d17b14b13a6b50d85d342d
Summary:
## Changes in this pull request
### 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/1466
Reviewed By: bdotdub
Differential Revision: D24235336
Pulled By: lorixx
fbshipit-source-id: bfc5fab26ae28ff9306d1b889b82b49c6ae2a636
Summary: Some product callsite just pass in nil object to this API thus keep causing the assertion. It's a bit too noisy and not much value here since we by default would still return `NSNotFound` here.
Reviewed By: stavash
Differential Revision: D24233480
fbshipit-source-id: 38b2eed3a9286bdd0a7eb0467894a13bbdca007a
Summary:
## Changes in this pull request
### 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/1453
Reviewed By: lorixx
Differential Revision: D23836038
Pulled By: joetam
fbshipit-source-id: 8245415992a5b1ed49f67ebfcf9f85a2745a8042
Summary:
This diff introduces two experimental APIs to make `IGListKit` more usable with Swift values types:
- The `ListIdentifiable` protocol, which provides the equivalent of the diff identifier half of `ListDiffable`. For equality, we just use Swift's native `Equatable`.
- The `ListValueSectionController` base class, which provides a section controller interface for use with `ListIdentifiable` values.
These two APIs work together through the use of a private box class. I'd like to keep this detail hidden from callers, so that product code only deals with native Swift types.
Differential Revision: D23606413
fbshipit-source-id: 23718c508643f165c74550f1515d77448bd42d42
Summary:
Pretty sure in `-invalidateLayout`, we should calculate the `NSIndexPaths` after the delay. Otherwise, those `NSIndexPaths` might be incorrect after the update.
Before this change, running `test_whenInvalidatingInsideBatchUpdate_andRemoveThatSectionController_thatCollectionViewDoesntCrash` crashes with:
> Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'attempting to invalidate an item at an invalid indexPath: <NSIndexPath: 0x9566e79c2077363c> {length = 2, path = 1 - 0} globalIndex: 1 numItems: 1'
Reviewed By: candance
Differential Revision: D23742819
fbshipit-source-id: 39496d04025ff3554673313df3c7516c33c305d4
Summary:
These methods are currently correctly annotated as `nullable`, because they can return `nil`. However, this is not practical for writing actual Swift code that uses the APIs. Overrides of `cellForItem(at:)` //must// return a `UICollectionViewCell`. Without a collection context, there's no good way to get one, so callsites must either:
- Force-unwrap the `collectionContext`.
- `guard let`/`else fatalError` the `collectionContext`.
- Return a default `UICollectionViewCell`, which just hides the bug.
None of these are good: we don't want to encourage things force-unwraps and `fatalError` in idiomatic product code, because people will see them and get the idea that these patterns are okay to use elsewhere, and they are not. This also happens with `IGListGenericSectionController`'s `object` value: to use it when creating a cell, it must be explicitly unwrapped.
We could make these `nonnull`, since there's no enforcement of the annotations in Objective-C, that feels a bit too far, and it would break existing code that uses the optional API, because you can't force-unwrap (etc.) a non-optional value. Instead, we can use `null_resettable` explicitly. [While it's not covered by name in the Apple docs](https://developer.apple.com/documentation/swift/objective-c_and_c_code_customization/designating_nullability_in_objective-c_apis):
> Without a nullability annotation or with a null_resettable annotation—Imported as implicitly unwrapped optionals
...it bridges the property to Swift as an implicitly-unwrapped optional. I suppose it's implied as the equivalent of "without a nullability annotation".
This works even for `readonly` properties that can't be reset, and it solves both issues: it feels like less of a "100% `nonnull`" guarantee, and it's backwards-compatible with code using optionals.
To demonstrate that this is backwards-compatible with existing Swift code, this test code, which runs through various optional/non-optional APIs, compiles:
```
final class TestSectionController: ListSectionController {
override func cellForItem(at index: Int) -> UICollectionViewCell {
let cell = collectionContext.dequeueReusableCell(for: self, at: index)
let optional = collectionContext?.dequeueReusableCell(for: self, at: index)
if let _ = collectionContext {}
return optional ?? cell
}
}
final class TestGenericSectionController: ListGenericSectionController<NSString> {
override func cellForItem(at index: Int) -> UICollectionViewCell {
let label = UILabel() // imagine it's from a cell
label.text = object as String
label.text = (object as String) + "Suffix" // wouldn't work with optional
label.text = object.map { $0 as String }
if let object = self.object {
label.text = object as String
}
return collectionContext.dequeueReusableCell(for: self, at: index)
}
}
```
Reviewed By: patters, lorixx
Differential Revision: D23603716
fbshipit-source-id: e4750dcfe0072d482951dbf2e9efb1ee3de46884
Summary: If we apply the data updates outside the `performBatchUpdates`'s `updates` block, we need to make sure the layout is up to date. Otherwise, `performBatchUpdates` will try to do a layout before calling `updates` and get the wrong cells. Calling `[self.collectionView numberOfSections]` will update the section/item count, but won't get the cells.
Reviewed By: Haud
Differential Revision: D23604257
fbshipit-source-id: 02bbf0484c46cf6e7a12ed02d45ededb3b84ac19
Summary:
This is currently just `id`, so when you override it, you can't use the local without casting. We should declare it as `ObjectType` so that it gets the specialized type instead.
The implementation just assigns to the generic `_object` property-backing ivar, so no difference in type-safety.
Differential Revision: D23589584
fbshipit-source-id: 3784929f89580fd603fffcf940c7a5b502501bc5
Summary:
* Added new files to xcodeproj
* Ran `pod install` on example projects
Reviewed By: natestedman
Differential Revision: D23542185
fbshipit-source-id: e0d03f915fa0861860d8ffc2ab8701d761d27069
Summary: We don't need to create the `emptyViewForListAdapter` if it's hidden
Reviewed By: patters
Differential Revision: D23429238
fbshipit-source-id: 8d85964c177f53a0e0cc0867339c1cbf0db2ee4e
Summary: We don't actually need to call `[UICollectionVew performBatchUpdates ...]` if we don't have changes.
Reviewed By: patters
Differential Revision: D23386181
fbshipit-source-id: 469a22a35b9ead5181e95f36a8bc59ba36373bbe
Summary:
We're seeing some crashes where the `UICollectionView`'s section count doesn't match the `fromObjets`. Ideally that shouldn't happen, but in case it does, lets fallback to a `reloadData` instead of crashing. Lets `IGFailAssert()` to keep an eye on it.
EDIT: Turns out `UICollectionView` will sometimes already do that for item-level inconsistencies! Example error message:
> The number of items contained in an existing section after the update (9) must be equal to the number of items contained in that section before the update (10), plus or minus the number of items inserted or deleted from that section (0 inserted, 0 deleted) and plus or minus the number of items moved into or out of that section (0 moved in, 0 moved out). - will perform reloadData.
But it still throws a `NSInternalInconsistencyException` for section-level inconsistencies.
Reviewed By: patters
Differential Revision: D23386178
fbshipit-source-id: a9930b619701cffd37d0d6e2bcb1268ef7c9d0ce
Summary:
It's easy to create a retain cycle or just forget to release a pointer, so lets test that the expendable parts of `IGListExperimentalAdapterUpdater` get deallocated after an update.
Lets also try to group related tests together. It's getting kind of hard to tell which tests already exist.
Reviewed By: patters
Differential Revision: D23386180
fbshipit-source-id: 90f0e6c7532287ee245e788dd752d45f368dc27e
Summary:
Changing the `IGListAdapter`'s `collectionView` or `dataSource` isn't safe. For example:
* If we change the `dataSource`, the `collectionView` will be out of sync with the `IGListAdapter`'s data and can easily crash.
* If we change the `collectionView` or `dataSource` during background diffing, the diffing result will be out of date by the time we get back to to the main main thread.
To fix this:
* On `[IGListAdapter setDataSource: ...]`, lets also change the `UICollectionView`'s `dataSource` to invalidate any section/item counts. We don't have to call `[UICollectionView reloadData]` just yet, we just want to let the collectionView know that the counts might have changed.
* On `[IGListAdapter setDataSource: ...]` and `[IGListAdapter setCollectionView:...]`, we should cancel any pending or on-going updates. The tricky part is that we should still execute the `itemUpdateBlocks` and `completionBlocks` in the right order.
I kind of want to make the `collectionView` readonly, but I think it would be too large of a public API change at this point.
Reviewed By: patters
Differential Revision: D23151919
fbshipit-source-id: 61cf025127706acaf22f153ec148ac0ea575bc98
Summary: Same as last diff, but for `IGListBatchUpdateTransaction`
Reviewed By: patters
Differential Revision: D23151920
fbshipit-source-id: 8941791b7870f34077a5e2beded2e913b453fbbf
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
