diff --git a/.changeset/flat-cars-say.md b/.changeset/flat-cars-say.md new file mode 100644 index 000000000000..1caa75a1106d --- /dev/null +++ b/.changeset/flat-cars-say.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: correctly reconcile each blocks after outroing branches are resumed diff --git a/.changeset/great-bikes-listen.md b/.changeset/great-bikes-listen.md new file mode 100644 index 000000000000..416e0fcceb08 --- /dev/null +++ b/.changeset/great-bikes-listen.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: destroy each items after siblings are resumed diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 501577053db8..7ae41e9d37cb 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -1,4 +1,4 @@ -/** @import { EachItem, EachState, Effect, MaybeSource, Source, TemplateNode, TransitionManager, Value } from '#client' */ +/** @import { EachItem, EachOutroGroup, EachState, Effect, MaybeSource, Source, TemplateNode, TransitionManager, Value } from '#client' */ /** @import { Batch } from '../../reactivity/batch.js'; */ import { EACH_INDEX_REACTIVE, @@ -29,8 +29,6 @@ import { block, branch, destroy_effect, - run_out_transitions, - pause_children, pause_effect, resume_effect } from '../../reactivity/effects.js'; @@ -75,19 +73,41 @@ function pause_effects(state, to_destroy, controlled_anchor) { var transitions = []; var length = to_destroy.length; + /** @type {EachOutroGroup} */ + var group; + var remaining = to_destroy.length; + for (var i = 0; i < length; i++) { - pause_children(to_destroy[i].e, transitions, true); + pause_effect( + to_destroy[i].e, + () => { + if (group) { + group.remaining -= 1; + + if (group.remaining === 0) { + var groups = /** @type {Set} */ (state.outrogroups); + + destroy_items(state, Array.from(group.items)); + groups.delete(group); + + if (groups.size === 0) { + state.outrogroups = null; + } + } + } else { + remaining -= 1; + } + }, + false + ); } - run_out_transitions(transitions, () => { + if (remaining === 0) { // If we're in a controlled each block (i.e. the block is the only child of an // element), and we are removing all items, _and_ there are no out transitions, // we can use the fast path — emptying the element and replacing the anchor var fast_path = transitions.length === 0 && controlled_anchor !== null; - // TODO only destroy effects if no pending batch needs them. otherwise, - // just set `item.o` back to `false` - if (fast_path) { var anchor = /** @type {Element} */ (controlled_anchor); var parent_node = /** @type {Element} */ (anchor.parentNode); @@ -97,23 +117,41 @@ function pause_effects(state, to_destroy, controlled_anchor) { state.items.clear(); link(state, to_destroy[0].prev, to_destroy[length - 1].next); - } - - for (var i = 0; i < length; i++) { - var item = to_destroy[i]; - if (!fast_path) { - state.items.delete(item.k); - link(state, item.prev, item.next); + for (i = 0; i < length; i++) { + destroy_effect(to_destroy[i].e); } - destroy_effect(item.e, !fast_path); + return; } - if (state.first === to_destroy[0]) { - state.first = to_destroy[0].prev; - } - }); + destroy_items(state, to_destroy); + } else { + group = { + remaining, + items: new Set(to_destroy) + }; + + (state.outrogroups ??= new Set()).add(group); + } +} + +/** + * Pause multiple effects simultaneously, and coordinate their + * subsequent destruction. Used in each blocks + * @param {EachState} state + * @param {EachItem[]} to_destroy + */ +function destroy_items(state, to_destroy) { + // TODO only destroy effects if no pending batch needs them. otherwise, + // just set `item.o` back to `false` + for (var i = 0; i < to_destroy.length; i++) { + var item = to_destroy[i]; + + state.items.delete(item.k); + link(state, item.prev, item.next); + destroy_effect(item.e); + } } /** @@ -335,7 +373,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f }); /** @type {EachState} */ - var state = { effect, flags, items, first }; + var state = { effect, flags, items, first, outrogroups: null }; first_run = false; @@ -409,6 +447,15 @@ function reconcile(state, array, anchor, flags, get_key) { item = /** @type {EachItem} */ (items.get(key)); + if (state.outrogroups !== null) { + for (const group of state.outrogroups) { + if (group.items.has(item)) { + group.remaining -= 1; + group.items.delete(item); + } + } + } + state.first ??= item; if (!item.o) { @@ -486,11 +533,7 @@ function reconcile(state, array, anchor, flags, get_key) { stashed = []; while (current !== null && current.k !== key) { - // If the each block isn't inert and an item has an effect that is already inert, - // skip over adding it to our seen Set as the item is already being handled - if ((current.e.f & INERT) === 0) { - (seen ??= new Set()).add(current); - } + (seen ??= new Set()).add(current); stashed.push(current); current = current.next; } @@ -509,8 +552,29 @@ function reconcile(state, array, anchor, flags, get_key) { let has_offscreen_items = items.size > length; + if (state.outrogroups !== null) { + for (const group of state.outrogroups) { + if (group.remaining === 0) { + destroy_items(state, Array.from(group.items)); + state.outrogroups?.delete(group); + } + } + + if (state.outrogroups.size === 0) { + state.outrogroups = null; + } + } + if (current !== null || seen !== undefined) { - var to_destroy = seen === undefined ? [] : array_from(seen); + var to_destroy = []; + + if (seen !== undefined) { + for (item of seen) { + if ((item.e.f & INERT) === 0) { + to_destroy.push(item); + } + } + } while (current !== null) { // If the each block isn't inert, then inert effects are currently outroing and will be removed once the transition is finished diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 4359378e01c8..5ec81c1db21f 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -593,17 +593,11 @@ export function pause_effect(effect, callback, destroy = true) { pause_children(effect, transitions, true); - run_out_transitions(transitions, () => { + var fn = () => { if (destroy) destroy_effect(effect); if (callback) callback(); - }); -} + }; -/** - * @param {TransitionManager[]} transitions - * @param {() => void} fn - */ -export function run_out_transitions(transitions, fn) { var remaining = transitions.length; if (remaining > 0) { var check = () => --remaining || fn(); @@ -620,7 +614,7 @@ export function run_out_transitions(transitions, fn) { * @param {TransitionManager[]} transitions * @param {boolean} local */ -export function pause_children(effect, transitions, local) { +function pause_children(effect, transitions, local) { if ((effect.f & INERT) !== 0) return; effect.f ^= INERT; diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index 5c682ed14067..458e98b21b8a 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -72,6 +72,11 @@ export type TemplateNode = Text | Element | Comment; export type Dom = TemplateNode | TemplateNode[]; +export type EachOutroGroup = { + remaining: number; + items: Set; +}; + export type EachState = { /** the each block effect */ effect: Effect; @@ -81,6 +86,8 @@ export type EachState = { items: Map; /** head of the linked list of items */ first: EachItem | null; + /** all outro groups that this item is a part of */ + outrogroups: Set | null; }; export type EachItem = { diff --git a/packages/svelte/tests/runtime-runes/samples/each-updates-12/_config.js b/packages/svelte/tests/runtime-runes/samples/each-updates-12/_config.js new file mode 100644 index 000000000000..1fee8ceb6790 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/each-updates-12/_config.js @@ -0,0 +1,33 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, raf }) { + const [clear, push] = target.querySelectorAll('button'); + + flushSync(() => clear.click()); + flushSync(() => push.click()); + raf.tick(500); + + assert.htmlEqual( + target.innerHTML, + ` + + + 1 + 2 + ` + ); + + raf.tick(1000); + + assert.htmlEqual( + target.innerHTML, + ` + + + 1 + ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/each-updates-12/main.svelte b/packages/svelte/tests/runtime-runes/samples/each-updates-12/main.svelte new file mode 100644 index 000000000000..a65ebd37a82b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/each-updates-12/main.svelte @@ -0,0 +1,19 @@ + + + + + +{#each items as item} + {item} +{/each} diff --git a/packages/svelte/tests/runtime-runes/samples/each-updates-13/_config.js b/packages/svelte/tests/runtime-runes/samples/each-updates-13/_config.js new file mode 100644 index 000000000000..fdf02e486cbe --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/each-updates-13/_config.js @@ -0,0 +1,23 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, raf }) { + const [clear, reverse] = target.querySelectorAll('button'); + + flushSync(() => clear.click()); + flushSync(() => reverse.click()); + raf.tick(1); + + assert.htmlEqual( + target.innerHTML, + ` + + + c + b + a + ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/each-updates-13/main.svelte b/packages/svelte/tests/runtime-runes/samples/each-updates-13/main.svelte new file mode 100644 index 000000000000..3de3382419ea --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/each-updates-13/main.svelte @@ -0,0 +1,19 @@ + + + + + +{#each items as item (item)} + {item} +{/each}