chore: simplify $inspect (#12065)

* simplify $inspect

* tidy up

* tidy up

* unused

* unused

Author: Rich-Harris

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

@@ -16,6 +16,7 @@ export const EFFECT_RAN = 1 << 14;
 export const EFFECT_TRANSPARENT = 1 << 15;
 /** Svelte 4 legacy mode props need to be handled with deriveds and be recognized elsewhere, hence the dedicated flag */
 export const LEGACY_DERIVED_PROP = 1 << 16;
+export const INSPECT_EFFECT = 1 << 17;
 
 export const STATE_SYMBOL = Symbol('$state');
 export const STATE_FROZEN_SYMBOL = Symbol('$state.frozen');

packages/svelte/src/internal/client/dev/inspect.js

@@ -1,20 +1,7 @@
-import { DEV } from 'esm-env';
 import { snapshot } from '../proxy.js';
-import { render_effect, validate_effect } from '../reactivity/effects.js';
-import { deep_read, untrack } from '../runtime.js';
+import { inspect_effect, validate_effect } from '../reactivity/effects.js';
 import { array_prototype, get_prototype_of, object_prototype } from '../utils.js';
 
-/** @type {Function | null} */
-export let inspect_fn = null;
-
-/** @param {Function | null} fn */
-export function set_inspect_fn(fn) {
-	inspect_fn = fn;
-}
-
-/** @type {Array<import('#client').ValueDebug>} */
-export let inspect_captured_signals = [];
-
 /**
  * @param {() => any[]} get_value
  * @param {Function} [inspector]
@@ -25,32 +12,9 @@ export function inspect(get_value, inspector = console.log) {
 
 	let initial = true;
 
-	// we assign the function directly to signals, rather than just
-	// calling `inspector` directly inside the effect, so that
-	// we get useful stack traces
-	var fn = () => {
-		const value = untrack(() => deep_snapshot(get_value()));
-		inspector(initial ? 'init' : 'update', ...value);
-	};
-
-	render_effect(() => {
-		inspect_fn = fn;
-		deep_read(get_value());
-		inspect_fn = null;
-
-		const signals = inspect_captured_signals.slice();
-		inspect_captured_signals = [];
-
-		if (initial) {
-			fn();
-			initial = false;
-		}
-
-		return () => {
-			for (const s of signals) {
-				s.inspect.delete(fn);
-			}
-		};
+	inspect_effect(() => {
+		inspector(initial ? 'init' : 'update', ...deep_snapshot(get_value()));
+		initial = false;
 	});
 }
 

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

@@ -1,11 +1,5 @@
 import { DEV } from 'esm-env';
-import {
-	get,
-	batch_inspect,
-	current_component_context,
-	untrack,
-	current_effect
-} from './runtime.js';
+import { get, current_component_context, untrack, current_effect } from './runtime.js';
 import {
 	array_prototype,
 	define_property,
@@ -230,11 +224,6 @@ const state_proxy_handler = {
 			return value === UNINITIALIZED ? undefined : value;
 		}
 
-		if (DEV) {
-			if (typeof target[prop] === 'function' && prop !== Symbol.iterator) {
-				return batch_inspect(target, prop, receiver);
-			}
-		}
 		return Reflect.get(target, prop, receiver);
 	},
 

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

@@ -1,4 +1,3 @@
-import { DEV } from 'esm-env';
 import { CLEAN, DERIVED, DESTROYED, DIRTY, MAYBE_DIRTY, UNOWNED } from '../constants.js';
 import {
 	current_reaction,
@@ -39,10 +38,6 @@ export function derived(fn) {
 		version: 0
 	};
 
-	if (DEV) {
-		/** @type {import('#client').DerivedDebug} */ (signal).inspect = new Set();
-	}
-
 	if (current_reaction !== null && (current_reaction.f & DERIVED) !== 0) {
 		var current_derived = /** @type {import('#client').Derived<V>} */ (current_reaction);
 		if (current_derived.deriveds === null) {
@@ -107,10 +102,6 @@ export function update_derived(derived, force_schedule) {
 		derived.version = increment_version();
 
 		mark_reactions(derived, DIRTY, force_schedule);
-
-		if (DEV && force_schedule) {
-			for (var fn of /** @type {import('#client').DerivedDebug} */ (derived).inspect) fn();
-		}
 	}
 }
 

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

@@ -30,7 +30,8 @@ import {
 	EFFECT_TRANSPARENT,
 	DERIVED,
 	UNOWNED,
-	CLEAN
+	CLEAN,
+	INSPECT_EFFECT
 } from '../constants.js';
 import { set } from './sources.js';
 import { remove } from '../dom/reconciler.js';
@@ -204,6 +205,11 @@ export function user_pre_effect(fn) {
 	return render_effect(fn);
 }
 
+/** @param {() => void | (() => void)} fn */
+export function inspect_effect(fn) {
+	return create_effect(INSPECT_EFFECT, fn, true);
+}
+
 /**
  * Internal representation of `$effect.root(...)`
  * @param {() => void | (() => void)} fn

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

@@ -10,7 +10,6 @@ import { mutable_source, set, source } from './sources.js';
 import { derived, derived_safe_equal } from './deriveds.js';
 import { get, is_signals_recorded, untrack, update } from '../runtime.js';
 import { safe_equals } from './equality.js';
-import { inspect_fn } from '../dev/inspect.js';
 import * as e from '../errors.js';
 import { LEGACY_DERIVED_PROP } from '../constants.js';
 
@@ -308,7 +307,7 @@ export function prop(props, key, flags, fallback) {
 
 		// legacy nonsense — need to ensure the source is invalidated when necessary
 		// also needed for when handling inspect logic so we can inspect the correct source signal
-		if (is_signals_recorded || (DEV && inspect_fn)) {
+		if (is_signals_recorded) {
 			// set this so that we don't reset to the parent value if `d`
 			// is invalidated because of `invalidate_inner_signals` (rather
 			// than because the parent or child value changed)

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

@@ -6,15 +6,15 @@ import {
 	current_effect,
 	current_untracked_writes,
 	get,
-	is_batching_effect,
 	is_runes,
 	mark_reactions,
 	schedule_effect,
 	set_current_untracked_writes,
-	set_last_inspected_signal,
 	set_signal_status,
 	untrack,
-	increment_version
+	increment_version,
+	execute_effect,
+	inspect_effects
 } from '../runtime.js';
 import { equals, safe_equals } from './equality.js';
 import { CLEAN, DERIVED, DIRTY, BRANCH_EFFECT } from '../constants.js';
@@ -23,25 +23,18 @@ import * as e from '../errors.js';
 
 /**
  * @template V
- * @param {V} value
+ * @param {V} v
  * @returns {import('#client').Source<V>}
  */
 /*#__NO_SIDE_EFFECTS__*/
-export function source(value) {
-	/** @type {import('#client').Source<V>} */
-	const source = {
+export function source(v) {
+	return {
 		f: 0, // TODO ideally we could skip this altogether, but it causes type errors
+		v,
 		reactions: null,
-		equals: equals,
-		v: value,
+		equals,
 		version: 0
 	};
-
-	if (DEV) {
-		/** @type {import('#client').ValueDebug<V>} */ (source).inspect = new Set();
-	}
-
-	return source;
 }
 
 /**
@@ -129,11 +122,11 @@ export function set(source, value) {
 		}
 
 		if (DEV) {
-			if (is_batching_effect) {
-				set_last_inspected_signal(/** @type {import('#client').ValueDebug} */ (source));
-			} else {
-				for (const fn of /** @type {import('#client').ValueDebug} */ (source).inspect) fn();
+			for (const effect of inspect_effects) {
+				execute_effect(effect);
 			}
+
+			inspect_effects.clear();
 		}
 	}
 

packages/svelte/src/internal/client/reactivity/types.d.ts

@@ -53,12 +53,6 @@ export interface Effect extends Reaction {
 	component_function?: any;
 }
 
-export interface ValueDebug<V = unknown> extends Value<V> {
-	inspect: Set<Function>;
-}
-
-export interface DerivedDebug<V = unknown> extends Derived<V>, ValueDebug<V> {}
-
 export type Source<V = unknown> = Value<V>;
 
 export type MaybeSource<T = unknown> = T | Source<T>;

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

@@ -29,13 +29,13 @@ import {
 	ROOT_EFFECT,
 	LEGACY_DERIVED_PROP,
 	DISCONNECTED,
-	STATE_FROZEN_SYMBOL
+	STATE_FROZEN_SYMBOL,
+	INSPECT_EFFECT
 } from './constants.js';
 import { flush_tasks } from './dom/task.js';
 import { add_owner } from './dev/ownership.js';
 import { mutate, set, source } from './reactivity/sources.js';
 import { update_derived } from './reactivity/deriveds.js';
-import { inspect_captured_signals, inspect_fn, set_inspect_fn } from './dev/inspect.js';
 import * as e from './errors.js';
 import { lifecycle_outside_component } from '../shared/errors.js';
 
@@ -63,9 +63,7 @@ export function set_is_destroying_effect(value) {
 	is_destroying_effect = value;
 }
 
-// Used for $inspect
-export let is_batching_effect = false;
-let is_inspecting_signal = false;
+export let inspect_effects = new Set();
 
 // Handle effect queues
 
@@ -106,14 +104,6 @@ export function set_current_untracked_writes(value) {
 	current_untracked_writes = value;
 }
 
-/** @type {null | import('#client').ValueDebug} */
-export let last_inspected_signal = null;
-
-/** @param {null | import('#client').ValueDebug} signal */
-export function set_last_inspected_signal(signal) {
-	last_inspected_signal = signal;
-}
-
 /** @type {number} Used by sources and deriveds for handling updates to unowned deriveds */
 let current_version = 0;
 
@@ -159,38 +149,6 @@ export function is_runes() {
 	return current_component_context !== null && current_component_context.l === null;
 }
 
-/**
- * @param {import('#client').ProxyStateObject} target
- * @param {string | symbol} prop
- * @param {any} receiver
- */
-export function batch_inspect(target, prop, receiver) {
-	const value = Reflect.get(target, prop, receiver);
-	/**
-	 * @this {any}
-	 */
-	return function () {
-		const previously_batching_effect = is_batching_effect;
-		is_batching_effect = true;
-		try {
-			return Reflect.apply(value, this, arguments);
-		} finally {
-			is_batching_effect = previously_batching_effect;
-			if (last_inspected_signal !== null && !is_inspecting_signal) {
-				is_inspecting_signal = true;
-				try {
-					for (const fn of last_inspected_signal.inspect) {
-						fn();
-					}
-				} finally {
-					is_inspecting_signal = false;
-				}
-				last_inspected_signal = null;
-			}
-		}
-	};
-}
-
 /**
  * Determines whether a derived or effect is dirty.
  * If it is MAYBE_DIRTY, will set the status to CLEAN
@@ -793,12 +751,6 @@ export async function tick() {
  * @returns {V}
  */
 export function get(signal) {
-	if (DEV && inspect_fn) {
-		var s = /** @type {import('#client').ValueDebug} */ (signal);
-		s.inspect.add(inspect_fn);
-		inspect_captured_signals.push(s);
-	}
-
 	const flags = signal.f;
 	if ((flags & DESTROYED) !== 0) {
 		return signal.v;
@@ -846,15 +798,7 @@ export function get(signal) {
 		(flags & DERIVED) !== 0 &&
 		check_dirtiness(/** @type {import('#client').Derived} */ (signal))
 	) {
-		if (DEV) {
-			// we want to avoid tracking indirect dependencies
-			const previous_inspect_fn = inspect_fn;
-			set_inspect_fn(null);
-			update_derived(/** @type {import('#client').Derived} **/ (signal), false);
-			set_inspect_fn(previous_inspect_fn);
-		} else {
-			update_derived(/** @type {import('#client').Derived} **/ (signal), false);
-		}
+		update_derived(/** @type {import('#client').Derived} **/ (signal), false);
 	}
 
 	return signal.v;
@@ -914,6 +858,11 @@ export function mark_reactions(signal, to_status, force_schedule) {
 		var reaction = reactions[i];
 		var flags = reaction.f;
 
+		if (DEV && (flags & INSPECT_EFFECT) !== 0) {
+			inspect_effects.add(reaction);
+			continue;
+		}
+
 		// We skip any effects that are already dirty. Additionally, we also
 		// skip if the reaction is the same as the current effect (except if we're not in runes or we
 		// are in force schedule mode).