remove PROXY_REMOVE_PATH (#16126)

* remove PROXY_REMOVE_PATH

* simplify a bit

* simplify

* tweak

* tweak implementation

* tweak implementation

* tweak implementation

* hoist

* tweak

* fix

* WIP (reduce number of with_parent calls, move towards possibility of combining tag and tag_proxy)

* DRY out

packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js

@@ -9,7 +9,6 @@ import { get_rune } from '../../../scope.js';
 import { get_prop_source, is_prop_source, is_state_source, should_proxy } from '../utils.js';
 import { is_hoisted_function } from '../../utils.js';
 import { get_value } from './shared/declarations.js';
-import { PROXY_REMOVE_PATH } from '#client/constants';
 
 /**
  * @param {VariableDeclaration} node
@@ -90,12 +89,11 @@ export function VariableDeclaration(node, context) {
 								binding.kind === 'bindable_prop' &&
 								should_proxy(initial, context.state.scope)
 							) {
-								initial = b.call(
-									'$.proxy',
-									initial,
-									dev ? b.literal(id.name) : undefined,
-									dev ? b.literal(PROXY_REMOVE_PATH) : undefined
-								);
+								initial = b.call('$.proxy', initial);
+
+								if (dev) {
+									initial = b.call('$.tag_proxy', initial, b.literal(id.name));
+								}
 							}
 
 							if (is_prop_source(binding, context.state)) {
@@ -136,20 +134,23 @@ export function VariableDeclaration(node, context) {
 					);
 					const is_state = is_state_source(binding, context.state.analysis);
 					const is_proxy = should_proxy(value, context.state.scope);
+
 					if (rune === '$state' && is_proxy) {
-						value = b.call(
-							'$.proxy',
-							value,
-							dev ? b.literal(id.name) : undefined,
-							dev ? b.literal(PROXY_REMOVE_PATH) : undefined
-						);
+						value = b.call('$.proxy', value);
+
+						if (dev && !is_state) {
+							value = b.call('$.tag_proxy', value, b.literal(id.name));
 						}
+					}
+
 					if (is_state) {
 						value = b.call('$.state', value);
-					}
-					if (dev && is_state) {
+
+						if (dev) {
 							value = b.call('$.tag', value, b.literal(id.name));
 						}
+					}
+
 					return value;
 				};
 

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

@@ -22,14 +22,6 @@ export const HEAD_EFFECT = 1 << 19;
 export const EFFECT_HAS_DERIVED = 1 << 20;
 export const EFFECT_IS_UPDATING = 1 << 21;
 
-// `$inspect.trace` proxy path flags
-/** Keep path the same */
-export const PROXY_PRESERVE_PATH = 1 << 1;
-/** Change proxy path to new "owner" */
-export const PROXY_CHANGE_PATH = 1 << 2;
-/** "Unown" proxy, so its path becomes `[$state proxy]` */
-export const PROXY_REMOVE_PATH = 1 << 3;
-
 export const STATE_SYMBOL = Symbol('$state');
 export const LEGACY_PROPS = Symbol('legacy props');
 export const LOADING_ATTR_SYMBOL = Symbol('');

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

@@ -2,7 +2,7 @@
 import { UNINITIALIZED } from '../../../constants.js';
 import { snapshot } from '../../shared/clone.js';
 import { define_property } from '../../shared/utils.js';
-import { DERIVED, STATE_SYMBOL } from '#client/constants';
+import { DERIVED, PROXY_PATH_SYMBOL, STATE_SYMBOL } from '#client/constants';
 import { effect_tracking } from '../reactivity/effects.js';
 import { active_reaction, captured_signals, set_captured_signals, untrack } from '../runtime.js';
 
@@ -43,7 +43,7 @@ function log_entry(signal, entry) {
 	const type = (signal.f & DERIVED) !== 0 ? '$derived' : '$state';
 	const current_reaction = /** @type {Reaction} */ (active_reaction);
 	const dirty = signal.wv > current_reaction.wv || current_reaction.wv === 0;
-	const { trace_name: name } = signal;
+	const { label: name } = signal;
 	const style = dirty
 		? 'color: CornflowerBlue; font-weight: bold'
 		: 'color: grey; font-weight: normal';
@@ -183,13 +183,25 @@ export function get_stack(label) {
 
 /**
  * @param {Value} source
- * @param {string} name
+ * @param {string} label
  */
-export function tag(source, name) {
-	source.trace_name = name;
+export function tag(source, label) {
+	source.label = label;
+	tag_proxy(source.v, label);
+
 	return source;
 }
 
+/**
+ * @param {unknown} value
+ * @param {string} label
+ */
+export function tag_proxy(value, label) {
+	// @ts-expect-error
+	value?.[PROXY_PATH_SYMBOL]?.(label);
+	return value;
+}
+
 /**
  * @param {unknown} value
  */

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

@@ -7,7 +7,7 @@ export { add_locations } from './dev/elements.js';
 export { hmr } from './dev/hmr.js';
 export { create_ownership_validator } from './dev/ownership.js';
 export { check_target, legacy_api } from './dev/legacy.js';
-export { trace, tag } from './dev/tracing.js';
+export { trace, tag, tag_proxy } from './dev/tracing.js';
 export { inspect } from './dev/inspect.js';
 export { validate_snippet_args } from './dev/validation.js';
 export { await_block as await } from './dom/blocks/await.js';

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

@@ -9,38 +9,21 @@ import {
 	object_prototype
 } from '../shared/utils.js';
 import { state as source, set } from './reactivity/sources.js';
-import {
-	PROXY_CHANGE_PATH,
-	PROXY_PATH_SYMBOL,
-	PROXY_PRESERVE_PATH,
-	PROXY_REMOVE_PATH,
-	STATE_SYMBOL
-} from '#client/constants';
+import { PROXY_PATH_SYMBOL, STATE_SYMBOL } from '#client/constants';
 import { UNINITIALIZED } from '../../constants.js';
 import * as e from './errors.js';
 import { get_stack, tag } from './dev/tracing.js';
 import { tracing_mode_flag } from '../flags/index.js';
 
+// TODO move all regexes into shared module?
+const regex_is_valid_identifier = /^[a-zA-Z_$][a-zA-Z_$0-9]*$/;
+
 /**
  * @template T
  * @param {T} value
- * @param {string} [path]
- * @param {number} [path_preservation]
  * @returns {T}
  */
-export function proxy(value, path, path_preservation = PROXY_PRESERVE_PATH) {
-	// if `DEV`, change the proxy `path` since we don't know if its still "owned" by its original source
-	if (
-		DEV &&
-		(path_preservation & PROXY_PRESERVE_PATH) === 0 &&
-		typeof value === 'object' &&
-		value !== null &&
-		STATE_SYMBOL in value &&
-		PROXY_PATH_SYMBOL in value
-	) {
-		value[PROXY_PATH_SYMBOL] =
-			(path_preservation & PROXY_CHANGE_PATH) === 0 ? '[$state proxy]' : path;
-	}
+export function proxy(value) {
 	// if non-proxyable, or is already a proxy, return `value`
 	if (typeof value !== 'object' || value === null || STATE_SYMBOL in value) {
 		return value;
@@ -55,20 +38,10 @@ export function proxy(value, path, path_preservation = PROXY_PRESERVE_PATH) {
 	/** @type {Map<any, Source<any>>} */
 	var sources = new Map();
 	var is_proxied_array = is_array(value);
-	var version = DEV ? tag(source(0), `${path} version`) : source(0);
+	var version = source(0);
 
 	var stack = DEV && tracing_mode_flag ? get_stack('CreatedAt') : null;
 	var reaction = active_reaction;
-	/** @type {(prop: any) => any} */
-	var to_trace_name = DEV
-		? (prop) => {
-				return typeof prop === 'symbol'
-					? `${path}[Symbol(${prop.description ?? ''})]`
-					: typeof prop === 'number' || Number(prop) === Number(prop)
-						? `${path}[${prop}]`
-						: `${path}.${prop}`;
-			}
-		: (prop) => undefined;
 
 	/**
 	 * @template T
@@ -88,8 +61,22 @@ export function proxy(value, path, path_preservation = PROXY_PRESERVE_PATH) {
 	if (is_proxied_array) {
 		// We need to create the length source eagerly to ensure that
 		// mutations to the array are properly synced with our proxy
-		const length_source = source(/** @type {any[]} */ (value).length, stack);
-		sources.set('length', DEV ? tag(length_source, to_trace_name('length')) : length_source);
+		sources.set('length', source(/** @type {any[]} */ (value).length, stack));
+	}
+
+	/** Used in dev for $inspect.trace() */
+	var path = '';
+
+	/** @param {string} new_path */
+	function update_path(new_path) {
+		path = new_path;
+
+		tag(version, `${path} version`);
+
+		// rename all child sources and child proxies
+		for (const [prop, source] of sources) {
+			tag(source, get_label(path, prop));
+		}
 	}
 
 	return new Proxy(/** @type {any} */ (value), {
@@ -107,18 +94,20 @@ export function proxy(value, path, path_preservation = PROXY_PRESERVE_PATH) {
 				e.state_descriptors_fixed();
 			}
 
+			with_parent(() => {
 				var s = sources.get(prop);
 
 				if (s === undefined) {
-				s = with_parent(() => source(descriptor.value, stack));
-				s = DEV && typeof prop === 'string' ? tag(s, to_trace_name(prop)) : s;
+					s = source(descriptor.value, stack);
 					sources.set(prop, s);
+
+					if (DEV && typeof prop === 'string') {
+						tag(s, get_label(path, prop));
+					}
 				} else {
-				set(
-					s,
-					with_parent(() => proxy(descriptor.value, to_trace_name(prop)))
-				);
+					set(s, descriptor.value, true);
 				}
+			});
 
 			return true;
 		},
@@ -129,8 +118,12 @@ export function proxy(value, path, path_preservation = PROXY_PRESERVE_PATH) {
 			if (s === undefined) {
 				if (prop in target) {
 					const s = with_parent(() => source(UNINITIALIZED, stack));
-					sources.set(prop, DEV ? tag(s, to_trace_name(prop)) : s);
+					sources.set(prop, s);
 					update_version(version);
+
+					if (DEV) {
+						tag(s, get_label(path, prop));
+					}
 				}
 			} else {
 				// When working with arrays, we need to also ensure we update the length when removing
@@ -154,8 +147,9 @@ export function proxy(value, path, path_preservation = PROXY_PRESERVE_PATH) {
 			if (prop === STATE_SYMBOL) {
 				return value;
 			}
+
 			if (DEV && prop === PROXY_PATH_SYMBOL) {
-				return path;
+				return update_path;
 			}
 
 			var s = sources.get(prop);
@@ -163,10 +157,17 @@ export function proxy(value, path, path_preservation = PROXY_PRESERVE_PATH) {
 
 			// create a source, but only if it's an own property and not a prototype property
 			if (s === undefined && (!exists || get_descriptor(target, prop)?.writable)) {
-				s = with_parent(() =>
-					source(proxy(exists ? target[prop] : UNINITIALIZED, to_trace_name(prop)), stack)
-				);
-				s = DEV ? tag(s, to_trace_name(prop)) : s;
+				s = with_parent(() => {
+					var p = proxy(exists ? target[prop] : UNINITIALIZED);
+					var s = source(p, stack);
+
+					if (DEV) {
+						tag(s, get_label(path, prop));
+					}
+
+					return s;
+				});
+
 				sources.set(prop, s);
 			}
 
@@ -202,7 +203,7 @@ export function proxy(value, path, path_preservation = PROXY_PRESERVE_PATH) {
 		},
 
 		has(target, prop) {
-			if (prop === STATE_SYMBOL || (DEV && prop === PROXY_PATH_SYMBOL)) {
+			if (prop === STATE_SYMBOL) {
 				return true;
 			}
 
@@ -214,10 +215,17 @@ export function proxy(value, path, path_preservation = PROXY_PRESERVE_PATH) {
 				(active_effect !== null && (!has || get_descriptor(target, prop)?.writable))
 			) {
 				if (s === undefined) {
-					s = with_parent(() =>
-						source(has ? proxy(target[prop], to_trace_name(prop)) : UNINITIALIZED, stack)
-					);
-					s = DEV ? tag(s, to_trace_name(prop)) : s;
+					s = with_parent(() => {
+						var p = has ? proxy(target[prop]) : UNINITIALIZED;
+						var s = source(p, stack);
+
+						if (DEV) {
+							tag(s, get_label(path, prop));
+						}
+
+						return s;
+					});
+
 					sources.set(prop, s);
 				}
 
@@ -231,17 +239,6 @@ export function proxy(value, path, path_preservation = PROXY_PRESERVE_PATH) {
 		},
 
 		set(target, prop, value, receiver) {
-			if (DEV && prop === PROXY_PATH_SYMBOL) {
-				path = value;
-				tag(version, `${path} version`);
-				// rename all child sources and child proxies
-				for (const [prop, source] of sources) {
-					tag(source, to_trace_name(prop));
-					if (typeof source.v === 'object' && source.v !== null && PROXY_PATH_SYMBOL in source.v) {
-						source.v[PROXY_PATH_SYMBOL] = to_trace_name(prop);
-					}
-				}
-			}
 			var s = sources.get(prop);
 			var has = prop in target;
 
@@ -256,8 +253,11 @@ export function proxy(value, path, path_preservation = PROXY_PRESERVE_PATH) {
 						// else a later read of the property would result in a source being created with
 						// the value of the original item at that index.
 						other_s = with_parent(() => source(UNINITIALIZED, stack));
-						other_s = DEV ? tag(other_s, to_trace_name(i)) : other_s;
 						sources.set(i + '', other_s);
+
+						if (DEV) {
+							tag(other_s, get_label(path, i));
+						}
 					}
 				}
 			}
@@ -268,20 +268,23 @@ export function proxy(value, path, path_preservation = PROXY_PRESERVE_PATH) {
 			// object property before writing to that property.
 			if (s === undefined) {
 				if (!has || get_descriptor(target, prop)?.writable) {
-					s = with_parent(() => source(undefined, stack));
-					s = DEV ? tag(s, to_trace_name(prop)) : s;
-					set(
-						s,
-						with_parent(() => proxy(value, to_trace_name(prop), PROXY_CHANGE_PATH))
-					);
+					s = with_parent(() => {
+						var s = source(undefined, stack);
+						set(s, proxy(value));
+						return s;
+					});
+
 					sources.set(prop, s);
+
+					if (DEV) {
+						tag(s, get_label(path, prop));
+					}
 				}
 			} else {
 				has = s.v !== UNINITIALIZED;
-				set(
-					s,
-					with_parent(() => proxy(value, to_trace_name(prop), PROXY_CHANGE_PATH))
-				);
+
+				var p = with_parent(() => proxy(value));
+				set(s, p);
 			}
 
 			var descriptor = Reflect.getOwnPropertyDescriptor(target, prop);
@@ -334,6 +337,16 @@ export function proxy(value, path, path_preservation = PROXY_PRESERVE_PATH) {
 	});
 }
 
+/**
+ * @param {string} path
+ * @param {string | symbol} prop
+ */
+function get_label(path, prop) {
+	if (typeof prop === 'symbol') return `${path}[Symbol(${prop.description ?? ''})]`;
+	if (regex_is_valid_identifier.test(prop)) return `${path}.${prop}`;
+	return /^\d+$/.test(prop) ? `${path}[${prop}]` : `${path}['${prop}']`;
+}
+
 /**
  * @param {Source<number>} signal
  * @param {1 | -1} [d]

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

@@ -27,12 +27,11 @@ import {
 	UNOWNED,
 	MAYBE_DIRTY,
 	BLOCK_EFFECT,
-	ROOT_EFFECT,
-	PROXY_REMOVE_PATH
+	ROOT_EFFECT
 } from '#client/constants';
 import * as e from '../errors.js';
 import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js';
-import { get_stack } from '../dev/tracing.js';
+import { get_stack, tag_proxy } from '../dev/tracing.js';
 import { component_context, is_runes } from '../context.js';
 import { proxy } from '../proxy.js';
 import { execute_derived } from './deriveds.js';
@@ -140,9 +139,11 @@ export function set(source, value, should_proxy = false) {
 		e.state_unsafe_mutation();
 	}
 
-	let new_value = should_proxy
-		? proxy(value, DEV ? source.trace_name : undefined, DEV ? PROXY_REMOVE_PATH : undefined)
-		: value;
+	let new_value = should_proxy ? proxy(value) : value;
+
+	if (DEV) {
+		tag_proxy(new_value, /** @type {string} */ (source.label));
+	}
 
 	return internal_set(source, new_value);
 }

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

@@ -21,7 +21,7 @@ export interface Value<V = unknown> extends Signal {
 	updated?: Error | null;
 	trace_need_increase?: boolean;
 	trace_v?: V;
-	trace_name?: string;
+	label?: string;
 	debug?: null | (() => void);
 }