feat: compiler-driven each block reactivity (#12744)

* WIP

* note to self

* WIP

* fix

* fix

* delete unwrap and is_signal

* simplify

* remove some junk

* regenerate

* reinstate key-is-item optimisation

packages/svelte/src/compiler/phases/2-analyze/visitors/EachBlock.js

@@ -29,6 +29,7 @@ export function EachBlock(node, context) {
 	// evaluate expression in parent scope
 	context.visit(node.expression, {
 		...context.state,
+		expression: node.metadata.expression,
 		scope: /** @type {Scope} */ (context.state.scope.parent)
 	});
 

packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js

@@ -115,10 +115,10 @@ export function Identifier(node, context) {
 		}
 	}
 
-	if (binding && binding.kind !== 'normal') {
+	if (binding) {
 		if (context.state.expression) {
 			context.state.expression.dependencies.add(binding);
-			context.state.expression.has_state = true;
+			context.state.expression.has_state ||= binding.kind !== 'normal';
 		}
 
 		if (

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

@@ -269,20 +269,6 @@ export function should_proxy_or_freeze(node, scope) {
 	return true;
 }
 
-/**
- * Port over the location information from the source to the target identifier.
- * but keep the target as-is (i.e. a new id is created).
- * This ensures esrap can generate accurate source maps.
- * @param {Identifier} target
- * @param {Identifier} source
- */
-export function with_loc(target, source) {
-	if (source.loc) {
-		return { ...target, loc: source.loc };
-	}
-	return target;
-}
-
 /**
  * @param {Pattern} node
  * @param {import('zimmerframe').Context<SvelteNode, ComponentClientTransformState>} context

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

@@ -13,7 +13,7 @@ import {
 import { dev } from '../../../../state.js';
 import { extract_paths, object } from '../../../../utils/ast.js';
 import * as b from '../../../../utils/builders.js';
-import { build_getter, with_loc } from '../utils.js';
+import { build_getter } from '../utils.js';
 import { get_value } from './shared/declarations.js';
 
 /**
@@ -48,18 +48,26 @@ export function EachBlock(node, context) {
 		if (node.index) {
 			flags |= EACH_INDEX_REACTIVE;
 		}
+	}
 
-		// In runes mode, if key === item, we don't need to wrap the item in a source
+	const key_is_item =
-		const key_is_item =
+		node.key?.type === 'Identifier' &&
-			/** @type {Expression} */ (node.key).type === 'Identifier' &&
+		node.context.type === 'Identifier' &&
-			node.context.type === 'Identifier' &&
+		node.context.name === node.key.name;
-			node.context.name === node.key.name;
+
+	for (const binding of node.metadata.expression.dependencies) {
+		// if the expression doesn't reference any external state, we don't need to
+		// create a source for the item. TODO cover more cases (e.g. `x.filter(y)`
+		// should also qualify if `y` doesn't reference state, and non-state
+		// bindings should also be fine
+		if (binding.scope.function_depth >= context.state.scope.function_depth) {
+			continue;
+		}
 
-		if (!context.state.analysis.runes || !key_is_item) {
+		if (!context.state.analysis.runes || !key_is_item || binding.kind === 'store_sub') {
 			flags |= EACH_ITEM_REACTIVE;
+			break;
 		}
-	} else {
-		flags |= EACH_ITEM_REACTIVE;
 	}
 
 	// Since `animate:` can only appear on elements that are the sole child of a keyed each block,
@@ -134,21 +142,13 @@ export function EachBlock(node, context) {
 	const index =
 		each_node_meta.contains_group_binding || !node.index ? each_node_meta.index : b.id(node.index);
 	const item = each_node_meta.item;
-	const binding = /** @type {Binding} */ (context.state.scope.get(item.name));
-	const getter = (/** @type {Identifier} */ id) => {
-		const item_with_loc = with_loc(item, id);
-		return b.call('$.unwrap', item_with_loc);
-	};
 
 	if (node.index) {
-		child_state.transform[node.index] = {
+		if ((flags & EACH_INDEX_REACTIVE) !== 0) {
-			read: (id) => {
+			child_state.transform[node.index] = { read: get_value };
-				const index_with_loc = with_loc(index, id);
+		} else {
-				return (flags & EACH_INDEX_REACTIVE) === 0
+			delete child_state.transform[node.index];
-					? index_with_loc
+		}
-					: b.call('$.get', index_with_loc);
-			}
-		};
 
 		delete key_state.transform[node.index];
 	}
@@ -172,7 +172,7 @@ export function EachBlock(node, context) {
 
 	if (node.context.type === 'Identifier') {
 		child_state.transform[node.context.name] = {
-			read: getter,
+			read: (flags & EACH_ITEM_REACTIVE) !== 0 ? get_value : (node) => node,
 			assign: (_, value) => {
 				const left = b.member(
 					each_node_meta.array_name ? b.call(each_node_meta.array_name) : collection,
@@ -187,12 +187,12 @@ export function EachBlock(node, context) {
 
 		delete key_state.transform[node.context.name];
 	} else {
-		const unwrapped = getter(binding.node);
+		const unwrapped = (flags & EACH_ITEM_REACTIVE) !== 0 ? b.call('$.get', item) : item;
-		const paths = extract_paths(node.context);
 
-		for (const path of paths) {
+		for (const path of extract_paths(node.context)) {
 			const name = /** @type {Identifier} */ (path.node).name;
 			const needs_derived = path.has_default_value; // to ensure that default value is only called once
+
 			const fn = b.thunk(
 				/** @type {Expression} */ (context.visit(path.expression?.(unwrapped), child_state))
 			);
@@ -203,11 +203,11 @@ export function EachBlock(node, context) {
 
 			child_state.transform[name] = {
 				read,
-				assign: (node, value) => {
+				assign: (_, value) => {
 					const left = /** @type {Pattern} */ (path.update_expression(unwrapped));
 					return b.sequence([b.assignment('=', left, value), ...sequence]);
 				},
-				mutate: (node, mutation) => {
+				mutate: (_, mutation) => {
 					return b.sequence([mutation, ...sequence]);
 				}
 			};

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

@@ -9,15 +9,11 @@ import { build_hoisted_params } from '../../utils.js';
 export const visit_function = (node, context) => {
 	const metadata = node.metadata;
 
-	let state = context.state;
+	let state = { ...context.state, in_constructor: false };
 
 	if (node.type === 'FunctionExpression') {
 		const parent = /** @type {Node} */ (context.path.at(-1));
-		const in_constructor = parent.type === 'MethodDefinition' && parent.kind === 'constructor';
+		state.in_constructor = parent.type === 'MethodDefinition' && parent.kind === 'constructor';
-
-		state = { ...context.state, in_constructor };
-	} else {
-		state = { ...context.state, in_constructor: false };
 	}
 
 	if (metadata?.hoisted === true) {

packages/svelte/src/compiler/phases/scope.js

@@ -3,6 +3,7 @@
 /** @import { AnimateDirective, Binding, Component, DeclarationKind, EachBlock, ElementLike, LetDirective, SvelteComponent, SvelteNode, SvelteSelf, TransitionDirective, UseDirective } from '#compiler' */
 import is_reference from 'is-reference';
 import { walk } from 'zimmerframe';
+import { create_expression_metadata } from './nodes.js';
 import * as b from '../utils/builders.js';
 import * as e from '../errors.js';
 import {
@@ -574,6 +575,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
 			}
 
 			node.metadata = {
+				expression: create_expression_metadata(),
 				keyed: false,
 				contains_group_binding: false,
 				array_name: needs_array_deduplication ? state.scope.root.unique('$$array') : null,

packages/svelte/src/compiler/types/template.d.ts

@@ -396,6 +396,7 @@ export interface EachBlock extends BaseNode {
 	index?: string;
 	key?: Expression;
 	metadata: {
+		expression: ExpressionMetadata;
 		keyed: boolean;
 		contains_group_binding: boolean;
 		/** Set if something in the array expression is shadowed within the each block */

packages/svelte/src/internal/client/dom/blocks/each.js

@@ -149,11 +149,6 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f
 			!(STATE_SYMBOL in array)
 		) {
 			flags ^= EACH_IS_STRICT_EQUALS;
-
-			// Additionally if we're in an keyed each block, we'll need ensure the items are all wrapped in signals.
-			if ((flags & EACH_KEYED) !== 0 && (flags & EACH_ITEM_REACTIVE) === 0) {
-				flags ^= EACH_ITEM_REACTIVE;
-			}
 		}
 
 		/** `true` if there was a hydration mismatch. Needs to be a `let` or else it isn't treeshaken out */

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

@@ -137,7 +137,6 @@ export {
 	exclude_from_object,
 	pop,
 	push,
-	unwrap,
 	deep_read,
 	deep_read_state,
 	getAllContexts,

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

@@ -845,17 +845,6 @@ export function set_signal_status(signal, status) {
 	signal.f = (signal.f & STATUS_MASK) | status;
 }
 
-/**
- * @template V
- * @param {V | Value<V>} val
- * @returns {val is Value<V>}
- */
-export function is_signal(val) {
-	return (
-		typeof val === 'object' && val !== null && typeof (/** @type {Value<V>} */ (val).f) === 'number'
-	);
-}
-
 /**
  * Retrieves the context that belongs to the closest parent component with the specified `key`.
  * Must be called during component initialisation.
@@ -1139,20 +1128,6 @@ export function deep_read(value, visited = new Set()) {
 	}
 }
 
-/**
- * @template V
- * @param {V | Value<V>} value
- * @returns {V}
- */
-export function unwrap(value) {
-	if (is_signal(value)) {
-		// @ts-ignore
-		return get(value);
-	}
-	// @ts-ignore
-	return value;
-}
-
 if (DEV) {
 	/**
 	 * @param {string} rune

packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js

@@ -5,10 +5,10 @@ export default function Each_string_template($$anchor) {
 	var fragment = $.comment();
 	var node = $.first_child(fragment);
 
-	$.each(node, 1, () => ['foo', 'bar', 'baz'], $.index, ($$anchor, thing, $$index) => {
+	$.each(node, 0, () => ['foo', 'bar', 'baz'], $.index, ($$anchor, thing, $$index) => {
 		var text = $.text();
 
-		$.template_effect(() => $.set_text(text, `${$.unwrap(thing) ?? ""}, `));
+		$.template_effect(() => $.set_text(text, `${thing ?? ""}, `));
 		$.append($$anchor, text);
 	});
 

packages/svelte/types/index.d.ts

@@ -1851,6 +1851,7 @@ declare module 'svelte/compiler' {
 		index?: string;
 		key?: Expression;
 		metadata: {
+			expression: ExpressionMetadata;
 			keyed: boolean;
 			contains_group_binding: boolean;
 			/** Set if something in the array expression is shadowed within the each block */