Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Crash] Auto-diffing bugs and crashes #126

Closed
jessesquires opened this issue Jul 16, 2018 · 3 comments · Fixed by #136
Closed

[Crash] Auto-diffing bugs and crashes #126

jessesquires opened this issue Jul 16, 2018 · 3 comments · Fixed by #136
Assignees
@jessesquires
Labels
bug
Milestone
0.2.0

Comments

@jessesquires
Copy link
Contributor

jessesquires commented Jul 16, 2018 •
edited
Loading

Diffing for collection views with multiple sections fails. And possibly other scenarios.

We don't use this in PlanGrid, so we aren't currently affected by this.

Seems to be a bug in Dwifft. Switching to IGListKit/Diffing is now a top priority.

See also: #125

Currently had to disable this test:
https://github.com/plangrid/ReactiveLists/blob/master/Tests/CollectionView/CollectionViewDriverDiffingTests.swift#L78

UPDATE:

Repro steps

For both table and collection views:

  1. Launch example app
  2. Delete all items from first section
  3. Tap "Flip" button to swap sections until you crash
@jessesquires jessesquires self-assigned this Jul 16, 2018
@jessesquires jessesquires mentioned this issue Jul 16, 2018
Merged
@jessesquires jessesquires modified the milestones: 0.1.2, 0.2.0 Jul 16, 2018
@jessesquires
Copy link
Contributor Author

Also note: the logic is the same between TableViewDriver and CollectionViewDriver, but TableViewDriver does not exhibit the problem.

@jessesquires
Copy link
Contributor Author

Ok, well diffing is all fucked now with 0.1.2

@jessesquires jessesquires changed the title [Crash] Auto-diffing for collection views is broken [Crash] Auto-diffing bugs and crashes Jul 20, 2018
@jessesquires jessesquires mentioned this issue Aug 27, 2018
Merged
4 tasks
@jessesquires
Copy link
Contributor Author

@benasher44 updated original description with repro steps

benasher44 added a commit that referenced this issue Sep 11, 2018
@benasher44
Dwifft -> DifferenceKit (#136)
f3c8d00 
This swaps out Dwifft for DifferenceKit, which uses Paul Heckel's more efficient diffing algo (same one used by IGListKit). Most of the work here is adding layers to avoid Self/associatedtype requirements issues with our protocols. These layers could be removed in the future though, pending changes that would allow this in Swift 5 or generics changes to TableViewModel/CollectionViewModel.

This also fixes #126
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jessesquires jessesquires

Labels
bug
Projects
None yet
Milestone
0.2.0
Development

Successfully merging a pull request may close this issue.

Dwifft -> DifferenceKit plangrid/ReactiveLists
1 participant
@jessesquires