Summary:
This is a bigger PR than I'd like but there is a bunch of stuff in here that feels like we should land all at once to fix the Travis setup.
Currently, there is a few things wrong. Briefly:
1. Travis is failing to even build the library (see: [build 2298](https://travis-ci.org/Instagram/IGListKit/jobs/506564900))
2. There are legitimately failing tests
3. Travis seems to be flakey running UI tests
This PR deals with the first two.
The Xcode projects were not building because some recently added files weren't added to the xcodeproj (ie. 46a124ddfe and 4662454c4a).
Also added `Foundation.h` import for `bool` definition for `IGSystemVersion` (also a new file)
The main `Podfile.lock` was already being generated via `1.5.3` (0c0b31ad7c) but the `Gemfile` had the wrong version and none of the example project `Podfile.lock`s were re-generated with it.
Once I got the projects correctly building and tests running, I discovered there were a bunch of tests that were legitimately failing. It's unclear when these started failing and what caused it.
I thought it made sense to disable these tests for now and then create Github issues to fix them as starter tasks instead of making this diff even larger.. I can create those if that sounds good.
Somewhat related, it seems like the UI tests on Travis may be a bit flakey. They pass locally, but breaks on Travis [[example](https://travis-ci.com/bdotdub/IGListKit/jobs/195758250)] with error messages like `Failure requesting automation session for com.instagram.IGListKitExamples:85190`
----
- [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/1317
Reviewed By: rnystrom
Differential Revision: D15157375
Pulled By: rnystrom
fbshipit-source-id: b29131f59b74398b6d2b3a73453248cb3325a955
Summary:
* Currently, there's no easy way to measure how long we spend dequeueing cells or handling scroll events across the app. This makes it a bit difficult to catch scroll-perf issues early on.
* Fix: Lets create `IGListAdapterPerformanceDelegate` which gives us that information.
Reviewed By: rnystrom
Differential Revision: D14430449
fbshipit-source-id: 7bc170bdaa37807c8719976e8818f3a578ee17dd
Summary:
We have observed that keeping the coalescing time here would actually decrease the stability here, it makes the app crash more often.
It's probably caused by some race condition during that time and UICV updates become out of sync.
Reviewed By: rnystrom
Differential Revision: D14379265
fbshipit-source-id: 502c1c14fb8bdfc35969f721687e82888b160110
Summary:
This assert was hoping to find if there are any duplicates from the `fromObjects`. However, from the finding, we did not see any assertion or trace that this is the case.
The `objectsWithDuplicateIdentifiersRemoved` is actually doing extra work during updates.
We should remove this for the sake of better scroll perf and faster UI update.
Reviewed By: calimarkus
Differential Revision: D14339124
fbshipit-source-id: ff40fd23a05640058673d46f63a9e4bf516dd3f6
Summary:
Added working range handler tests. This will allow changes/experiments to be made to the working range handler with improved confidence.
Two of the added tests are currently disabled; working range updates don't currently take inserts and deletes into account and so these tests fail.
Reviewed By: rnystrom
Differential Revision: D13717357
fbshipit-source-id: dd4523d3162cfe13ee03e9d10ffa22b58514ca5f
Summary: This change allows arbitrary layouts to be used in tandem with IGListCollectionView and recieve udpated section indices, by conforming to to conform IGListCollectionViewLayoutCompatible.
Reviewed By: maxoll
Differential Revision: D13671204
fbshipit-source-id: e030f413aefff71754731d1259037077ffb73959
Summary:
Issue fixed: #1288
- [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/1289
Reviewed By: timonus
Differential Revision: D13590406
Pulled By: rnystrom
fbshipit-source-id: 0814b0c2383a185da747176bad57dfb15c7e883f
Summary: This diff adds a new `IGListExperimentBackgroundDiffingSerial` experiment that behaves similarly to `IGListExperimentBackgroundDiffing` but executes changes in a per-`IGListAdapterUpdater` serial queue instead of a global queue. Global queues are concurrent, and concurrent queues by nature don't ensure the execution order of the blocks that are enqueued into them, so there's a chance that the former `IGListExperimentBackgroundDiffing` experiment would lead to changes being applied out of order.
Reviewed By: calimarkus
Differential Revision: D13567403
fbshipit-source-id: a2aebb20e5e7dc22601fad3c2dfc80112747c81b
Summary: Frequently my first response to new issues. Codifying as part of new issue flow.
Reviewed By: maxoll
Differential Revision: D12839035
fbshipit-source-id: 81a907cd93c2d53e95d6f806242a06cc56368d6a
Summary:
Some calls to cellForItemAtIndex: on IGListSectionController expect this method to return a reference to an existing, already dequeued cell. It doesn't do this - it dequeues a new cell instance and returns that.
Added a warning note to the docs.
Reviewed By: lorixx
Differential Revision: D13222395
fbshipit-source-id: bb7a80f9f005e2675eef3d01a236a5f9f0008b46
Summary:
Issue fixed: SwiftLint warnings
- [x] Operator Usage Whitespace Violation: Operators should be surrounded by a single whitespace when they are being used. (operator_usage_whitespace)
- [x] Sorted Imports Violation: Imports should be sorted. (sorted_imports)
- [x] Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
- [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/1283
Differential Revision: D13237028
Pulled By: rnystrom
fbshipit-source-id: b364f329b0e99ca56704d3d2851cf225759b2a78
Summary:
This happens when we set collectionView.dataSource to be nil before calling performUpdates:, the crash is easily reproduced by a simple unit test.
To make the infra more robust and we don't crash the app, let's add an early return before applying a collectionView update.
Reviewed By: rnystrom, calimarkus
Differential Revision: D12896196
fbshipit-source-id: ab024d0e7e9282d50c3be297e1e67cfccaff8242
Summary:
Issue fixed: #1235
- [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/1236
Reviewed By: lorixx
Differential Revision: D12839081
Pulled By: rnystrom
fbshipit-source-id: 3e9e1192f16912d560d76f3730a377f303708cd7
Summary:
Issue fixed: #1219
No tests were added and no entry was added to `CHANGELOG.md` since this is a trivial change.
- [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/1242
Differential Revision: D12840487
Pulled By: rnystrom
fbshipit-source-id: ded4b09765c5666e23a9aa3596328e90df2b3229
Summary:
Issue fixed: #1223
- [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/1241
Differential Revision: D12838898
Pulled By: rnystrom
fbshipit-source-id: af47550dd320fb0e813e8ae22bc7051c2460a05d
Summary: This would help us understands what was the diff result from the algorithm and compare it with the `updates` to check what went wrong. Essentially, this would make our debugging easier going forward.
Reviewed By: calimarkus
Differential Revision: D12828892
fbshipit-source-id: 6dc52cdba1adb5a841760b51599e2df4c845c364
Summary:
There are some crashes related to this path when the number of sections are changed when using the `preferItemReloadsForSectionReloads` optimization for item reloads.
For now, this change would just limit the flag only useable when the section count is unchanged. The proper fix would be finding a repro-case for the unsafe trace.
The follow-up here is adding more logging to capture the crash reason and investigate any variable that leads to this crash.
Reviewed By: calimarkus
Differential Revision: D12828061
fbshipit-source-id: 040e66c051c54f889ee9fbc3e4a48ab6bf93e162
Summary: Let's use simple NSNumber wrapping to make diffable objects for most primitive types. This should be faster than building strings.
Reviewed By: calimarkus
Differential Revision: D10416548
fbshipit-source-id: af5b0a5afc6cd2c45c7e0bf2aa872864c12cbd98
Summary: This lets callers provide a fallback object value (e.g. `[NSNull null]`) for safety when the returned object is required to be non-null.
Reviewed By: natestedman
Differential Revision: D10504059
fbshipit-source-id: 1217755e84a927fe8cfbd0e944af55613f1f7c75
Summary: Provide additional context in willPerformBatchUpdates, which can be useful for debugging.
Reviewed By: lorixx
Differential Revision: D10436588
fbshipit-source-id: a59803affc24ca20f7726e36117df6126b984afb
Summary: Verify our assumption that the `fromObjects` should never contain duplicate identifiers.
Reviewed By: calimarkus
Differential Revision: D10410190
fbshipit-source-id: 4952698fee6373224b4c8395778009540c5ae380
Summary:
section moves are extremely unsafe and crashy.
If we still use the `preferItemReloadsForSectionReloads` on, the item reloads won't be correct when we applies the same logic for cases without section moves.
It leads to crash:
```
2018-09-04 23:28:12.184875-0700 xctest[51440:12103918] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Invalid update: invalid number of items in section 0. The number of items contained in an existing section after the update (1) must be equal to the number of items contained in that section before the update (1), plus or minus the number of items inserted or deleted from that section (0 inserted, 1 deleted) and plus or minus the number of items moved into or out of that section (0 moved in, 0 moved out).'
```
even if we change to use -reloadSections: instead, we still get crash like the following:
```
2018-09-04 23:21:14.129961-0700 Direct[45654:12078189] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'attempt to perform a delete and a move from the same section (25)'
```
Thus let's only add the moves.count == 0 check before using the prefer item reloads over section reload optimization.
Reviewed By: calimarkus
Differential Revision: D9661723
fbshipit-source-id: 8c3ddcd5178cb1aa58ccd9ae274cd8e5198f7609
Summary:
This made the items reload safer, as I tested that passing in an out-of-bound section index to -[UICollectionView numberOfItemsInSection:] would crash the app.
UIKit did not handle this gracefully.
Reviewed By: calimarkus
Differential Revision: D9624061
fbshipit-source-id: bcf14a3d4746b648046992adf221faff36a0257c
Summary: Adding a test to catch the bug that caused the crash in T33376364. In that bug, dequeueing two cells of the same class, but with different reuse identifiers, would cause a crash. Here in the test, if the same bug were to occur the dequeuing of the second cell would throw an exception and the test would fail.
Differential Revision: D9560911
fbshipit-source-id: 9310839aaf441e919b5d443feaa44c71e991d146
Summary:
When we added the ability to register the same class with multiple reuse identifiers, forgot to adjust the `registeredClasses` set to reflect that. This resulted in a bug where the collection view could be asked for a cell of a class that had already been registered, but with a different reuse identifier, which would end in a crash.
This diff fixes the crash, I'll follow up with another diff that adds unit tests around this, but want to fix master ASAP.
Differential Revision: D9560338
fbshipit-source-id: e8df6fe5d9929b44842c6abc01d52064ccb4147e
Summary:
We have seen unnecessary flashiness updates for any section reload updates.
Basically we have 1 section for 1 cell, which is recommended by the IGListKit document in https://instagram.github.io/IGListKit/best-practices-and-faq.html, specifically:
> "We highly recommend using single-item sections when possible."
However, the issue is that whenever we update the underlying data model, we would trigger a delete+insert for the section reload.
In order to mitigate that, we can fix this delete+insert changes or we use the `IGListBindingSectionController`, however, the latter one requires more refactoring plus view model wrapper to have `isEqualToDifferable:` to return YES, which seems less than ideal.
I am proposing that we add an option to the `IGListAdapterUpdater`, as `preferItemReloadsForSectionReloads` so that when it's set to YES, we would trigger the proper `-[UICollectionView reloadItemsAtIndexPaths:]` which handles the cell update properly.
This option also handles the case where the number of items in the section is changed before and after the updates, in that case, it will fallback to do the "delete+insert" section operation.
Reviewed By: rnystrom
Differential Revision: D9519519
fbshipit-source-id: 22ecca07679ebdd212cf771c61e40887cb6a9ba8
Summary: Causes more crashes, removing since this isn't a useful feature.
Reviewed By: maxoll
Differential Revision: D8543859
fbshipit-source-id: 681aeaa23143e119405eefa2c2775180c51a5657
Summary: After experimenting with scroll performance gains, default `IGListAdapter` behavior to use the faster means of collecting visible section controllers.
Reviewed By: maxoll
Differential Revision: D8543686
fbshipit-source-id: e33a78c0f1d4f208705ff5d490ab032eb6c36e70
Summary:
Issue fixed: #1174
- [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/1213
Differential Revision: D9003398
Pulled By: rnystrom
fbshipit-source-id: 2c68f42e21abaea9191f26309668d866544f80b4
Summary:
Issue fixed: #
- [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/1211
Differential Revision: D9003392
Pulled By: rnystrom
fbshipit-source-id: 73ef837300f8fdb6d9a6005e86f8e9e842827979
Summary:
Fixed the order of variables in this assertion.
- [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/1203
Differential Revision: D8770959
Pulled By: rnystrom
fbshipit-source-id: 982996cc218d5db32a0fb67d045ac05870741d71
Summary:
- Run setup script with CP 1.5.3
- Update to Xcode9.4 recommended settings
- Fix Xcode warnings about NSUInteger
This way we can avoid these changes on other open PRs/diffs.
Pull Request resolved: https://github.com/Instagram/IGListKit/pull/1216
Differential Revision: D8990061
Pulled By: rnystrom
fbshipit-source-id: ae15a267d7e6f19f71203ac4cd7d80ee966ee48b
Summary: The `IGFailAssert` for duplicate object identifiers causes unit tests to fail in CI / IGListKit because assertions aren't silenced. Changing this to a log for now.
Reviewed By: rnystrom
Differential Revision: D8773452
fbshipit-source-id: 369415fda4ba3eef8fe8684dc792b0a9905a4759
Summary:
Issue fixed: #1187
- [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)
Closes https://github.com/Instagram/IGListKit/pull/1188
Differential Revision: D8204493
Pulled By: rnystrom
fbshipit-source-id: 52bf75e6436d95b127d44302c4eb6f0bfdc1da08
Summary:
Issue fixed: #859
- [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)
Closes https://github.com/Instagram/IGListKit/pull/1186
Differential Revision: D8264918
Pulled By: rnystrom
fbshipit-source-id: 9f32c085f305299efd839bb365a5d32109ff4f17
Summary: There is one private method in IGListKit which isn't being called from anywhere. We should just remove it.
Reviewed By: rnystrom
Differential Revision: D8346790
fbshipit-source-id: b74766373ca6b9262e03c728b26b647e164fcb03
