diff --git a/.changeset/hot-jobs-tap.md b/.changeset/hot-jobs-tap.md new file mode 100644 index 0000000000..006b6fd106 --- /dev/null +++ b/.changeset/hot-jobs-tap.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: improve order of pre-effect execution diff --git a/packages/svelte/src/internal/client/dom/legacy/lifecycle.js b/packages/svelte/src/internal/client/dom/legacy/lifecycle.js index e186881197..f6c79de380 100644 --- a/packages/svelte/src/internal/client/dom/legacy/lifecycle.js +++ b/packages/svelte/src/internal/client/dom/legacy/lifecycle.js @@ -2,6 +2,7 @@ import { run } from '../../../common.js'; import { user_pre_effect, user_effect } from '../../reactivity/effects.js'; import { current_component_context, + current_effect, deep_read_state, flush_local_render_effects, get, @@ -25,7 +26,10 @@ export function init() { // beforeUpdate might change state that affects rendering, ensure the render effects following from it // are batched up with the current run. Avoids for example child components rerunning when they're // now hidden because beforeUpdate did set an if block to false. - flush_local_render_effects(); + const parent = current_effect?.parent; + if (parent != null) { + flush_local_render_effects(parent); + } }); } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index c5b336ca2d..707f4d9619 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -21,8 +21,7 @@ import { DESTROYED, INERT, MANAGED, - STATE_SYMBOL, - EFFECT_RAN + STATE_SYMBOL } from './constants.js'; import { flush_tasks } from './dom/task.js'; import { add_owner } from './dev/ownership.js'; @@ -403,9 +402,10 @@ export function execute_effect(effect) { current_effect = previous_effect; current_component_context = previous_component_context; } + const parent = effect.parent; - if ((effect.f & PRE_EFFECT) !== 0 && current_queued_pre_and_render_effects.length > 0) { - flush_local_pre_effects(component_context); + if ((effect.f & PRE_EFFECT) !== 0 && parent !== null) { + flush_local_pre_effects(parent); } } @@ -540,36 +540,86 @@ export function schedule_effect(signal) { } /** + * + * This function recursively collects effects in topological order from the starting effect passed in. + * Effects will be collected when they match the filtered bitwise flag passed in only. The collected + * array will be populated with all the effects. + * + * @param {import('./types.js').Effect} effect + * @param {number} filter_flags + * @param {import('./types.js').Effect[]} collected * @returns {void} */ -export function flush_local_render_effects() { - const effects = []; - for (let i = 0; i < current_queued_pre_and_render_effects.length; i++) { - const effect = current_queued_pre_and_render_effects[i]; - if ((effect.f & RENDER_EFFECT) !== 0 && effect.ctx === current_component_context) { - effects.push(effect); - current_queued_pre_and_render_effects.splice(i, 1); - i--; +function collect_effects(effect, filter_flags, collected) { + var effects = effect.effects; + if (effects === null) { + return; + } + var i, s, child, flags; + var render = []; + var user = []; + + for (i = 0; i < effects.length; i++) { + child = effects[i]; + flags = child.f; + if ((flags & CLEAN) !== 0) { + continue; + } + + if ((flags & PRE_EFFECT) !== 0) { + if ((filter_flags & PRE_EFFECT) !== 0) { + collected.push(child); + } + collect_effects(child, filter_flags, collected); + } else if ((flags & RENDER_EFFECT) !== 0) { + render.push(child); + } else if ((flags & EFFECT) !== 0) { + user.push(child); + } + } + + if (render.length > 0) { + if ((filter_flags & RENDER_EFFECT) !== 0) { + collected.push(...render); + } + for (s = 0; s < render.length; s++) { + collect_effects(render[s], filter_flags, collected); + } + } + if (user.length > 0) { + if ((filter_flags & EFFECT) !== 0) { + collected.push(...user); + } + for (s = 0; s < user.length; s++) { + collect_effects(user[s], filter_flags, collected); } } - flush_queued_effects(effects); } /** - * @param {null | import('./types.js').ComponentContext} context + * @param {import('./types.js').Effect} effect * @returns {void} */ -export function flush_local_pre_effects(context) { - const effects = []; - for (let i = 0; i < current_queued_pre_and_render_effects.length; i++) { - const effect = current_queued_pre_and_render_effects[i]; - if ((effect.f & PRE_EFFECT) !== 0 && effect.ctx === context) { - effects.push(effect); - current_queued_pre_and_render_effects.splice(i, 1); - i--; - } - } - flush_queued_effects(effects); +export function flush_local_render_effects(effect) { + /** + * @type {import("./types.js").Effect[]} + */ + var render_effects = []; + collect_effects(effect, RENDER_EFFECT, render_effects); + flush_queued_effects(render_effects); +} + +/** + * @param {import('./types.js').Effect} effect + * @returns {void} + */ +export function flush_local_pre_effects(effect) { + /** + * @type {import("./types.js").Effect[]} + */ + var pre_effects = []; + collect_effects(effect, PRE_EFFECT, pre_effects); + flush_queued_effects(pre_effects); } /** diff --git a/packages/svelte/src/reactivity/map.test.ts b/packages/svelte/src/reactivity/map.test.ts index fbbb11e340..979e8ded05 100644 --- a/packages/svelte/src/reactivity/map.test.ts +++ b/packages/svelte/src/reactivity/map.test.ts @@ -36,7 +36,7 @@ test('map.values()', () => { map.clear(); }); - assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, [], false]); // TODO update when we fix effect ordering bug + assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, false, []]); cleanup(); }); diff --git a/packages/svelte/src/reactivity/set.test.ts b/packages/svelte/src/reactivity/set.test.ts index aff076fc39..e93ac4e565 100644 --- a/packages/svelte/src/reactivity/set.test.ts +++ b/packages/svelte/src/reactivity/set.test.ts @@ -30,7 +30,7 @@ test('set.values()', () => { set.clear(); }); - assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, [], false]); // TODO update when we fix effect ordering bug + assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, false, []]); cleanup(); });