hello-pangea / dnd

Beautiful and accessible drag and drop for lists with React.
https://dnd.hellopangea.com
Other
2.13k stars 88 forks source link

Flicker after synchronous state update with react-query #424

Open Liboul opened 2 years ago

Liboul commented 2 years ago

Hey 👋 Love the library, thanks for taking over react-beautiful-dnd, adding typing, supporting React 18 ... it's great! I had this issue pop up while making the switch from react-beautiful-dnd, which makes me think it's a small regression

Expected behavior

Making a synchronous state update through react-query's setQueryData should not cause a flicker. By "flicker", I mean that the dropped element goes back to its original place then to the right place.

Actual behavior

The dropped element goes back to its original place for a time

Steps to reproduce

  1. Implement onDragEnd to make an optimistic synchronous update, using react-query's setQueryData.
  2. Drop an item.
  3. Witness the flicker.

Suggested solution?

Under the hood, react-query uses react's hook useSyncExternalStore. My best guess is that @hello-pangea/dnd does not wait for the state update before moving the dropped item back to its original place, as is described here

We try hard to ensure that an entire lifecycle is completed before a new one starts. If you find that not to be the case - it is a bug: please raise it!

What version of React are you using?

18.2.0

What version of @hello-pangea/dnd are you running?

16.0.0

What browser are you using?

Chrome Version 105.0.5195.102

Demo

Minimal use case implemented in this sandbox: https://codesandbox.io/s/react-beautiful-dnd-flicker-after-synchronous-update-with-reactquery-n207n1

https://user-images.githubusercontent.com/16024169/190988569-a145890f-1852-4c2b-9d65-d68fbb123b10.mov

Note: To enhance the issue on this dummy example, I faked a costly rendering time.

Gorbus commented 1 year ago

We're having exactly the same issue in the same configuration, any progress on this ?

Liboul commented 1 year ago

So far the only hack I have is to duplicate the state between react-query's cache and a local component state, but that's really not ideal, as it's easy to get out of sync and it requires more code.

I did try to flushSync in various places, to no avail.

Xhale1 commented 1 year ago

Interesting. I ran into a similar issue when switching from react 17 to 18, which makes me think it's primarily an issue with how the library supports react 18.

The solution ended up being a simple flushSync call, I'll play with the code sandbox and see if I can find a solution in the morning. Thanks for the report!

Gorbus commented 1 year ago

Thanks a lot for having a look @Xhale1 , for us it's a very problematic regression. Ready to test if you have anything you think may help.

Gorbus commented 1 year ago

Any success on the subject by any chance @Xhale1 ? Still preventing us to upgrade to latest react unfortunately, but couldn't find a fix on my own :(

Liboul commented 1 year ago

I guess one could try dichotomizing versions:

To see which exact version caused the regression, which would narrow down the culprits. I don't have any time to dedicate to this right now, maybe @Gorbus?

nunocaseiro commented 1 year ago

I have the same issue. Is there any update regarding the fix?

Gorbus commented 1 year ago

As react-beautiful-dnd was updated to be react 18 compatible we went back to the other library : react-beautiful-dnd/

ghost commented 1 year ago

Is there any updates on this? I am struggling with the same issue...

nunocaseiro commented 1 year ago

+1

hichemfantar commented 1 year ago

As react-beautiful-dnd was updated to be react 18 compatible we went back to the other library : react-beautiful-dnd/

Are you having the same issue with beautiful dnd?

saogregl commented 1 year ago

Is there any updates on this? I am experiencing the same issue... Been thinking about keeping a local state to "isolate" the draggables from the state updates performed by react query, but this requires more (seemingly) unnecessary code.

arnolddddddd commented 1 year ago

Also having the same issue react v 18.2.0

elsigh commented 1 year ago

I also had the same issue with react-beautiful-dnd. I believe the cause is more wrt react 18 based on all the comments I've read. Perhaps under the hood this might be a good opportunity to try useOptimistic?

kr4chinin commented 3 months ago

Still facing this issue, any recent workarounds?

irishcarbomb777 commented 2 weeks ago

Also facing this issue. I switched to react-beautiful-dnd and the problem goes away.

marcbejar commented 1 week ago

Using an optimistic update with the new reordered array returned from the following function worked for me.

` const reorder = (list, startIndex, endIndex) => { const result = Array.from(list); const [removed] = result.splice(startIndex, 1); result.splice(endIndex, 0, removed);

return result; }; `

Function found on official example: https://codesandbox.io/p/sandbox/k260nyxq9v?file=%2Findex.js%3A13%2C1-19%2C3