diff --git a/.changeset/brave-walls-flow.md b/.changeset/brave-walls-flow.md new file mode 100644 index 0000000000..c00bdf37f9 --- /dev/null +++ b/.changeset/brave-walls-flow.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: improved effect sequencing and execution order diff --git a/.changeset/cool-peas-lick.md b/.changeset/cool-peas-lick.md new file mode 100644 index 0000000000..4b3e562c46 --- /dev/null +++ b/.changeset/cool-peas-lick.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +breaking: onDestroy functions run child-first diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index e8246db02e..d185a9c21a 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -608,20 +608,38 @@ const legacy_scope_tweaker = { /** @type {import('../types.js').ReactiveStatement} */ const reactive_statement = { assignments: new Set(), - dependencies: new Set() + dependencies: [] }; next({ ...state, reactive_statement, function_depth: state.scope.function_depth + 1 }); + // Every referenced binding becomes a dependency, unless it's on + // the left-hand side of an `=` assignment for (const [name, nodes] of state.scope.references) { const binding = state.scope.get(name); if (binding === null) continue; - // Include bindings that have references other than assignments and their own declarations - if ( - nodes.some((n) => n.node !== binding.node && !reactive_statement.assignments.has(n.node)) - ) { - reactive_statement.dependencies.add(binding); + for (const { node, path } of nodes) { + /** @type {import('estree').Expression} */ + let left = node; + + let i = path.length - 1; + let parent = /** @type {import('estree').Expression} */ (path.at(i)); + while (parent.type === 'MemberExpression') { + left = parent; + parent = /** @type {import('estree').Expression} */ (path.at(--i)); + } + + if ( + parent.type === 'AssignmentExpression' && + parent.operator === '=' && + parent.left === left + ) { + continue; + } + + reactive_statement.dependencies.push(binding); + break; } } @@ -630,8 +648,8 @@ const legacy_scope_tweaker = { // Ideally this would be in the validation file, but that isn't possible because this visitor // calls "next" before setting the reactive statements. if ( - reactive_statement.dependencies.size && - [...reactive_statement.dependencies].every( + reactive_statement.dependencies.length && + reactive_statement.dependencies.every( (d) => d.scope === state.analysis.module.scope && d.declaration_kind !== 'const' ) ) { @@ -660,15 +678,29 @@ const legacy_scope_tweaker = { } }, AssignmentExpression(node, { state, next }) { - if (state.reactive_statement && node.operator === '=') { - if (node.left.type === 'MemberExpression') { - const id = object(node.left); - if (id !== null) { - state.reactive_statement.assignments.add(id); - } - } else { + if (state.reactive_statement) { + const id = node.left.type === 'MemberExpression' ? object(node.left) : node.left; + if (id !== null) { for (const id of extract_identifiers(node.left)) { - state.reactive_statement.assignments.add(id); + const binding = state.scope.get(id.name); + + if (binding) { + state.reactive_statement.assignments.add(binding); + } + } + } + } + + next(); + }, + UpdateExpression(node, { state, next }) { + if (state.reactive_statement) { + const id = node.argument.type === 'MemberExpression' ? object(node.argument) : node.argument; + if (id?.type === 'Identifier') { + const binding = state.scope.get(id.name); + + if (binding) { + state.reactive_statement.assignments.add(binding); } } } @@ -1343,21 +1375,21 @@ function order_reactive_statements(unsorted_reactive_declarations) { const lookup = new Map(); for (const [node, declaration] of unsorted_reactive_declarations) { - declaration.assignments.forEach(({ name }) => { - const statements = lookup.get(name) ?? []; + for (const binding of declaration.assignments) { + const statements = lookup.get(binding.node.name) ?? []; statements.push([node, declaration]); - lookup.set(name, statements); - }); + lookup.set(binding.node.name, statements); + } } /** @type {Array<[string, string]>} */ const edges = []; for (const [, { assignments, dependencies }] of unsorted_reactive_declarations) { - for (const { name } of assignments) { - for (const { node } of dependencies) { - if (![...assignments].find(({ name }) => node.name === name)) { - edges.push([name, node.name]); + for (const assignment of assignments) { + for (const dependency of dependencies) { + if (!assignments.has(dependency)) { + edges.push([assignment.node.name, dependency.node.name]); } } } @@ -1381,14 +1413,17 @@ function order_reactive_statements(unsorted_reactive_declarations) { */ const add_declaration = (node, declaration) => { if ([...reactive_declarations.values()].includes(declaration)) return; - declaration.dependencies.forEach(({ node: { name } }) => { - if ([...declaration.assignments].some((a) => a.name === name)) return; - for (const [node, earlier] of lookup.get(name) ?? []) { + + for (const binding of declaration.dependencies) { + if (declaration.assignments.has(binding)) continue; + for (const [node, earlier] of lookup.get(binding.node.name) ?? []) { add_declaration(node, earlier); } - }); + } + reactive_declarations.set(node, declaration); }; + for (const [node, declaration] of unsorted_reactive_declarations) { add_declaration(node, declaration); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index e54a2c14d2..1d8b7acb71 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -233,7 +233,7 @@ function setup_select_synchronization(value_binding, context) { context.state.init.push( b.stmt( b.call( - '$.pre_effect', + '$.render_effect', b.thunk( b.block([ b.stmt( diff --git a/packages/svelte/src/compiler/phases/types.d.ts b/packages/svelte/src/compiler/phases/types.d.ts index df880bf783..2c663b1704 100644 --- a/packages/svelte/src/compiler/phases/types.d.ts +++ b/packages/svelte/src/compiler/phases/types.d.ts @@ -23,8 +23,8 @@ export interface Template { } export interface ReactiveStatement { - assignments: Set; - dependencies: Set; + assignments: Set; + dependencies: Binding[]; } export interface RawWarning { diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index 12983dab32..77b5f399d5 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -1,9 +1,9 @@ export const DERIVED = 1 << 1; export const EFFECT = 1 << 2; -export const PRE_EFFECT = 1 << 3; -export const RENDER_EFFECT = 1 << 4; -export const BLOCK_EFFECT = 1 << 5; -export const BRANCH_EFFECT = 1 << 6; +export const RENDER_EFFECT = 1 << 3; +export const BLOCK_EFFECT = 1 << 4; +export const BRANCH_EFFECT = 1 << 5; +export const ROOT_EFFECT = 1 << 6; export const UNOWNED = 1 << 7; export const CLEAN = 1 << 8; export const DIRTY = 1 << 9; @@ -12,7 +12,6 @@ export const INERT = 1 << 11; export const DESTROYED = 1 << 12; export const IS_ELSEIF = 1 << 13; export const EFFECT_RAN = 1 << 14; -export const ROOT_EFFECT = 1 << 15; export const UNINITIALIZED = Symbol(); export const STATE_SYMBOL = Symbol('$state'); diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/this.js b/packages/svelte/src/internal/client/dom/elements/bindings/this.js index dfc0261e19..940ac9f62f 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/this.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/this.js @@ -47,8 +47,6 @@ export function bind_this(element_or_component, update, get_value, get_parts) { }); return () => { - // Defer to the next tick so that all updates can be reconciled first. - // This solves the case where one variable is shared across multiple this-bindings. effect(() => { if (parts && is_bound_this(get_value(...parts), element_or_component)) { update(null, ...parts); diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 7a9259698e..0cd66c8cb7 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -19,7 +19,6 @@ import { BRANCH_EFFECT, RENDER_EFFECT, EFFECT, - PRE_EFFECT, DESTROYED, INERT, IS_ELSEIF, @@ -32,13 +31,12 @@ import { noop } from '../../common.js'; import { remove } from '../dom/reconciler.js'; /** - * @param {import('./types.js').EffectType} type + * @param {number} type * @param {(() => void | (() => void))} fn * @param {boolean} sync - * @param {boolean} init * @returns {import('#client').Effect} */ -function create_effect(type, fn, sync, init = true) { +function create_effect(type, fn, sync) { var is_root = (type & ROOT_EFFECT) !== 0; /** @type {import('#client').Effect} */ var effect = { @@ -46,7 +44,6 @@ function create_effect(type, fn, sync, init = true) { dom: null, deps: null, f: type | DIRTY, - l: 0, fn, effects: null, deriveds: null, @@ -55,10 +52,6 @@ function create_effect(type, fn, sync, init = true) { transitions: null }; - if (current_effect !== null) { - effect.l = current_effect.l + 1; - } - if (current_reaction !== null && !is_root) { if (current_reaction.effects === null) { current_reaction.effects = [effect]; @@ -67,20 +60,18 @@ function create_effect(type, fn, sync, init = true) { } } - if (init) { - if (sync) { - var previously_flushing_effect = is_flushing_effect; + if (sync) { + var previously_flushing_effect = is_flushing_effect; - try { - set_is_flushing_effect(true); - execute_effect(effect); - effect.f |= EFFECT_RAN; - } finally { - set_is_flushing_effect(previously_flushing_effect); - } - } else { - schedule_effect(effect); + try { + set_is_flushing_effect(true); + execute_effect(effect); + effect.f |= EFFECT_RAN; + } finally { + set_is_flushing_effect(previously_flushing_effect); } + } else { + schedule_effect(effect); } return effect; @@ -97,7 +88,6 @@ export function effect_active() { /** * Internal representation of `$effect(...)` * @param {() => void | (() => void)} fn - * @returns {import('#client').Effect} */ export function user_effect(fn) { if (current_effect === null) { @@ -115,14 +105,12 @@ export function user_effect(fn) { current_component_context !== null && !current_component_context.m; - const effect = create_effect(EFFECT, fn, false, !defer); - if (defer) { const context = /** @type {import('#client').ComponentContext} */ (current_component_context); - (context.e ??= []).push(effect); + (context.e ??= []).push(fn); + } else { + effect(fn); } - - return effect; } /** @@ -140,7 +128,7 @@ export function user_pre_effect(fn) { ); } - return pre_effect(fn); + return render_effect(fn); } /** @@ -149,6 +137,8 @@ export function user_pre_effect(fn) { * @returns {() => void} */ export function effect_root(fn) { + // TODO is `untrack` correct here? Should `fn` re-run if its dependencies change? + // Should it even be modelled as an effect? const effect = create_effect(ROOT_EFFECT, () => untrack(fn), true); return () => { destroy_effect(effect); @@ -167,34 +157,45 @@ export function 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 pre_effect(() => { + var context = /** @type {import('#client').ComponentContext} */ (current_component_context); + + /** @type {{ effect: null | import('#client').Effect, ran: boolean }} */ + var token = { effect: null, ran: false }; + context.l1.push(token); + + token.effect = render_effect(() => { deps(); - if (component_context.l1.includes(token)) { - return; - } - component_context.l1.push(token); - set(component_context.l2, true); - return untrack(fn); + + // If this legacy pre effect has already run before the end of the reset, then + // bail-out to emulate the same behavior. + if (token.ran) return; + + token.ran = true; + set(context.l2, true); + untrack(fn); }); } 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 + var context = /** @type {import('#client').ComponentContext} */ (current_component_context); + + render_effect(() => { + if (!get(context.l2)) return; + + // Run dirty `$:` statements + for (var token of context.l1) { + var effect = token.effect; + + if (check_dirtiness(effect)) { + execute_effect(effect); + } + + token.ran = false; } + + context.l2.v = false; // set directly to avoid rerunning this effect }); } @@ -202,14 +203,6 @@ export function legacy_pre_effect_reset() { * @param {() => void | (() => void)} fn * @returns {import('#client').Effect} */ -export function pre_effect(fn) { - return create_effect(PRE_EFFECT, fn, true); -} - -/** - * @param {(() => void)} fn - * @returns {import('#client').Effect} - */ export function render_effect(fn) { return create_effect(RENDER_EFFECT, fn, true); } diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index 2b2563dbf3..a4afb6c468 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -1,7 +1,4 @@ -import type { Block, ComponentContext, Dom, Equals, TransitionManager } from '#client'; -import type { EFFECT, PRE_EFFECT, RENDER_EFFECT } from '../constants'; - -export type EffectType = typeof EFFECT | typeof PRE_EFFECT | typeof RENDER_EFFECT; +import type { ComponentContext, Dom, Equals, TransitionManager } from '#client'; export interface Signal { /** Flags bitmask */ @@ -44,8 +41,6 @@ export interface Effect extends Reaction { fn: () => void | (() => void); /** The teardown function returned from the effect function */ teardown: null | (() => void); - /** The depth from the root signal, used for ordering render/pre-effects topologically **/ - l: number; /** Transition managers created with `$.transition` */ transitions: null | TransitionManager[]; } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index e600295d6f..c1a61e0a75 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -8,10 +8,9 @@ import { object_prototype } from './utils.js'; import { unstate } from './proxy.js'; -import { destroy_effect, user_pre_effect } from './reactivity/effects.js'; +import { destroy_effect, effect, user_pre_effect } from './reactivity/effects.js'; import { EFFECT, - PRE_EFFECT, RENDER_EFFECT, DIRTY, MAYBE_DIRTY, @@ -22,8 +21,7 @@ import { INERT, BRANCH_EFFECT, STATE_SYMBOL, - BLOCK_EFFECT, - ROOT_EFFECT + BLOCK_EFFECT } from './constants.js'; import { flush_tasks } from './dom/task.js'; import { add_owner } from './dev/ownership.js'; @@ -51,10 +49,7 @@ let is_inspecting_signal = false; // Handle effect queues /** @type {import('./types.js').Effect[]} */ -let current_queued_pre_and_render_effects = []; - -/** @type {import('./types.js').Effect[]} */ -let current_queued_effects = []; +let current_queued_root_effects = []; let flush_count = 0; // Handle signal reactivity tree dependencies and reactions @@ -406,15 +401,10 @@ export function execute_effect(effect) { current_effect = previous_effect; current_component_context = previous_component_context; } - const parent = effect.parent; - - if ((flags & PRE_EFFECT) !== 0 && parent !== null) { - flush_local_pre_effects(parent); - } } function infinite_loop_guard() { - if (flush_count > 100) { + if (flush_count > 1000) { flush_count = 0; throw new Error( 'ERR_SVELTE_TOO_MANY_UPDATES' + @@ -427,6 +417,18 @@ function infinite_loop_guard() { flush_count++; } +/** + * @param {Array} root_effects + * @returns {void} + */ +function flush_queued_root_effects(root_effects) { + for (var i = 0; i < root_effects.length; i++) { + var signal = root_effects[i]; + var effects = get_nested_effects(signal, RENDER_EFFECT | EFFECT); + flush_queued_effects(effects); + } +} + /** * @param {Array} effects * @returns {void} @@ -461,12 +463,9 @@ function process_microtask() { if (flush_count > 101) { return; } - const previous_queued_pre_and_render_effects = current_queued_pre_and_render_effects; - const previous_queued_effects = current_queued_effects; - current_queued_pre_and_render_effects = []; - current_queued_effects = []; - flush_queued_effects(previous_queued_pre_and_render_effects); - flush_queued_effects(previous_queued_effects); + const previous_queued_root_effects = current_queued_root_effects; + current_queued_root_effects = []; + flush_queued_root_effects(previous_queued_root_effects); if (!is_micro_task_queued) { flush_count = 0; } @@ -477,8 +476,6 @@ function process_microtask() { * @returns {void} */ export function schedule_effect(signal) { - const flags = signal.f; - if (current_scheduler_mode === FLUSH_MICROTASK) { if (!is_micro_task_queued) { is_micro_task_queued = true; @@ -486,61 +483,19 @@ export function schedule_effect(signal) { } } - if ((flags & EFFECT) !== 0) { - current_queued_effects.push(signal); - // Prevent any nested user effects from potentially triggering - // before this effect is scheduled. We know they will be destroyed - // so we can make them inert to avoid having to find them in the - // queue and remove them. - if ((flags & BRANCH_EFFECT) === 0) { - mark_subtree_children_inert(signal, true); - } - } else { - // We need to ensure we insert the signal in the right topological order. In other words, - // we need to evaluate where to insert the signal based off its level and whether or not it's - // a pre-effect and within the same block. By checking the signals in the queue in reverse order - // we can find the right place quickly. TODO: maybe opt to use a linked list rather than an array - // for these operations. - const length = current_queued_pre_and_render_effects.length; - let should_append = length === 0; - - if (!should_append) { - const target_level = signal.l; - const is_pre_effect = (flags & PRE_EFFECT) !== 0; - let target_signal; - let target_signal_level; - let is_target_pre_effect; - let i = length; - while (true) { - target_signal = current_queued_pre_and_render_effects[--i]; - target_signal_level = target_signal.l; - if (target_signal_level <= target_level) { - if (i + 1 === length) { - should_append = true; - } else { - is_target_pre_effect = (target_signal.f & PRE_EFFECT) !== 0; - if ( - target_signal_level < target_level || - target_signal !== signal || - (is_target_pre_effect && !is_pre_effect) - ) { - i++; - } - current_queued_pre_and_render_effects.splice(i, 0, signal); - } - break; - } - if (i === 0) { - current_queued_pre_and_render_effects.unshift(signal); - break; - } - } - } + var effect = signal; - if (should_append) { - current_queued_pre_and_render_effects.push(signal); + while (effect.parent !== null) { + effect = effect.parent; + var flags = effect.f; + + if ((flags & BRANCH_EFFECT) !== 0) { + if ((flags & CLEAN) === 0) return; + set_signal_status(effect, MAYBE_DIRTY); } } + + current_queued_root_effects.push(effect); } /** @@ -549,81 +504,128 @@ export function schedule_effect(signal) { * Effects will be collected when they match the filtered bitwise flag passed in only. The collected * array will be populated with all the effects. * + * In an ideal world, we could just execute effects as we encounter them using this approach. However, + * this isn't possible due to how effects in Svelte are modelled to be possibly side-effectful. Thus, + * executing an effect might invalidate other parts of the tree, which means this this tree walking function + * will possibly pick up effects that are dirty too soon. + * * @param {import('./types.js').Effect} effect * @param {number} filter_flags - * @param {import('./types.js').Effect[]} collected + * @param {boolean} shallow + * @param {import('./types.js').Effect[]} collected_render + * @param {import('./types.js').Effect[]} collected_user * @returns {void} */ -function collect_effects(effect, filter_flags, collected) { +function recursively_collect_effects( + effect, + filter_flags, + shallow, + collected_render, + collected_user +) { var effects = effect.effects; - if (effects === null) { - return; - } - var i, s, child, flags; + if (effects === null) return; + var render = []; var user = []; - for (i = 0; i < effects.length; i++) { - child = effects[i]; - flags = child.f; - if ((flags & CLEAN) !== 0) { - continue; + for (var i = 0; i < effects.length; i++) { + var child = effects[i]; + var flags = child.f; + var is_branch = flags & BRANCH_EFFECT; + + if (is_branch) { + // Skip this branch if it's clean + if ((flags & CLEAN) !== 0) continue; + set_signal_status(child, CLEAN); } - if ((flags & PRE_EFFECT) !== 0) { - if ((filter_flags & PRE_EFFECT) !== 0) { - collected.push(child); + if ((flags & RENDER_EFFECT) !== 0) { + if (is_branch) { + if (shallow) continue; + recursively_collect_effects(child, filter_flags, false, collected_render, collected_user); + } else { + render.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 (is_branch) { + if (shallow) continue; + recursively_collect_effects(child, filter_flags, false, collected_render, collected_user); + } else { + user.push(child); + } } } if (render.length > 0) { if ((filter_flags & RENDER_EFFECT) !== 0) { - collected.push(...render); + collected_render.push(...render); } - for (s = 0; s < render.length; s++) { - collect_effects(render[s], filter_flags, collected); + + if (!shallow) { + for (i = 0; i < render.length; i++) { + recursively_collect_effects( + render[i], + filter_flags, + false, + collected_render, + collected_user + ); + } } } + if (user.length > 0) { if ((filter_flags & EFFECT) !== 0) { - collected.push(...user); + collected_user.push(...user); } - for (s = 0; s < user.length; s++) { - collect_effects(user[s], filter_flags, collected); + + if (!shallow) { + for (i = 0; i < user.length; i++) { + recursively_collect_effects(user[i], filter_flags, false, collected_render, collected_user); + } } } } /** + * + * 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 - * @returns {void} + * @param {number} filter_flags + * @param {boolean} [shallow] + * @returns {import('./types.js').Effect[]} */ -export function flush_local_render_effects(effect) { +function get_nested_effects(effect, filter_flags, shallow = false) { /** * @type {import("./types.js").Effect[]} */ var render_effects = []; - collect_effects(effect, RENDER_EFFECT, render_effects); - flush_queued_effects(render_effects); + /** + * @type {import("./types.js").Effect[]} + */ + var user_effects = []; + // When working with custom-elements, the root effects might not have a root + if (effect.effects === null && (effect.f & BRANCH_EFFECT) === 0) { + return [effect]; + } + recursively_collect_effects(effect, filter_flags, shallow, render_effects, user_effects); + return [...render_effects, ...user_effects]; } /** * @param {import('./types.js').Effect} effect * @returns {void} */ -export function flush_local_pre_effects(effect) { +export function flush_local_render_effects(effect) { /** * @type {import("./types.js").Effect[]} */ - var pre_effects = []; - collect_effects(effect, PRE_EFFECT, pre_effects); - flush_queued_effects(pre_effects); + var render_effects = get_nested_effects(effect, RENDER_EFFECT, true); + flush_queued_effects(render_effects); } /** @@ -643,40 +645,36 @@ export function flushSync(fn) { * @returns {any} */ export function flush_sync(fn, flush_previous = true) { - const previous_scheduler_mode = current_scheduler_mode; - const previous_queued_pre_and_render_effects = current_queued_pre_and_render_effects; - const previous_queued_effects = current_queued_effects; - let result; + var previous_scheduler_mode = current_scheduler_mode; + var previous_queued_root_effects = current_queued_root_effects; try { infinite_loop_guard(); - /** @type {import('./types.js').Effect[]} */ - const pre_and_render_effects = []; /** @type {import('./types.js').Effect[]} */ - const effects = []; + const root_effects = []; + current_scheduler_mode = FLUSH_SYNC; - current_queued_pre_and_render_effects = pre_and_render_effects; - current_queued_effects = effects; + current_queued_root_effects = root_effects; + if (flush_previous) { - flush_queued_effects(previous_queued_pre_and_render_effects); - flush_queued_effects(previous_queued_effects); - } - if (fn !== undefined) { - result = fn(); + flush_queued_root_effects(previous_queued_root_effects); } - if (current_queued_pre_and_render_effects.length > 0 || effects.length > 0) { - flushSync(); + + var result = fn?.(); + + if (current_queued_root_effects.length > 0 || root_effects.length > 0) { + flush_sync(); } + flush_tasks(); flush_count = 0; + + return result; } finally { current_scheduler_mode = previous_scheduler_mode; - current_queued_pre_and_render_effects = previous_queued_pre_and_render_effects; - current_queued_effects = previous_queued_effects; + current_queued_root_effects = previous_queued_root_effects; } - - return result; } /** @@ -795,40 +793,6 @@ export function invalidate_inner_signals(fn) { } } -/** - * @param {import('#client').Effect} signal - * @param {boolean} inert - * @returns {void} - */ -function mark_subtree_children_inert(signal, inert) { - const effects = signal.effects; - - if (effects !== null) { - for (var i = 0; i < effects.length; i++) { - mark_subtree_inert(effects[i], inert); - } - } -} - -/** - * @param {import('#client').Effect} signal - * @param {boolean} inert - * @returns {void} - */ -export function mark_subtree_inert(signal, inert) { - const flags = signal.f; - const is_already_inert = (flags & INERT) !== 0; - - if (is_already_inert !== inert) { - signal.f ^= INERT; - if (!inert && (flags & CLEAN) === 0) { - schedule_effect(signal); - } - } - - mark_subtree_children_inert(signal, inert); -} - /** * @param {import('#client').Value} signal * @param {number} to_status @@ -1100,7 +1064,8 @@ export function push(props, runes = false, fn) { l1: [], l2: source(false), // update_callbacks - u: null + u: null, + ondestroy: null }; if (DEV) { @@ -1125,7 +1090,7 @@ export function pop(component) { if (effects !== null) { context_stack_item.e = null; for (let i = 0; i < effects.length; i++) { - schedule_effect(effects[i]); + effect(effects[i]); } } current_component_context = context_stack_item.p; diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index 0a89b0045a..db89576e7c 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -20,8 +20,8 @@ export type ComponentContext = { s: Record; /** exports (and props, if `accessors: true`) */ x: Record | null; - /** effects */ - e: null | Effect[]; + /** deferred effects */ + e: null | Array<() => void | (() => void)>; /** mounted */ m: boolean; /** parent */ @@ -43,6 +43,8 @@ export type ComponentContext = { /** onMount callbacks */ m: Array<() => any>; }; + // TODO move this to a separate server component context object + ondestroy: null | Array<() => void>; }; export type Equals = (this: Value, value: unknown) => boolean; @@ -51,8 +53,6 @@ export type TemplateNode = Text | Element | Comment; export type Dom = TemplateNode | TemplateNode[]; -export interface Block {} - export type EachState = { /** flags */ flags: number; diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 313ed39099..a1b7d103ab 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -221,6 +221,13 @@ export function push(runes) { } export function pop() { + var context = /** @type {import('#client').ComponentContext} */ ($.current_component_context); + var ondestroy = context.ondestroy; + + if (ondestroy) { + on_destroy.push(...ondestroy); + } + $.pop(); } diff --git a/packages/svelte/src/main/main-server.js b/packages/svelte/src/main/main-server.js index 58ee20e9ae..4c0135b48a 100644 --- a/packages/svelte/src/main/main-server.js +++ b/packages/svelte/src/main/main-server.js @@ -1,4 +1,4 @@ -import { on_destroy } from '../internal/server/index.js'; +import { current_component_context } from '../internal/client/runtime.js'; export { createEventDispatcher, @@ -18,9 +18,10 @@ export { /** @returns {void} */ export function onMount() {} -/** @param {Function} fn */ +/** @param {() => void} fn */ export function onDestroy(fn) { - on_destroy.push(fn); + const context = /** @type {import('#client').ComponentContext} */ (current_component_context); + (context.ondestroy ??= []).push(fn); } /** @returns {void} */ diff --git a/packages/svelte/src/reactivity/map.test.ts b/packages/svelte/src/reactivity/map.test.ts index e758f453c5..8f4bb791c1 100644 --- a/packages/svelte/src/reactivity/map.test.ts +++ b/packages/svelte/src/reactivity/map.test.ts @@ -1,4 +1,4 @@ -import { pre_effect, effect_root } from '../internal/client/reactivity/effects.js'; +import { render_effect, effect_root } from '../internal/client/reactivity/effects.js'; import { flushSync } from '../main/main-client.js'; import { ReactiveMap } from './map.js'; import { assert, test } from 'vitest'; @@ -15,15 +15,15 @@ test('map.values()', () => { const log: any = []; const cleanup = effect_root(() => { - pre_effect(() => { + render_effect(() => { log.push(map.size); }); - pre_effect(() => { + render_effect(() => { log.push(map.has(3)); }); - pre_effect(() => { + render_effect(() => { log.push(Array.from(map.values())); }); }); @@ -51,15 +51,15 @@ test('map.get(...)', () => { const log: any = []; const cleanup = effect_root(() => { - pre_effect(() => { + render_effect(() => { log.push('get 1', map.get(1)); }); - pre_effect(() => { + render_effect(() => { log.push('get 2', map.get(2)); }); - pre_effect(() => { + render_effect(() => { log.push('get 3', map.get(3)); }); }); @@ -87,15 +87,15 @@ test('map.has(...)', () => { const log: any = []; const cleanup = effect_root(() => { - pre_effect(() => { + render_effect(() => { log.push('has 1', map.has(1)); }); - pre_effect(() => { + render_effect(() => { log.push('has 2', map.has(2)); }); - pre_effect(() => { + render_effect(() => { log.push('has 3', map.has(3)); }); }); @@ -132,7 +132,7 @@ test('map handling of undefined values', () => { const cleanup = effect_root(() => { map.set(1, undefined); - pre_effect(() => { + render_effect(() => { log.push(map.get(1)); }); diff --git a/packages/svelte/src/reactivity/set.test.ts b/packages/svelte/src/reactivity/set.test.ts index def9c64d3b..bc7dea6650 100644 --- a/packages/svelte/src/reactivity/set.test.ts +++ b/packages/svelte/src/reactivity/set.test.ts @@ -1,4 +1,4 @@ -import { pre_effect, effect_root } from '../internal/client/reactivity/effects.js'; +import { render_effect, effect_root } from '../internal/client/reactivity/effects.js'; import { flushSync } from '../main/main-client.js'; import { ReactiveSet } from './set.js'; import { assert, test } from 'vitest'; @@ -9,15 +9,15 @@ test('set.values()', () => { const log: any = []; const cleanup = effect_root(() => { - pre_effect(() => { + render_effect(() => { log.push(set.size); }); - pre_effect(() => { + render_effect(() => { log.push(set.has(3)); }); - pre_effect(() => { + render_effect(() => { log.push(Array.from(set)); }); }); @@ -41,15 +41,15 @@ test('set.has(...)', () => { const log: any = []; const cleanup = effect_root(() => { - pre_effect(() => { + render_effect(() => { log.push('has 1', set.has(1)); }); - pre_effect(() => { + render_effect(() => { log.push('has 2', set.has(2)); }); - pre_effect(() => { + render_effect(() => { log.push('has 3', set.has(3)); }); }); diff --git a/packages/svelte/tests/runtime-legacy/samples/ondestroy-deep/_config.js b/packages/svelte/tests/runtime-legacy/samples/ondestroy-deep/_config.js index 3bf5c9d6d1..96e388a7e1 100644 --- a/packages/svelte/tests/runtime-legacy/samples/ondestroy-deep/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/ondestroy-deep/_config.js @@ -2,17 +2,16 @@ import { test } from '../../test'; import { destroyed, reset } from './destroyed.js'; export default test({ - test({ assert, component }) { - // for hydration, ssr may have pushed to `destroyed` + before_test() { reset(); + }, + test({ assert, component }) { component.visible = false; - assert.deepEqual(destroyed, ['A', 'B', 'C']); - - reset(); + assert.deepEqual(destroyed, ['C', 'B', 'A']); }, test_ssr({ assert }) { - assert.deepEqual(destroyed, ['A', 'B', 'C']); + assert.deepEqual(destroyed, ['C', 'B', 'A']); } }); 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 index c6194149f1..35619e3a0a 100644 --- 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 @@ -1,12 +1,21 @@ -import { tick } from 'svelte'; +import { flushSync } from 'svelte'; import { test } from '../../test'; +import { log } from './log.js'; export default test({ html: '', - async test({ assert, target }) { + + before_test() { + log.length = 0; + }, + + test({ assert, target }) { + assert.deepEqual(log, [2, 1]); + const button = target.querySelector('button'); - button?.click(); - await tick(); + + flushSync(() => button?.click()); + assert.deepEqual(log, [2, 1, 2, 1]); assert.htmlEqual(target.innerHTML, ''); } diff --git a/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-prevent-loop/log.js b/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-prevent-loop/log.js new file mode 100644 index 0000000000..d3df521f4d --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/reactive-assignment-prevent-loop/log.js @@ -0,0 +1,2 @@ +/** @type {any[]} */ +export const log = []; 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 index 91aff79c89..c65c992b0c 100644 --- 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 @@ -1,9 +1,22 @@ diff --git a/packages/svelte/tests/runtime-legacy/samples/reactive-import-statement/_config.js b/packages/svelte/tests/runtime-legacy/samples/reactive-import-statement/_config.js index e05e8180b2..ea3ab4b356 100644 --- a/packages/svelte/tests/runtime-legacy/samples/reactive-import-statement/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/reactive-import-statement/_config.js @@ -1,3 +1,4 @@ +import { flushSync } from 'svelte'; import { ok, test } from '../../test'; import { reset_numbers } from './data'; @@ -18,7 +19,9 @@ export default test({ const clickEvent = new window.MouseEvent('click', { bubbles: true }); - await btn.dispatchEvent(clickEvent); + flushSync(() => { + btn.dispatchEvent(clickEvent); + }); assert.htmlEqual( target.innerHTML, @@ -31,7 +34,9 @@ export default test({ ` ); - await btn.dispatchEvent(clickEvent); + flushSync(() => { + btn.dispatchEvent(clickEvent); + }); assert.htmlEqual( target.innerHTML, diff --git a/packages/svelte/tests/runtime-legacy/samples/reactive-statement-store/_config.js b/packages/svelte/tests/runtime-legacy/samples/reactive-statement-store/_config.js index 0b705cc4f1..99007bfdd9 100644 --- a/packages/svelte/tests/runtime-legacy/samples/reactive-statement-store/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/reactive-statement-store/_config.js @@ -2,7 +2,6 @@ import { tick } from 'svelte'; import { test } from '../../test'; export default test({ - skip: true, // failing test for https://github.com/sveltejs/svelte/issues/10787 html: ``, async test({ assert, target }) { target.querySelector('button')?.click(); diff --git a/packages/svelte/tests/runtime-runes/samples/effect-order-2/_config.js b/packages/svelte/tests/runtime-runes/samples/effect-order-2/_config.js new file mode 100644 index 0000000000..302026493f --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-order-2/_config.js @@ -0,0 +1,30 @@ +import { test } from '../../test'; +import { flushSync } from 'svelte'; +import { log } from './log.js'; + +export default test({ + before_test() { + log.length = 0; + }, + + async test({ assert, target }) { + const [b1] = target.querySelectorAll('button'); + flushSync(() => { + b1.click(); + }); + flushSync(() => { + b1.click(); + }); + assert.deepEqual(log, [ + { a: 1 }, + { b: 1 }, + { c: 1 }, + { a: 2 }, + { b: 2 }, + { c: 2 }, + { a: 3 }, + { b: 3 }, + { c: 3 } + ]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/effect-order-2/log.js b/packages/svelte/tests/runtime-runes/samples/effect-order-2/log.js new file mode 100644 index 0000000000..d3df521f4d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-order-2/log.js @@ -0,0 +1,2 @@ +/** @type {any[]} */ +export const log = []; diff --git a/packages/svelte/tests/runtime-runes/samples/effect-order-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/effect-order-2/main.svelte new file mode 100644 index 0000000000..fb9e9a6c53 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-order-2/main.svelte @@ -0,0 +1,28 @@ + + + diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index ef2bc96fc3..54399db322 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -1,8 +1,8 @@ import { describe, assert, it } from 'vitest'; import * as $ from '../../src/internal/client/runtime'; import { - destroy_effect, effect, + effect_root, render_effect, user_effect } from '../../src/internal/client/reactivity/effects'; @@ -22,12 +22,12 @@ function run_test(runes: boolean, fn: (runes: boolean) => () => void) { $.push({}, runes); // Create a render context so that effect validations etc don't fail let execute: any; - const signal = render_effect(() => { + const destroy = effect_root(() => { execute = fn(runes); }); $.pop(); execute(); - destroy_effect(signal); + destroy(); }; } @@ -248,8 +248,10 @@ describe('signals', () => { test('effect with derived using unowned derived every time', () => { const log: Array = []; - const effect = user_effect(() => { - log.push($.get(calc)); + const destroy = effect_root(() => { + user_effect(() => { + log.push($.get(calc)); + }); }); return () => { @@ -261,7 +263,7 @@ describe('signals', () => { // Ensure we're not leaking consumers assert.deepEqual(count.reactions?.length, 1); assert.deepEqual(log, [0, 2, 'limit', 0]); - destroy_effect(effect); + destroy(); // Ensure we're not leaking consumers assert.deepEqual(count.reactions, null); };