chore: transformers (#12780)

* clear out getters on new scope

* fix

* fix

* fix

* fix

* consolidate legacy_reactive_import logic

* unused

* $$sanitized_props

* use getters mechanism for store_sub

* snapshot

* fix

* tests passing

* remove some stuff

* more

* fix

* tidy up

* simplify

* simplify

* getters -> transformers

* update

* use update transformers

* add assign transformer

* more

* tweak

* remove junk

* unused

* simplify

* tidy up

* tweak

* assign_property

* fix

* tidy up

* tidy up

* move store code

* this appears to be unused

* tidy up

* tweak

* simplify

* move code

* move stuff

* note to self

* move stuff

* each blocks

* note to self

* lengthen stack trace

* tweak

* more

* tidy up

* tidy up

* remove some junk

* tidy up

* move stuff

* remove stuff

* tweak

* tweak

* fix

* tweak

* tidy up

* tidy up

* tidy up

* tweak

* simplify

* tidy up

* simplify

* tidy up

* improve output

* delete comments

* more

* unused

* tidy up

* tidy up

* fix

* move some stuff

* tweak

* tidy up

* DRY

* synchronise

* DRY out

* tidy up

* tidy up

* tidy up

* add test that fails on main

* snapshot test

* changesets

* lint

* ugh

Author: Rich-Harris

.changeset/nine-ants-invite.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: invalidate signals following ++/-- inside each block

.changeset/perfect-hairs-matter.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+feat: better code generation for destructuring assignments

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

@@ -187,9 +187,7 @@ function get_delegated_event(event_name, handler, context) {
 				// Bail out if we reference anything from the EachBlock (for now) that mutates in non-runes mode,
 				(((!context.state.analysis.runes && binding.kind === 'each') ||
 					// or any normal not reactive bindings that are mutated.
-					binding.kind === 'normal' ||
-					// or any reactive imports (those are rewritten) (can only happen in legacy mode)
-					binding.kind === 'legacy_reactive_import') &&
+					binding.kind === 'normal') &&
 					binding.mutated))
 		) {
 			return unhoisted;

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

@@ -1,5 +1,6 @@
 /** @import { EachBlock } from '#compiler' */
 /** @import { Context } from '../types' */
+/** @import { Scope } from '../../scope' */
 import * as e from '../../../errors.js';
 import { validate_block_not_empty, validate_opening_tag } from './shared/utils.js';
 
@@ -25,5 +26,13 @@ export function EachBlock(node, context) {
 			node.key.type !== 'Identifier' || !node.index || node.key.name !== node.index;
 	}
 
-	context.next();
+	// evaluate expression in parent scope
+	context.visit(node.expression, {
+		...context.state,
+		scope: /** @type {Scope} */ (context.state.scope.parent)
+	});
+
+	context.visit(node.body);
+	if (node.key) context.visit(node.key);
+	if (node.fallback) context.visit(node.fallback);
 }

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

@@ -80,16 +80,8 @@ export function Identifier(node, context) {
 				binding.declaration_kind !== 'function') ||
 				binding.declaration_kind === 'import')
 		) {
-			if (binding.declaration_kind === 'import') {
-				if (
-					binding.mutated &&
-					// TODO could be more fine-grained - not every mention in the template implies a state binding
-					(context.state.reactive_statement || context.state.ast_type === 'template') &&
-					parent.type === 'MemberExpression'
-				) {
-					binding.kind = 'legacy_reactive_import';
-				}
-			} else if (
+			if (
+				binding.declaration_kind !== 'import' &&
 				binding.mutated &&
 				// TODO could be more fine-grained - not every mention in the template implies a state binding
 				(context.state.reactive_statement || context.state.ast_type === 'template')

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

@@ -1,7 +1,8 @@
-/** @import { Component, SvelteComponent, SvelteSelf } from '#compiler' */
+/** @import { Comment, Component, Fragment, SvelteComponent, SvelteSelf } from '#compiler' */
 /** @import { Context } from '../../types' */
 import * as e from '../../../../errors.js';
 import { get_attribute_expression, is_expression_attribute } from '../../../../utils/ast.js';
+import { determine_slot } from '../../../../utils/slot.js';
 import {
 	validate_attribute,
 	validate_attribute_name,
@@ -60,9 +61,45 @@ export function visit_component(node, context) {
 		}
 	}
 
-	context.next({
-		...context.state,
-		parent_element: null,
-		component_slots: new Set()
-	});
+	// If the component has a slot attribute — `<Foo slot="whatever" .../>` —
+	// then `let:` directives apply to other attributes, instead of just the
+	// top-level contents of the component. Yes, this is very weird.
+	const default_state = determine_slot(node)
+		? context.state
+		: { ...context.state, scope: node.metadata.scopes.default };
+
+	for (const attribute of node.attributes) {
+		context.visit(attribute, attribute.type === 'LetDirective' ? default_state : context.state);
+	}
+
+	/** @type {Comment[]} */
+	let comments = [];
+
+	/** @type {Record<string, Fragment['nodes']>} */
+	const nodes = { default: [] };
+
+	for (const child of node.fragment.nodes) {
+		if (child.type === 'Comment') {
+			comments.push(child);
+			continue;
+		}
+
+		const slot_name = determine_slot(child) ?? 'default';
+		(nodes[slot_name] ??= []).push(...comments, child);
+
+		if (slot_name !== 'default') comments = [];
+	}
+
+	const component_slots = new Set();
+
+	for (const slot_name in nodes) {
+		const state = {
+			...context.state,
+			scope: node.metadata.scopes[slot_name],
+			parent_element: null,
+			component_slots
+		};
+
+		context.visit({ ...node.fragment, nodes: nodes[slot_name] }, state);
+	}
 }

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

@@ -4,7 +4,6 @@
 /** @import { Visitors, ComponentClientTransformState, ClientTransformState } from './types' */
 import { walk } from 'zimmerframe';
 import * as b from '../../../utils/builders.js';
-import { set_scope } from '../../scope.js';
 import { build_getter } from './utils.js';
 import { render_stylesheet } from '../css/index.js';
 import { dev, filename } from '../../../state.js';
@@ -15,6 +14,7 @@ import { Attribute } from './visitors/Attribute.js';
 import { AwaitBlock } from './visitors/AwaitBlock.js';
 import { BinaryExpression } from './visitors/BinaryExpression.js';
 import { BindDirective } from './visitors/BindDirective.js';
+import { BlockStatement } from './visitors/BlockStatement.js';
 import { BreakStatement } from './visitors/BreakStatement.js';
 import { CallExpression } from './visitors/CallExpression.js';
 import { ClassBody } from './visitors/ClassBody.js';
@@ -37,6 +37,7 @@ import { LabeledStatement } from './visitors/LabeledStatement.js';
 import { LetDirective } from './visitors/LetDirective.js';
 import { MemberExpression } from './visitors/MemberExpression.js';
 import { OnDirective } from './visitors/OnDirective.js';
+import { Program } from './visitors/Program.js';
 import { RegularElement } from './visitors/RegularElement.js';
 import { RenderTag } from './visitors/RenderTag.js';
 import { SlotElement } from './visitors/SlotElement.js';
@@ -58,14 +59,31 @@ import { VariableDeclaration } from './visitors/VariableDeclaration.js';
 
 /** @type {Visitors} */
 const visitors = {
-	_: set_scope,
+	_: function set_scope(node, { next, state }) {
+		const scope = state.scopes.get(node);
+
+		if (scope && scope !== state.scope) {
+			const transform = { ...state.transform };
+
+			for (const [name, binding] of scope.declarations) {
+				if (binding.kind === 'normal') {
+					delete transform[name];
+				}
+			}
+
+			next({ ...state, transform, scope });
+		} else {
+			next();
+		}
+	},
 	AnimateDirective,
 	ArrowFunctionExpression,
 	AssignmentExpression,
 	Attribute,
 	AwaitBlock,
 	BinaryExpression,
 	BindDirective,
+	BlockStatement,
 	BreakStatement,
 	CallExpression,
 	ClassBody,
@@ -88,6 +106,7 @@ const visitors = {
 	LetDirective,
 	MemberExpression,
 	OnDirective,
+	Program,
 	RegularElement,
 	RenderTag,
 	SlotElement,
@@ -123,6 +142,7 @@ export function client_component(analysis, options) {
 		is_instance: false,
 		hoisted: [b.import_all('$', 'svelte/internal/client')],
 		node: /** @type {any} */ (null), // populated by the root node
+		legacy_reactive_imports: [],
 		legacy_reactive_statements: new Map(),
 		metadata: {
 			context: {
@@ -136,8 +156,7 @@ export function client_component(analysis, options) {
 		preserve_whitespace: options.preserveWhitespace,
 		public_state: new Map(),
 		private_state: new Map(),
-		getters: {},
-		setters: {},
+		transform: {},
 		in_constructor: false,
 
 		// these are set inside the `Fragment` visitor, and cannot be used until then
@@ -155,6 +174,7 @@ export function client_component(analysis, options) {
 
 	const instance_state = {
 		...state,
+		transform: { ...state.transform },
 		scope: analysis.instance.scope,
 		scopes: analysis.instance.scopes,
 		is_instance: true
@@ -167,21 +187,17 @@ export function client_component(analysis, options) {
 	const template = /** @type {ESTree.Program} */ (
 		walk(
 			/** @type {SvelteNode} */ (analysis.template.ast),
-			{ ...state, scope: analysis.instance.scope, scopes: analysis.template.scopes },
+			{
+				...state,
+				transform: instance_state.transform,
+				scope: analysis.instance.scope,
+				scopes: analysis.template.scopes
+			},
 			visitors
 		)
 	);
 
-	// Very very dirty way of making import statements reactive in legacy mode if needed
-	if (!analysis.runes) {
-		for (const [name, binding] of analysis.module.scope.declarations) {
-			if (binding.kind === 'legacy_reactive_import') {
-				instance.body.unshift(
-					b.var('$$_import_' + name, b.call('$.reactive_import', b.thunk(b.id(name))))
-				);
-			}
-		}
-	}
+	instance.body.unshift(...state.legacy_reactive_imports);
 
 	/** @type {ESTree.Statement[]} */
 	const store_setup = [];
@@ -623,11 +639,9 @@ export function client_module(analysis, options) {
 		options,
 		scope: analysis.module.scope,
 		scopes: analysis.module.scopes,
-		legacy_reactive_statements: new Map(),
 		public_state: new Map(),
 		private_state: new Map(),
-		getters: {},
-		setters: {},
+		transform: {},
 		in_constructor: false
 	};
 

packages/svelte/src/compiler/phases/3-transform/client/types.d.ts

@@ -5,7 +5,8 @@ import type {
 	Identifier,
 	PrivateIdentifier,
 	Expression,
-	AssignmentExpression
+	AssignmentExpression,
+	UpdateExpression
 } from 'estree';
 import type { Namespace, SvelteNode, ValidatedCompileOptions } from '#compiler';
 import type { TransformState } from '../types.js';
@@ -22,19 +23,18 @@ export interface ClientTransformState extends TransformState {
 	 */
 	readonly in_constructor: boolean;
 
-	/** The $: calls, which will be ordered in the end */
-	readonly legacy_reactive_statements: Map<LabeledStatement, Statement>;
-	/**
-	 * A map of `[name, node]` pairs, where `Identifier` nodes matching `name`
-	 * will be replaced with `node` (e.g. `x` -> `$.get(x)`)
-	 */
-	readonly getters: Record<string, Expression | ((id: Identifier) => Expression)>;
-	/**
-	 * Counterpart to `getters`
-	 */
-	readonly setters: Record<
+	readonly transform: Record<
 		string,
-		(assignment: AssignmentExpression, context: Context) => Expression
+		{
+			/** turn `foo` into e.g. `$.get(foo)` */
+			read: (id: Identifier) => Expression;
+			/** turn `foo = bar` into e.g. `$.set(foo, bar)` */
+			assign?: (node: Identifier, value: Expression) => Expression;
+			/** turn `foo.bar = baz` into e.g. `$.mutate(foo, $.get(foo).bar = baz);` */
+			mutate?: (node: Identifier, mutation: AssignmentExpression) => Expression;
+			/** turn `foo++` into e.g. `$.update(foo)` */
+			update?: (node: UpdateExpression) => Expression;
+		}
 	>;
 }
 
@@ -79,6 +79,12 @@ export interface ComponentClientTransformState extends ClientTransformState {
 
 	/** The anchor node for the current context */
 	readonly node: Identifier;
+
+	/** Imports that should be re-evaluated in legacy mode following a mutation */
+	readonly legacy_reactive_imports: Statement[];
+
+	/** The $: calls, which will be ordered in the end */
+	readonly legacy_reactive_statements: Map<LabeledStatement, Statement>;
 }
 
 export interface StateField {