diff --git a/.changeset/upset-eyes-relax.md b/.changeset/upset-eyes-relax.md new file mode 100644 index 000000000000..b5c9b4e831e8 --- /dev/null +++ b/.changeset/upset-eyes-relax.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: maintain correct linked list of effects when updating each blocks diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 0eb8f889c3ab..320c0346902c 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -129,8 +129,11 @@ function pause_effects(state, to_destroy, controlled_anchor) { export function each(node, flags, get_collection, get_key, render_fn, fallback_fn = null) { var anchor = node; - /** @type {EachState} */ - var state = { flags, items: new Map(), first: null }; + /** @type {Map} */ + var items = new Map(); + + /** @type {EachItem | null} */ + var first = null; var is_controlled = (flags & EACH_IS_CONTROLLED) !== 0; var is_reactive_value = (flags & EACH_ITEM_REACTIVE) !== 0; @@ -166,7 +169,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f var first_run = true; function commit() { - reconcile(each_effect, array, state, anchor, flags, get_key); + reconcile(state, array, anchor, flags, get_key); if (fallback !== null) { if (array.length === 0) { @@ -177,7 +180,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f resume_effect(fallback.effect); } - each_effect.first = fallback.effect; + effect.first = fallback.effect; } else { pause_effect(fallback.effect, () => { // TODO only null out if no pending batch needs it, @@ -189,7 +192,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f } } - var each_effect = block(() => { + var effect = block(() => { array = /** @type {V[]} */ (get(each_array)); var length = array.length; @@ -230,7 +233,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f var value = array[i]; var key = get_key(value, i); - var item = first_run ? null : state.items.get(key); + var item = first_run ? null : items.get(key); if (item) { // update before reconciliation, to trigger any async updates @@ -263,7 +266,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f item.o = true; if (prev === null) { - state.first = item; + first = item; } else { prev.next = item; } @@ -271,7 +274,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f prev = item; } - state.items.set(key, item); + items.set(key, item); } keys.add(key); @@ -302,7 +305,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f if (!first_run) { if (defer) { - for (const [key, item] of state.items) { + for (const [key, item] of items) { if (!keys.has(key)) { batch.skipped_effects.add(item.e); } @@ -331,6 +334,9 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f get(each_array); }); + /** @type {EachState} */ + var state = { effect, flags, items, first }; + first_run = false; if (hydrating) { @@ -341,15 +347,14 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f /** * Add, remove, or reorder items output by an each block as its input changes * @template V - * @param {Effect} each_effect - * @param {Array} array * @param {EachState} state + * @param {Array} array * @param {Element | Comment | Text} anchor * @param {number} flags * @param {(value: V, index: number) => any} get_key * @returns {void} */ -function reconcile(each_effect, array, state, anchor, flags, get_key) { +function reconcile(state, array, anchor, flags, get_key) { var is_animated = (flags & EACH_IS_ANIMATED) !== 0; var length = array.length; @@ -536,16 +541,6 @@ function reconcile(each_effect, array, state, anchor, flags, get_key) { } }); } - - // TODO i have an inkling that the rest of this function is wrong... - // the offscreen items need to be linked, so that they all update correctly. - // the last onscreen item should link to the first offscreen item, etc - each_effect.first = state.first && state.first.e; - each_effect.last = prev && prev.e; - - if (prev) { - prev.e.next = null; - } } /** @@ -601,11 +596,11 @@ function create_item(anchor, prev, value, key, index, render_fn, flags, get_coll item.e = branch(() => render_fn(/** @type {Node} */ (anchor), v, i, get_collection)); - item.e.prev = prev && prev.e; - if (prev !== null) { + // we only need to set `prev.next = item`, because + // `item.prev = prev` was set on initialization. + // the effects themselves are already linked prev.next = item; - prev.e.next = item.e; } return item; @@ -640,12 +635,23 @@ function move(item, next, anchor) { function link(state, prev, next) { if (prev === null) { state.first = next; + state.effect.first = next && next.e; } else { + if (prev.e.next) { + prev.e.next.prev = null; + } + prev.next = next; prev.e.next = next && next.e; } - if (next !== null) { + if (next === null) { + state.effect.last = prev && prev.e; + } else { + if (next.e.prev) { + next.e.prev.next = null; + } + next.prev = prev; next.e.prev = prev && prev.e; } diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index 19645fcc329f..c1003ecc1a45 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -64,6 +64,8 @@ export type TemplateNode = Text | Element | Comment; export type Dom = TemplateNode | TemplateNode[]; export type EachState = { + /** the each block effect */ + effect: Effect; /** flags */ flags: number; /** a key -> item lookup */ diff --git a/packages/svelte/tests/runtime-runes/samples/each-effect-linking/_config.js b/packages/svelte/tests/runtime-runes/samples/each-effect-linking/_config.js new file mode 100644 index 000000000000..d1c23527a9e0 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/each-effect-linking/_config.js @@ -0,0 +1,32 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + const [step_back, step_forward, jump_back, jump_forward] = target.querySelectorAll('button'); + const [div] = target.querySelectorAll('div'); + + step_back.click(); + await tick(); + + step_forward.click(); + await tick(); + + step_forward.click(); + await tick(); + + // if the effects get linked in a circle, we will never get here + assert.htmlEqual(div.innerHTML, '

5

6

7

'); + + jump_forward.click(); + await tick(); + + step_forward.click(); + await tick(); + + step_forward.click(); + await tick(); + + assert.htmlEqual(div.innerHTML, '

12

13

14

'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/each-effect-linking/main.svelte b/packages/svelte/tests/runtime-runes/samples/each-effect-linking/main.svelte new file mode 100644 index 000000000000..ea33b11e222f --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/each-effect-linking/main.svelte @@ -0,0 +1,33 @@ + + + + + + + + + + +
+ {#each items as item (item)} +

{item}

+ {/each} +