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.

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

Unable to sort a Collection with @cycle/state #937

Open
monojack opened this issue Mar 31, 2020 · 4 comments
Open

Unable to sort a Collection with @cycle/state #937

monojack opened this issue Mar 31, 2020 · 4 comments

Comments

@monojack
Copy link

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"
@tpresley
Copy link

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.

@jvanbruegge
Copy link
Member

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

@monojack
Copy link
Author

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

@staltz
Copy link
Member

staltz commented Apr 13, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants