cyclejs icon indicating copy to clipboard operation
cyclejs copied to clipboard

Unable to sort a `Collection` with `@cycle/state`

Open monojack opened this issue 5 years ago • 4 comments

I'm trying to render a list of state.items with makeCollection and I'm using a state lens where the getter sorts that list based on a state.sort value and returns it.

Clicking a "sort" button would update the state.sort value and the lens getter is invoked, creates a new list, sorts and then returns it, but the dom items are not reordered until the next update to state.items.

After further investigating the issue, I found that pickCombine might be the culprit and changing these lines to

if (!ils.has(key)) {
  ils.set(key, new PickCombineListener(key, out, this, sink))
}
sink._remove(ils.get(key))
sink._add(ils.get(key))

would make it work, though I'm not sure about the implications and I don't know if it's introducing a memory leak.

Code to reproduce the issue: Edit beautiful-knuth-k3jsm

Expected behavior: Clicking the Toggle sort button would sort the list items in asc/desc/none order.

Actual behavior: The DOM doesn't update until you add a new item.

Versions of packages used:

"@cycle/dom": "22.6.0",
"@cycle/isolate": "5.2.0",
"@cycle/run": "5.4.0",
"@cycle/state": "1.4.0",
"snabbdom": "0.7.4",
"xstream": "11.11.0"

monojack avatar Mar 31 '20 17:03 monojack

I have the same issue. My hacky workaround has been to set the array to [] then immediately add the sorted items back to the array. It works, but becomes noticeable (flash of white screen) with large arrays. Would be nice to have a proper fix.

tpresley avatar Apr 10 '21 19:04 tpresley

The code sandbox does not really work for me, but I see the code does not use key on the items. To be able to sort them, you should provide one

jvanbruegge avatar Apr 12 '21 09:04 jvanbruegge

I might have this wrong, but I thought that's exactly what itemKey option in makeCollection() is for...

P.S. I don't know why the sandbox wouldn't work for you, sometimes it gets stuck for me as well and I just hit the refresh button on the browser view

monojack avatar Apr 13 '21 09:04 monojack

I think I can reproduce the issue on CodeSandbox. I think the assumption is correct that this may be a bug in pickCombine, we may be missing a test case for simple reshuffling of the instances, such as what a sort does.

staltz avatar Apr 13 '21 10:04 staltz