IGListKit
IGListKit copied to clipboard
Instagram
Fix interactive reordering and destroying collection view at the same time
Fix interactive reordering and destroying collection view at the same time
Changes in this pull request
Issue fixed: https://github.com/Instagram/IGListKit/issues/1341#issuecomment-514055500
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.mdfor any breaking changes, enhancements, or bug fixes. - [ ] I have reviewed the contributing guide
@lorixx https://github.com/Instagram/IGListKit/issues/1341#issuecomment-514055500 here is the explanation. It's hard to write unit test for this case because it requires mocking gesture events, but it's easy to reproduce.
Hey folks! I'm going through all the PRs to see which ones we can merge for the next version of IGListKit.
I agree with @lorixx's assessment that we can probably set the adapter to be strongly associated with the collection view since the adapter itself weakly references the collection view.
I'll try making the change and doing some memory profiling to confirm this is safe to do.
Okay, looks like strongly retaining the adapter from the collection view is fine. I was able to confirm in the sample apps that the collection views are properly deallocated once they leave the screen (Same for the list adapters too).
Um. Okay. I just tried running the test suite on it and a variety of tests testing that the collection view and list adapter get destroyed without issue started failing. So, I just tested the repro steps listed in #1341 and it was working fine.
Since this PR is before the deallocation tests were added, I'm going to assume this has actually been resolved over time, and close the PR now.
@PhilCai1993 If you're still around, can you please test the latest version of IGListKit and see if the issue is still visible for you? If so, can you please open a new task with a video of the crash? Thanks a lot! Sorry we weren't able to get to your PR before.