chore: tidy up effect init (#10931)

* move signal init logic into create_effect

* tidy up

* call set_signal_status inside execute_effect

* tidy up

* unused import

Author: Rich-Harris

packages/svelte/src/internal/client/reactivity/effects.js

@@ -7,8 +7,10 @@ import {
 	destroy_children,
 	execute_effect,
 	get,
+	is_flushing_effect,
 	remove_reactions,
 	schedule_effect,
+	set_is_flushing_effect,
 	set_signal_status,
 	untrack
 } from '../runtime.js';
@@ -20,7 +22,8 @@ import {
 	PRE_EFFECT,
 	DESTROYED,
 	INERT,
-	IS_ELSEIF
+	IS_ELSEIF,
+	EFFECT_RAN
 } from '../constants.js';
 import { set } from './sources.js';
 import { noop } from '../../common.js';
@@ -35,7 +38,7 @@ import { remove } from '../dom/reconciler.js';
  */
 function create_effect(type, fn, sync, init = true) {
 	/** @type {import('#client').Effect} */
-	const signal = {
+	const effect = {
 		parent: current_effect,
 		dom: null,
 		deps: null,
@@ -51,22 +54,34 @@ function create_effect(type, fn, sync, init = true) {
 	};
 
 	if (current_effect !== null) {
-		signal.l = current_effect.l + 1;
+		effect.l = current_effect.l + 1;
 	}
 
 	if (current_reaction !== null) {
 		if (current_reaction.effects === null) {
-			current_reaction.effects = [signal];
+			current_reaction.effects = [effect];
 		} else {
-			current_reaction.effects.push(signal);
+			current_reaction.effects.push(effect);
 		}
 	}
 
 	if (init) {
-		schedule_effect(signal, sync);
+		if (sync) {
+			const 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);
+		}
 	}
 
-	return signal;
+	return effect;
 }
 
 /**

packages/svelte/src/internal/client/reactivity/sources.js

@@ -135,7 +135,7 @@ export function set(signal, value) {
 		) {
 			if (current_dependencies !== null && current_dependencies.includes(signal)) {
 				set_signal_status(current_effect, DIRTY);
-				schedule_effect(current_effect, false);
+				schedule_effect(current_effect);
 			} else {
 				if (current_untracked_writes === null) {
 					set_current_untracked_writes([signal]);

packages/svelte/src/internal/client/runtime.js

@@ -36,7 +36,13 @@ const FLUSH_SYNC = 1;
 let current_scheduler_mode = FLUSH_MICROTASK;
 // Used for handling scheduling
 let is_micro_task_queued = false;
-let is_flushing_effect = false;
+export let is_flushing_effect = false;
+
+/** @param {boolean} value */
+export function set_is_flushing_effect(value) {
+	is_flushing_effect = value;
+}
+
 // Used for $inspect
 export let is_batching_effect = false;
 let is_inspecting_signal = false;
@@ -210,9 +216,6 @@ export function check_dirtiness(reaction) {
  * @returns {V}
  */
 export function execute_reaction_fn(signal) {
-	const fn = signal.fn;
-	const flags = signal.f;
-
 	const previous_dependencies = current_dependencies;
 	const previous_dependencies_index = current_dependencies_index;
 	const previous_untracked_writes = current_untracked_writes;
@@ -224,11 +227,11 @@ export function execute_reaction_fn(signal) {
 	current_dependencies_index = 0;
 	current_untracked_writes = null;
 	current_reaction = signal;
-	current_skip_reaction = !is_flushing_effect && (flags & UNOWNED) !== 0;
+	current_skip_reaction = !is_flushing_effect && (signal.f & UNOWNED) !== 0;
 	current_untracking = false;
 
 	try {
-		let res = fn();
+		let res = signal.fn();
 		let dependencies = /** @type {import('./types.js').Value<unknown>[]} **/ (signal.deps);
 		if (current_dependencies !== null) {
 			let i;
@@ -373,33 +376,35 @@ export function destroy_children(signal) {
 }
 
 /**
- * @param {import('./types.js').Effect} signal
+ * @param {import('./types.js').Effect} effect
  * @returns {void}
  */
-export function execute_effect(signal) {
-	if ((signal.f & DESTROYED) !== 0) {
+export function execute_effect(effect) {
+	if ((effect.f & DESTROYED) !== 0) {
 		return;
 	}
 
-	const previous_effect = current_effect;
-	const previous_component_context = current_component_context;
+	set_signal_status(effect, CLEAN);
 
-	const component_context = signal.ctx;
+	var component_context = effect.ctx;
 
-	current_effect = signal;
+	var previous_effect = current_effect;
+	var previous_component_context = current_component_context;
+
+	current_effect = effect;
 	current_component_context = component_context;
 
 	try {
-		destroy_children(signal);
-		signal.teardown?.();
-		const teardown = execute_reaction_fn(signal);
-		signal.teardown = typeof teardown === 'function' ? teardown : null;
+		destroy_children(effect);
+		effect.teardown?.();
+		var teardown = execute_reaction_fn(effect);
+		effect.teardown = typeof teardown === 'function' ? teardown : null;
 	} finally {
 		current_effect = previous_effect;
 		current_component_context = previous_component_context;
 	}
 
-	if ((signal.f & PRE_EFFECT) !== 0 && current_queued_pre_and_render_effects.length > 0) {
+	if ((effect.f & PRE_EFFECT) !== 0 && current_queued_pre_and_render_effects.length > 0) {
 		flush_local_pre_effects(component_context);
 	}
 }
@@ -436,7 +441,6 @@ function flush_queued_effects(effects) {
 
 			if ((signal.f & (DESTROYED | INERT)) === 0) {
 				if (check_dirtiness(signal)) {
-					set_signal_status(signal, CLEAN);
 					execute_effect(signal);
 				}
 			}
@@ -466,85 +470,73 @@ function process_microtask() {
 
 /**
  * @param {import('./types.js').Effect} signal
- * @param {boolean} sync
  * @returns {void}
  */
-export function schedule_effect(signal, sync) {
+export function schedule_effect(signal) {
 	const flags = signal.f;
-	if (sync) {
-		const previously_flushing_effect = is_flushing_effect;
-		try {
-			is_flushing_effect = true;
-			execute_effect(signal);
-			set_signal_status(signal, CLEAN);
-		} finally {
-			is_flushing_effect = previously_flushing_effect;
+
+	if (current_scheduler_mode === FLUSH_MICROTASK) {
+		if (!is_micro_task_queued) {
+			is_micro_task_queued = true;
+			queueMicrotask(process_microtask);
 		}
-	} else {
-		if (current_scheduler_mode === FLUSH_MICROTASK) {
-			if (!is_micro_task_queued) {
-				is_micro_task_queued = true;
-				queueMicrotask(process_microtask);
-			}
+	}
+
+	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 & MANAGED) === 0) {
+			mark_subtree_children_inert(signal, true);
 		}
-		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 & MANAGED) === 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);
+	} 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++;
 						}
-						break;
-					}
-					if (i === 0) {
-						current_queued_pre_and_render_effects.unshift(signal);
-						break;
+						current_queued_pre_and_render_effects.splice(i, 0, signal);
 					}
+					break;
+				}
+				if (i === 0) {
+					current_queued_pre_and_render_effects.unshift(signal);
+					break;
 				}
 			}
+		}
 
-			if (should_append) {
-				current_queued_pre_and_render_effects.push(signal);
-			}
+		if (should_append) {
+			current_queued_pre_and_render_effects.push(signal);
 		}
 	}
-
-	signal.f |= EFFECT_RAN;
 }
 
 /**
@@ -696,7 +688,7 @@ export function get(signal) {
 			current_untracked_writes.includes(signal)
 		) {
 			set_signal_status(current_effect, DIRTY);
-			schedule_effect(current_effect, false);
+			schedule_effect(current_effect);
 		}
 	}
 
@@ -772,7 +764,7 @@ export function mark_subtree_inert(signal, inert) {
 	if (is_already_inert !== inert) {
 		signal.f ^= INERT;
 		if (!inert && (flags & CLEAN) === 0) {
-			schedule_effect(signal, false);
+			schedule_effect(signal);
 		}
 	}
 
@@ -819,7 +811,7 @@ export function mark_reactions(signal, to_status, force_schedule) {
 					force_schedule
 				);
 			} else {
-				schedule_effect(/** @type {import('#client').Effect} */ (reaction), false);
+				schedule_effect(/** @type {import('#client').Effect} */ (reaction));
 			}
 		}
 	}
@@ -1075,7 +1067,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], false);
+				schedule_effect(effects[i]);
 			}
 		}
 		current_component_context = context_stack_item.p;