Transition mixes up element order when multiple items are removed simultaneously

[mart-jansink]

When you remove multiple items simultaneously, the reordering logic at https://github.com/react-spring/react-spring/blob/master/src/Transition.js#L163 fails to get it correct. This results in React creating new elements which in turn resets e.g. the scroll offset of a div (which was the symptom that lead me to this issue).

I've reproduced it at https://codesandbox.io/s/10ol3v9oj, but you'll need to add a breakpoint at line 2348 of https://10ol3v9oj.codesandbox.io/node_modules/react-spring/dist/web.cjs.js like it is shown below:

At the step that both item 1 and 2 are removed at once the following happens:

• first item 1 is removed and deleted correctly, after which item 0 and item 2 are still rendered in the correct order
• item 2 is removed but during its removal first item 2 and then item 0 are rendered, as you can see in the elements tab of the console and as indicated by the red 0

As far as I can tell this is because the onRest callback is first called for item 1, which results in a rerender before the onRest callback is called for item 2. This then results in another rerender but the reordering logic can't find the correct position for item 2 because item 1 is already gone.

Ideally, both items would be deleted at once, preventing the extra rerender all together. Somehow the order in which the onRest callbacks fire would also solve the issue, but I couldn't find a good way to do so in the code. For me, the actual order of the rendered elements doesn't really matter (they're absolutely positioned) as long as it doesn't change, so I could solve it by calling .reverse() on the array of items before passing them as a prop.

[drcmda]

The demo uses an old version of react-spring, is it the same with the lastest version as well (7.x)?

[mart-jansink]

Yes it is. My apologies, I forked this sandbox from one of the demos. It's now updated and I still see the issue when looking at the elements tab of the console:

[aocenas]

Have the same issue. When 2 or more items which are next to each other are removed this code https://github.com/react-spring/react-spring/blob/master/src/useTransition.js#L252 will put all but one of them at the beginning of the array because left item is already deleted.

Wouldn't it make sense to just order all the changes based on currentKeys? Something like currentKeys.map(key => mapOfAllTransitions[key]) instead of splitting it to deleted array and then trying to recreate the ordering back back?

[aocenas]

Seems the issue is how deleted items are unshifted instead of pushed, which then breaks the ordering logic. Created a PR for that.

[drcmda]

if we can establish that everything runs fine after this i'll make a patch tomorrow. thanks!

[aleclarson]

You can try this with v9 now: yarn add react-spring@next

Let us know how it goes: #642