Summary:
`diffIdentifier` should not have changed before an update starts, otherwise we can't look up the corresponding section-controller.
Lets add an assert when we detect this issue. In a few months, we should evaluate if this is still useful.
Differential Revision: D85456085
fbshipit-source-id: 9aee649477618e2fdcd5b8e972d8eae0719bed85
Summary: Objects in `IGListSectionMap` should be `id<IGListDiffable>`. This is gonna make the next diff easier too. We probably should do the same with `IGListAdapter`, but that requires a bigger refactor for another time.
Differential Revision: D85372926
fbshipit-source-id: 0b9828ce28bd7906a6abf13d6bb3cc2a9fbb2e9c
Summary: One of the implementation files in IGListKit didn't have the appropriate copyright headers, which was causing one of the open source health checks to throw a warning.
Reviewed By: DimaVartanian
Differential Revision: D83947233
fbshipit-source-id: c5ce059373a57b4b235810f42e21bb4ab5181051
Summary:
## Stack
We aim to remove +load methods from the codebase to reduce pre-main startup time and to unblock enabling startup optimizations
## Diff
UICollectionViewLayout+InteractiveReordering marked as Internal in IGListKit. I am assuming that this is an internal implementation detail of the IGListKit and is not supposed to run globally outside of the IGListKit context. Based on that assumption I migrated global +load API to a `setup` method, which is being execution in IGListAdapter constructor, core class of the IGListKit library
Reviewed By: AfrazCodes
Differential Revision: D81592721
fbshipit-source-id: 718b936e669669c66ec5b6c80fefc4bd106170c2
Summary: We're finding ourselves with the need to delesect a lot of cells, but still allow selction. We could do it in each section-controller, but it's easy to mess up and forget in new section-controllers. So, lets add "auto deselection" to `IGListAdapter` directly. When enabled, section-controllers still get the `-didSelect`, but they don't need to deselect the cell.
Differential Revision: D80272474
fbshipit-source-id: 3d905f1f37def6af3cbc6f4d35e256826b7555fb
Summary:
### Context
This stack implements base `UIKit` types in Objective-C by removing the requirement to provide `NSCoder` based constructor in Swift.
### This diff
`IGListKit` seems to have `NSCoder` constructors marked unavailable except for the single type `IGListCollectionViewLayout`. So adding it there.
### Project context / references
[Opti project](https://fburl.com/gdoc/7hekhwd5)
[META UIKit RFC](https://fburl.com/gdoc/9maag852)
Reviewed By: TimOliver
Differential Revision: D77116862
fbshipit-source-id: 9ae48258131a439b1e8ae84ac23ff5ea14b87281
Summary: Many protocols that are conformed only by VCs needed to be marked main actor.
Reviewed By: ebgraham
Differential Revision: D74761535
fbshipit-source-id: b5d047b625df5cf16759aa5c19e5bf57594a3d99
Summary: The default implementation of `-[UICollectionView preferredFocusedView]` can create/dequeue off-screen cells, which causes perf issues and bugs. Lets create `IGListExperimentFixPreferredFocusedView` to return a visbile index-path.
Differential Revision: D74484148
fbshipit-source-id: f710be56b938f8474278cf88b7e2e4268f702f1d
Summary:
Easy little diffs to correct the spelling heathens of the past. O:)
As titled.
Differential Revision: D73490845
fbshipit-source-id: f8868eb5a1ebaac771e73d832baa1245a9cd2c5a
Summary:
## CONTEXT
`IGListAdapterDelegate` currently has methods that are fired when an object is *first* displayed and when an object *ended* display on screen. For many use cases this works but there are use cases in which it would be helpful to know whenever a cell will be displayed or ends display. This is especially relevant in the `IGListAdapterDelegateAnnouncer` which will enable you to add code for tracking cells globally. This change also balances the `IGListAdapterDelegate` with `IGListDisplayDelegate` which has methods which include the cell as well
## PLAN
This diff will be go out in multiple parts
- delegate method addition (many file change due to our pattern of not preferring default implementation of protocol methods)
- Update to `IGListDisplayHandler` and `IGListAdapterDelegateAnnouncer` for invocation of delegate methods
- Unit test additions
## THIS DIFF
This diff changes adds the invocation of `IGListAdapterDelegate` in `IGListDisplayHandler` and `IGListAdapterDelegateAnnouncer`. The cell lifecycle method will be called every time. cell will display and did end display as opposed to the existing methods which are only called the *first* and *last* time an object is displayed
Differential Revision: D70348078
fbshipit-source-id: 820cd22af217f01f2d4916a70e203d9afc4c6a6d
Summary:
## CONTEXT
`IGListAdapterDelegate` currently has methods that are fired when an object is *first* displayed and when an object *ended* display on screen. For many use cases this works but there are use cases in which it would be helpful to know whenever a cell will be displayed or ends display. This is especially relevant in the `IGListAdapterDelegateAnnouncer` which will enable you to add code for tracking cells globally. This change also balances the `IGListAdapterDelegate` with `IGListDisplayDelegate` which has methods which include the cell as well
## PLAN
This diff will be go out in multiple parts
- delegate method addition (many file change due to our pattern of not preferring default implementation of protocol methods)
- Update to `IGListDisplayHandler` and `IGListAdapterDelegateAnnouncer` for invocation of delegate methods
- Unit test additions
## THIS DIFF
This diff changes the delegate to include the methods that pass in the cell. This is a huge many file diff because we prefer to not include default implementations of protocol methods. This makes the delegate change a breaking change.
Reviewed By: maxolls
Differential Revision: D70348102
fbshipit-source-id: 40709ba8a9112d8855d5ce5ecfd5930b5c79bf26
Summary:
The GitHub CI seems to be failing still since we added some new header files and didn't include the necessary ones in the IGListKit.h umbrella header.
This PR:
* Updates the umbrella header to include the new header files
* Updates the visibility of the private headers so they don't need to be included in the umbrella.
* Does a general clean-up of the Xcode project, re-ordering all of the newly imported files.
Assuming this all passes the external CI, we can land it in Phabricator.
Pull Request resolved: https://github.com/Instagram/IGListKit/pull/1622
Test Plan: GitHub CI has passed: https://github.com/Instagram/IGListKit/actions/runs/13851019458
Reviewed By: fabiomassimo
Differential Revision: D71178515
Pulled By: TimOliver
fbshipit-source-id: 1ea643a7bc7a401c45af2776c2a66e3b806035d7
Summary:
## Changes in this pull request
Issue fixed: [1620](https://github.com/Instagram/IGListKit/issues/1620)
### 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/main/.github/CONTRIBUTING.md)
Pull Request resolved: https://github.com/Instagram/IGListKit/pull/1621
Test Plan:
Tested that IG builds with these changes without issue. All seems good.
As
Reviewed By: NSProgrammer
Differential Revision: D70959852
Pulled By: TimOliver
fbshipit-source-id: 087411463ad1b7c0698d7aba543d85c9013abf39
Summary:
It's inefficient to get the unordered array of visibleObjects to then try to find the order of those objects after. Instead, introduce a new `indexesOfVisibleObjects` API to `IGListAdapter` which will inherently preserve order.
Will use this to help find the previous and next "item" to scroll to with Main Feed.
Differential Revision: D70835725
fbshipit-source-id: a5f66681e0184299d92f220be194b5554639a7da
Summary: We'd like to know which `IGListAdapter` is associated with a `UIViewController`, so lets keep track of that. Every adapter should have only 1 controller and it can't change.
Differential Revision: D70631091
fbshipit-source-id: 7f818d7e8f1858a4c80e223be6d5aad5cfc639cf
Summary: Adding more debug info to figure out what's happening in sundial.
Differential Revision: D70545124
fbshipit-source-id: f0ce51529a712328941b9f9b08b70894388bdb14
Summary:
## Context
UICollectionViews are allowed to return nil attribute values; it is not a programmer error and in some cases it can be used as a feature. `IGListCollectionViewLayout` doesn't consider this; if subclasses return nil attributes then a crash will occur.
## This Diff
Let's apply a check to ensure the attribtues are non-nil before adding to the result array.
Differential Revision: D70492114
fbshipit-source-id: b7203ded2928d2892693f95bb56b45f03c2530a2
Summary:
## Context
`IGListCollectionViewLayout` users may have need to know more about the invalidation contexts it uses -- either to supply accurate invalidation contexts, or to understand more about invalidations (example -- usages of preferred layout attributes).
## This Diff
Exposes `IGListCollectionViewLayoutInvalidationContext` publicly, and renames properties to something a little nicer.
Differential Revision: D69679455
fbshipit-source-id: 2bd3e414ea1f48522546854d0287f3ca9d873ec6
Summary: Add more metadata when we return nil cells in IGListKit.
Differential Revision: D69577117
fbshipit-source-id: 0834857eb429154f35bacf7e00d347ce99d28ee6
Summary: When the value is nil, it will result in a crash on iOS 18.
Differential Revision: D69565468
fbshipit-source-id: 5936ec253cd1e0dfb8b494ebb1364c6a14e79170
Summary: We were not passing the right coordinates when finding the indexPath of the cell we are hovering over causing all sort of havoc
Differential Revision: D69207295
fbshipit-source-id: 9b2df93ae00eceb85fde86f766a3b225fc8c56f9
Summary: This allows to get `layoutAttributesForItemAtIndex` for a given cell via list kit, not directly via UICollectionView → layout → …
Reviewed By: avielg
Differential Revision: D68189875
fbshipit-source-id: 9f81143d1a78ade6b9c14b3c012f22f222ded5b2
Summary: This was a killswitch to be safe, so lets clean it up.
Differential Revision: D67072925
fbshipit-source-id: ac54972030e7d53e7abd95025e45773aa3c5efc6
Summary:
Threads has now run into multiple instances of device-only crashes resulting from usage of automatically converted Swift async versions of `performUpdates(animated:)` and `reloadData()`.
See D64428846, D64876801.
The crashes appear to stem from the opaque, automatically generated async methodology:
```
objc completion handler block implementation for escaping callee_unowned convention(block) (unowned Bool) -> () with result type Bool
```
Our `IGListKit` infra assuming ownership of these `callee_unowned` blocks seems to be problematic: the block can be released/nil'd, and our downstream [`[-[IGListReloadTransaction _executeCompletionBlocks:]`](https://www.internalfb.com/intern/logview/redirect/?filename=IGListReloadTransaction.m&line=82&repo=fbsource&revision=c1e4b4a34ff90e4785983b260f10543af6536215&build=655525612) execution then crashes.
Given this incompatibility with automatic conversion, here I propose:
1. marking the relevant `IGListAdapter.h` APIs with `NS_SWIFT_DISABLE_ASYNC`
2. providing manual async APIs in a new `IGListAdapter+Async.swift` extension hosted in `IGListSwiftKit` that existing callers can now make use of.
Differential Revision: D64881920
fbshipit-source-id: b15a5141a79491c1871b92f4f84a0c29b6045ee0
Summary:
In a few cases, we need to listen to all `IGListAdapter` events. We bend over backwards to detect adapters and swap their delegate with a proxy objects. Lets make things simpler and build it right into `IGListKit` by allowing global announcers, starting with `IGListAdapterDelegate`, but we could expand to the others if we like this.
Generally, we want to avoid anything global, but given the complexity of the alternative, this feels like the better tradeoff.
Differential Revision: D64042609
fbshipit-source-id: 0ca6bada27e640fee5a231148427be41994e4d43
Summary:
iOS 18 has new exceptions, which can be a bit tough to debug. The common ones we see:
1. Cell dequeue outside of -cellForIndex
2. Dequeue being called more than once
3. Returning a cell that was not dequeued
Lets add an assert for each. #1 and #2 can actually assert on the problematic call, which will give a nice stack strace. #3 will still have a generic stack trace, but at least we'll see the section-controller.
Differential Revision: D62809660
fbshipit-source-id: 454c44598d4b21e09e6f66eb63c502b1b966a993
Summary:
Enable strict concurrency for `BCNLoginNavigationController`.
Let's embrace Swift 6 syntax :)
This case is easy since the whole `BCNLoginNavigationController` is on main actor.
Differential Revision: D60809973
fbshipit-source-id: 53745fcfbfb63895d224344de6e5063eb67cea7d
Summary:
Mark `NS_SWIFT_UI_ACTOR` for IGFeedItem cells and other protocols.
In order to unblock D60809973, I need to update all these which is error triggered by enabling strict concurrency on Swift 6.
It's a trail and error and fix process, seem details inline.
Differential Revision: D61847332
fbshipit-source-id: 04909647da435c97875b10b3649d923c0b50bb76
Summary:
## Context
Merging multiple batch updates of the same section is inherently unsafe, because each block can mutate the underlaying list and lead to insert/deletes that operate on different before/after lists. The long term fix is to get rid of item level batch updates (insert/delete calls) and use diffing like for sections, but that's a much larger project.
Right now, we have a crash spiking because we're reloading the same cell multiple times. Each time `IGListDiff(...)` returns a pair of insert/delete for the same index (lets say 0). Once these updates are merge together, they look like this:
```
Item inserts = 0, 0
Item deletes = 0, 0
```
We dedupe deletes in `IGListBatchUpdateData`, but don't do anything for inserts, so it looks liket this:
```
Item inserts = 0, 0
Item deletes = 0
```
So we create an insert/delete unbalance and can crash. Tbh, not sure how this hasn't been a bigger issue in the past.
D21822502 tried to remove all duplicate inserts, but that caused other types of crashes, because multiple inserts are allowed.
## This diff
* Lets keep the net insert/delete count the same by keeping track of the number of deletes we remove and apply the same to inserts.
* Add some tests
* Add an experiment flag to kill this if needed to be safe. IG only: Note that I'm making the default true because the crash is happening pretty at start-up.
## Follow up
Over time, the batch update clean up logic was slowly added to multiple place (IGListBatchUpdateData, IGListAdapterUpdaterHelpers, IGListIndexPathResult), which is making it hard to follow. A first step would be to move it in 1 or 2 places and avoid redundant logic (like getting unique deletes). However, the real fix is getting rid of batch updates all together.
Differential Revision: D61719884
fbshipit-source-id: eda7c264c8239a6a106dbe0256fe777b38fae335
Summary: Had to do a preconcurrency import because `gIGRootViewController` is declared as a static var which doesn't work with concurrency checks since it's considered mutable shared state. Will update once I find a better solution.
Reviewed By: ale-sisto
Differential Revision: D61615924
fbshipit-source-id: 2352d488ae18813aae22e341ace97edab4c1c4ef
Summary: In order to be ready for Swift 6 and strict concurrency checking, we will mark IGListAdapter, IGListSectionController and IGListBindable as a NS_SWIFT_UI_ACTOR so that swift code that is isolated to the main actor can interact with these instances without needing to wrap in a separate Task or MainActor.run command
Differential Revision: D60775816
fbshipit-source-id: 24424bc625d577a1b237207bec29ccec7ef90e82
Summary: The delay should be using the `_coalescenceInterval` before it gets updated by `intervalIncrement`
Differential Revision: D60834604
fbshipit-source-id: 1bf18725a302843c12b50711f0628493c4a04e61
Summary:
By making `-performUpdate` completely synchronous sometimes, it unearthed a bug with the early exit where multiple updates can happen at once. Specifically, the shortcut doesn't block updates from being started within `-didUpdateToObject`.
Lets test removing it, since it's happening in a pretty sensitive part of updates.
Differential Revision: D60773894
fbshipit-source-id: 721dffcb6d5353d12693fa873eb0f502bcdf4c02
Summary: In case the collection-view is not visible, lets try batching more updates together to minimize the amount of off-screen work we do.
Differential Revision: D60691861
fbshipit-source-id: 154164fcd370e016c12bace60269c946a153c284
Summary: We'll need access to the tracker from different parts of `IGListKit`, so lets attach it to the view itself.
Differential Revision: D60691860
fbshipit-source-id: 87987b2939a0e13d733d25382ca4384fdfdd875e
Summary:
We want to give higher diffing and coalescing priority to `IGListAdapters` that have a visible `UICollectionView`. Let's create `IGListViewVisibilityTracker`, which uses the views `window`, `hidden`, and `alpha` properties to get a best guess ...
I've also noticed that a few view-controllers call `-performUpdate` even before their views are in the window, so their updates will be slowed down. Lets add `IGListViewVisibilityStateNotVisibleEarly` to make that distinction. A time comparison is not ideal, but there's isn't a great way to tell if a VC is about to be visible without making app specific assumptions.
If we see promising results, we can go even further and look if the frame is in the visible bounds of the window, but that's a bit more complicated and expensive. Lets start simple.
Differential Revision: D60691857
fbshipit-source-id: dccdd6f59210710f5e0c1ca4150336c1381b875b
Summary:
Currently, we always call `dispatch_async` to coalesce updates, which slowing down the first update. Lets try a compromise where we don't delay the first update, but enforce a max update per time.
For example, lets say `minInterval = 10ms`, `intervalIncrement = 10 ms`, and `maxInterval = 30 ms` ...
* At 0ms, the first `-queueUpdate` is called, which calls `_performUpdate` synchronously
* At 2ms, the update is done
* At 5ms, a second `-queueUpdate` is called. Since it's been less than 10ms, we wait 5ms and increase the next window to 20ms.
* At 10ms, the second `_performUpdate` is called.
So effectively, we speed up the first `-queueUpdate`, but enforce a max update per ms. I'm not convinced we really need the increment, but I want to see what happens IRL.
Differential Revision: D60691863
fbshipit-source-id: 85faf2dbc38f754976843fff63e6eada4972581e
Summary: Before changing how coalescing works, lets encapsulate the logic in its own class to keep `IGListAdapterUpdater` simpler.
Differential Revision: D60691862
fbshipit-source-id: 7463ee5349e6171218a25d9ddfa8fc88bdf606fa
Summary: Dispatching to a background thread and back to main can slow things down, since we might have to wait on the main thread to be available. For small diffs, we might be better off just doing it on main. If this does well, we can expend on it, like taking into account device type.
Differential Revision: D60691859
fbshipit-source-id: e1776e1e3bf493d99fcc6fe8af6d4c01d60da660
