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

Fast sorting on with optimistic updates while saving to server is broken #625

Closed
mwhnrt opened this issue Jan 14, 2025 · 11 comments · Fixed by #626
Closed

Fast sorting on with optimistic updates while saving to server is broken #625

mwhnrt opened this issue Jan 14, 2025 · 11 comments · Fixed by #626
Assignees
Labels
bug Something isn't working

Comments

@mwhnrt
Copy link

mwhnrt commented Jan 14, 2025

Moving items fast on the "Svelte dnd action - saving to a 'server' with optimistic updates and recovery from failed operations" example sometimes breaks the whole list by removing items.

each_key_duplicate.mov

This goes aloing with a Uncaught Svelte error: each_key_duplicate error and is possibly related to handling an error response from the faked server:

image

For it being easier to reproduce I just disabled the alert() when the "server" fails. Here's my playground which is directly forked from the example without any further changes: https://svelte.dev/playground/7a5c54b988734e478b41b4e947bcc06a?version=5.17.4

I originally discovered this bug in our application but found the same happens with the basic example here.

System:
macOS 14.6.1 (23G93)
Chrome 131.0.6778.265

@isaacHagoel
Copy link
Owner

Hi @mwhnrt ,
from a brief look it seems like the source of the issue is the naive implementation of saving to the server. It doesn't account for multiple updates at the same time.
The disappearing item and console.error seem to be separate issues (I could reproduce one without the other).
A naive temp solution would be to disable drag while there is an update in flight using the dragDisabled prop.
I will make some time this week to look into this in more depth and update.

@isaacHagoel
Copy link
Owner

mmm.. i discovered something strange.
@mwhnrt Can you reproduce the bug on this REPL https://svelte.dev/playground/6bdbaa13e781482084119946dabd4228?version=5.18.0 ?
I made a tiny change that was supposed to be a preparation for the actual solution but it made the problem go away for me.

@mwhnrt
Copy link
Author

mwhnrt commented Jan 16, 2025

@isaacHagoel just tested it and I wasn't able to reproduce the issue either. Also the errors from the console are gone. 🎉

What has been changed?

@isaacHagoel
Copy link
Owner

isaacHagoel commented Jan 16, 2025 via email

@isaacHagoel
Copy link
Owner

Ah okay, it doesn't actually work. it just doesn't revert when the server fails. Okay. I will keep investigating in the original direction.

@isaacHagoel
Copy link
Owner

okay, now it reverts and I can't reproduce the issue.
https://svelte.dev/playground/6bdbaa13e781482084119946dabd4228?version=5.18.0
the logic of mid drag updates was making an assumption that was correct with some older versions of the lib (that the shadow item retains a temp id throughout the drag).
@mwhnrt Can you please test?

@mwhnrt
Copy link
Author

mwhnrt commented Jan 16, 2025

@isaacHagoel sorry to say, I can still reproduce the bug. Took a while, but after some time dragging the items one disappeared. Screenshot_2025-01-17-00-54-25-669_com.android.chrome.jpg

@isaacHagoel
Copy link
Owner

isaacHagoel commented Jan 17, 2025 via email

@isaacHagoel
Copy link
Owner

just an update. The item doesn't disappear. If you inspect the DOM you'll see that it's there. There problem is that it stays with the styling of the shadow item (but without the attribute that marks it as the shadow item in the 'items' list). Next step is to figure out the exact race condition that causes it.

@isaacHagoel
Copy link
Owner

I fixed the issue (in version 0.9.55) and updated the official example.
The console error was a bug in the example logic as stated before.
The "disappearing" item (shadow item that wasn't made visible) was a subtle library bug.
@mwhnrt If you can still reproduce any of these issues (or see any other issues) please re-open this issue.
Thanks for reporting.

@isaacHagoel isaacHagoel self-assigned this Jan 19, 2025
@mwhnrt
Copy link
Author

mwhnrt commented Jan 19, 2025

Thanks @isaacHagoel will test that tomorrow!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants