diff --git a/.changeset/moody-houses-argue.md b/.changeset/moody-houses-argue.md new file mode 100644 index 0000000000..85f303af54 --- /dev/null +++ b/.changeset/moody-houses-argue.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: prevent reactive statement reruns when they have indirect cyclic dependencies diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index ed7da2333b..3d378df563 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -214,6 +214,10 @@ export function client_component(source, analysis, options) { instance.body.push(statement[1]); } + if (analysis.reactive_statements.size > 0) { + instance.body.push(b.stmt(b.call('$.legacy_pre_effect_reset'))); + } + /** * Used to store the group nodes * @type {import('estree').VariableDeclaration[]} diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-legacy.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-legacy.js index 9ffeb1df8b..ed4c6e8474 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-legacy.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-legacy.js @@ -157,7 +157,6 @@ export const javascript_visitors_legacy = { } const body = serialized_body.body; - const new_body = []; /** @type {import('estree').Expression[]} */ const sequence = []; @@ -176,18 +175,16 @@ export const javascript_visitors_legacy = { sequence.push(serialized); } - if (sequence.length > 0) { - new_body.push(b.stmt(b.sequence(sequence))); - } - - new_body.push(b.stmt(b.call('$.untrack', b.thunk(b.block(body))))); - - serialized_body.body = new_body; - // these statements will be topologically ordered later state.legacy_reactive_statements.set( node, - b.stmt(b.call('$.pre_effect', b.thunk(serialized_body))) + b.stmt( + b.call( + '$.legacy_pre_effect', + sequence.length > 0 ? b.thunk(b.sequence(sequence)) : b.thunk(b.block([])), + b.thunk(b.block(body)) + ) + ) ); return b.empty; diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 8ca43f8414..0c9da24f94 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -5,9 +5,13 @@ import { current_effect, destroy_signal, flush_local_render_effects, - schedule_effect + get, + is_runes, + schedule_effect, + untrack } from '../runtime.js'; import { DIRTY, MANAGED, RENDER_EFFECT, EFFECT, PRE_EFFECT } from '../constants.js'; +import { set } from './sources.js'; /** * @param {import('#client').Reaction} target_signal @@ -156,6 +160,7 @@ export function pre_effect(fn) { ); } const sync = current_effect !== null && (current_effect.f & RENDER_EFFECT) !== 0; + const runes = is_runes(current_component_context); return create_effect( PRE_EFFECT, () => { @@ -169,6 +174,47 @@ export function pre_effect(fn) { ); } +/** + * Internal representation of `$: ..` + * @param {() => any} deps + * @param {() => void | (() => void)} fn + * @returns {import('#client').Effect} + */ +export function legacy_pre_effect(deps, fn) { + const component_context = /** @type {import('#client').ComponentContext} */ ( + current_component_context + ); + const token = {}; + return create_effect( + PRE_EFFECT, + () => { + deps(); + if (component_context.l1.includes(token)) { + return; + } + component_context.l1.push(token); + set(component_context.l2, true); + return untrack(fn); + }, + true, + current_block, + true + ); +} + +export function legacy_pre_effect_reset() { + const component_context = /** @type {import('#client').ComponentContext} */ ( + current_component_context + ); + return render_effect(() => { + const x = get(component_context.l2); + if (x) { + component_context.l1.length = 0; + component_context.l2.v = false; // set directly to avoid rerunning this effect + } + }); +} + /** * This effect is used to ensure binding are kept in sync. We use a pre effect to ensure we run before the * bindings which are in later effects. However, we don't use a pre_effect directly as we don't want to flush anything. diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index abe296907c..cdf5090d26 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -30,7 +30,7 @@ import { } from './constants.js'; import { flush_tasks } from './dom/task.js'; import { add_owner } from './dev/ownership.js'; -import { mutate, set } from './reactivity/sources.js'; +import { mutate, set, source } from './reactivity/sources.js'; const IS_EFFECT = EFFECT | PRE_EFFECT | RENDER_EFFECT; @@ -430,11 +430,7 @@ export function execute_effect(signal) { current_effect = previous_effect; } const component_context = signal.x; - if ( - is_runes(component_context) && // Don't rerun pre effects more than once to accomodate for "$: only runs once" behavior - (signal.f & PRE_EFFECT) !== 0 && - current_queued_pre_and_render_effects.length > 0 - ) { + if ((signal.f & PRE_EFFECT) !== 0 && current_queued_pre_and_render_effects.length > 0) { flush_local_pre_effects(component_context); } } @@ -1163,6 +1159,9 @@ export function push(props, runes = false, fn) { s: props, // runes r: runes, + // legacy $: + l1: [], + l2: source(false), // update_callbacks u: null }; diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index 563835384d..bb0b26209e 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -42,6 +42,10 @@ export type ComponentContext = { c: null | Map; /** runes */ r: boolean; + /** legacy mode: if `$:` statements are allowed to run (ensures they only run once per render) */ + l1: any[]; + /** legacy mode: if `$:` statements are allowed to run (ensures they only run once per render) */ + l2: Source; /** update_callbacks */ u: null | { /** afterUpdate callbacks */ diff --git a/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-in-complex-declaration-with-store-2/_config.js b/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-in-complex-declaration-with-store-2/_config.js index e446ab0345..f073c43f7c 100644 --- a/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-in-complex-declaration-with-store-2/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-in-complex-declaration-with-store-2/_config.js @@ -1,10 +1,12 @@ +import { tick } from 'svelte'; import { test } from '../../test'; // destructure to store value export default test({ html: '

2 2 xxx 5 6 9 10 2

', async test({ assert, target, component }) { - await component.update(); + component.update(); + await tick(); assert.htmlEqual(target.innerHTML, '

11 11 yyy 12 13 14 15 11

'); } }); diff --git a/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-in-complex-declaration-with-store/_config.js b/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-in-complex-declaration-with-store/_config.js index 2570d9b06c..d23ffc4f9d 100644 --- a/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-in-complex-declaration-with-store/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-in-complex-declaration-with-store/_config.js @@ -1,10 +1,12 @@ +import { tick } from 'svelte'; import { test } from '../../test'; // destructure to store export default test({ html: '

2 2 xxx 5 6 9 10 2

', async test({ assert, target, component }) { - await component.update(); + component.update(); + await tick(); assert.htmlEqual(target.innerHTML, '

11 11 yyy 12 13 14 15 11

'); } }); diff --git a/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-prevent-loop/_config.js b/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-prevent-loop/_config.js new file mode 100644 index 0000000000..c6194149f1 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-prevent-loop/_config.js @@ -0,0 +1,13 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: '', + async test({ assert, target }) { + const button = target.querySelector('button'); + button?.click(); + await tick(); + + assert.htmlEqual(target.innerHTML, ''); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-prevent-loop/main.svelte b/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-prevent-loop/main.svelte new file mode 100644 index 0000000000..91aff79c89 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-prevent-loop/main.svelte @@ -0,0 +1,9 @@ + + +