payload
payload copied to clipboard
fix(ui): prevent race conditions after move row by only replacing state for latest change
What?
Race conditions occur when dragging blocks around with the result the wrong data is stored in the block.
To replicate this issue:
- Create a collection with two blocks and auto save on. The blocks should preferably have different field names in groups to make the issue more noticeable. And it seems that the chance of this happening increases when you add a conditional field on a group field (see also the reproducing video & code below)
- Create document with two blocks containing different data and make sure it is saved
- Set throttling to 3g
- Drag the block so that the order is switched. You might need to do this multiple times to catch the race condition.
- Drag again quickly so that the blocks are again in original order. It should be dragged fast, other wise the race conditions doesn't occur.
- When expanding the block, we've seen the wrong contents in the block: block 1 contains the content of block 2 and vice versa.
To illustrate this, please watch the following video: https://github.com/user-attachments/assets/671b0488-16ef-4ab0-b52c-289d6986efe9
At the start of the video, each block the properly field values stored.. After the dragging with throttling, the top two blocks contain no values anymore. And the bottom block contains incorrect values; these incorrect values are actually originally from a different block.
Code used in video (just an empty Payload project withFruits
collection added):
https://github.com/dpnolte/payload-blocks-drag-issue
Why?
This can have serious impact by loosing the canges you've made. We've seen this happening a couple of times in our setup. This happens more often if pages contain more blocks since many blocks will be moved in a longer list and when a block contains additional async fetches to get more data. So it would be really nice to have this fixed.
How?
The underlying cause seems to be that the execute on change processes any handlers as promises. And when the on change handlers are processed, the state is updated again with REPLACE_STATE
to incorporate any form data from the server (e.g., validation). Since the on change handlers are async, other form state change can be processed which updates the form fields in the mean time. It seems that this can happen at least by dispatching multiple MOVE_ROW
in short timespan. The consqeuence is that when changing the order of the blocks with MOVE_ROW
in the race condition time window, the replace state will overwrite the contents of the blocks based on the incorrect positioning of the blocks .
To fix this, this PR checks if the form state has not changed after the async change handlers are processed. If the form state is still the same, it will continue as it does now and dispatch a REPLACE_STATE
. If not, it will early exit and no REPLACE_STATE
will be dispatched to prevent setting a new state that is derived from an outdated source. Since the form fields have been changed, the onchange event will be triggered again so it will dispatch REPLACE_STATE
to merge server form state into it.