Svelte 5: #each randomly don't respect the array order after some mutations

[jamesst20]

Describe the bug

The are multiple random ways to reproduce the error, however, each random patterns are always reproductible.

Easiest pattern: Move Row A to third position then Move Row D to last position.

screen-capture.webm

Reproduction

Svelte 5: CodeSandbox

Svelte 4: CodeSandbox

REPL Simple dependency less (credits to 7nik in the comments below)

System Info

5.0.0-next.144

Severity

blocking all usage of svelte

[trueadm]

Please could you create a repo on the REPL rather than codesandbox?

[Conduitry]

There are a lot of moving parts here, including the third-party library https://www.npmjs.com/package/sortablejs. Please try to provide an actual minimal reproduction.

[trueadm]

I tried to reproduce it without sortablejs, but I didn't have any luck getting the issue to repro.

My hunch is that sortablejs is allowing the physical DOM elements to move position in the DOM? In which case, this will break in Svelte 5 as we use empty text nodes are positional markers around each block entries, and they'll now be in the wrong locations.

[7nik]

Seems reordering HTML elements breaks things, REPL.

It seems to work correctly on [email protected] which is around right before #11107.

[trueadm]

Yeah the refactor to linked lists also introduced the text node markers for each block entries, which is likely the cause of the problem here. We hope to move away from these markers in some cases in the future, mainly because the additional DOM marker nodes actually add quite a lot of overhead to the runtime in some cases (such as deletions, as we're now removing O(n * 2) nodes). So it might be that this resolves this problem too.

However, it's not best practice moving around DOM elements controlled by Svelte. So avoiding it would be ideal too.

[jamesst20]

Yeah the refactor to linked lists also introduced the text node markers for each block entries, which is likely the cause of the problem here. We hope to move away from these markers in some cases in the future, mainly because the additional DOM marker nodes actually add quite a lot of overhead to the runtime in some cases (such as deletions, as we're now removing O(n * 2) nodes). So it might be that this resolves this problem too.

However, it's not best practice moving around DOM elements controlled by Svelte. So avoiding it would be ideal too.

Is it planned to move away from it before the official release? If so, I can just live with the bug for now until Svelte 5 is officially released.

I understand moving around DOM items isn't ideal, however these kind of libs are pretty nice to play with as they can work with many kind of html structure. In my case, I use it a lot with tables and also tables with collapsible rows.

@trueadm Likely not of any use anymore but here is a REPL of morel or less exactly the same sandbox demo. For some reason dragging performance are very bad in there REPL

[trueadm]

I'm not sure if we can make this work in Svelte 5 to be honest. Not without majorly refactoring how each blocks work again. I removed the text anchors in #11836, but that doesn't fix the problem. Svelte 5 expects the DOM elements to be in certain positions and when that assumption isn't true, it breaks the logic. I wonder if you could apply a wrapper around sortable that reverts the DOM elements to their original positions before making the mutation to Svelte?

[7nik]

The lib has a feature request for the flag to prevent DOM changes and even old PR for it.

As a workaround, you can try wrapping the rows with the #key and changing it after each swap so the rows will be rendered from scratch.
Or use another dnd lib.

[dummdidumm]

If another libraries manipulates the DOM then that's outside of what Svelte supports. In this case, as pointed out, the culprit is sortable.js. Therefore closing.

Luckily you can adjust the code so that the manipulation by sortable.js is reverted and so Svelte can take care of it properly. Updated REPL.
The new code is this inside onEnd:

// cancel the UI update so Svelte will take care of it
e.item.remove();
if (e.oldIndex !== undefined) {
	e.from.insertBefore(e.item, e.from.childNodes[(e.oldIndex + 1) * 2 - 1]);
}

[jamesst20]

Thanks to everyone who spent time investigating this issue. It's very appreciated! @7nik @trueadm

@dummdidumm Indeed this fix the issue.

In the mean time I have come up with another solution that is dependency less and that doesn't move DOM item. In can be used with pretty much any element.

REPL

@trueadm I'm suprised #11836 doesn't fix the issue however I still hope you can finish your work as it looks promising performance wise.