chore: set `binding.kind` before analysis (#12843)

* analyse exports before walking

* more

* another

* this is unused

* move stuff/tidy up

* this appears to be unnecessary

* this is all unnecessary

* simplify

* simplify

* simplify

* simplify

* move more stuff over

* changeset

* unused

* separate reassignment from mutation

* regenerate

* lint

.changeset/pink-countries-repair.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+chore: set `binding.kind` before analysis

packages/svelte/src/compiler/migrate/index.js

@@ -331,10 +331,7 @@ const instance_script = {
 
 				const binding = /** @type {Compiler.Binding} */ (state.scope.get(declarator.id.name));
 
-				if (
+				if (state.analysis.uses_props && (declarator.init || binding.updated)) {
-					state.analysis.uses_props &&
-					(declarator.init || binding.mutated || binding.reassigned)
-				) {
 					throw new Error(
 						'$$props is used together with named props in a way that cannot be automatically migrated.'
 					);
@@ -350,7 +347,7 @@ const instance_script = {
 							)
 						: '',
 					optional: !!declarator.init,
-					bindable: binding.mutated || binding.reassigned,
+					bindable: binding.updated,
 					...extract_type_and_comment(declarator, state.str, path)
 				});
 				state.props_insertion_point = /** @type {number} */ (declarator.end);

packages/svelte/src/compiler/phases/2-analyze/index.js

@@ -1,13 +1,13 @@
-/** @import { Node, Program } from 'estree' */
+/** @import { Expression, Node, Program } from 'estree' */
-/** @import { Root, Script, SvelteNode, ValidatedCompileOptions, ValidatedModuleCompileOptions } from '#compiler' */
+/** @import { Binding, Root, Script, SvelteNode, ValidatedCompileOptions, ValidatedModuleCompileOptions } from '#compiler' */
 /** @import { AnalysisState, Visitors } from './types' */
 /** @import { Analysis, ComponentAnalysis, Js, ReactiveStatement, Template } from '../types' */
 import { walk } from 'zimmerframe';
 import * as e from '../../errors.js';
 import * as w from '../../warnings.js';
-import { is_text_attribute } from '../../utils/ast.js';
+import { extract_identifiers, is_text_attribute } from '../../utils/ast.js';
 import * as b from '../../utils/builders.js';
-import { Scope, ScopeRoot, create_scopes, get_rune } from '../scope.js';
+import { Scope, ScopeRoot, create_scopes, get_rune, set_scope } from '../scope.js';
 import check_graph_for_cycles from './utils/check_graph_for_cycles.js';
 import { create_attribute } from '../nodes.js';
 import { analyze_css } from './css/css-analyze.js';
@@ -62,6 +62,7 @@ import { Text } from './visitors/Text.js';
 import { TitleElement } from './visitors/TitleElement.js';
 import { UpdateExpression } from './visitors/UpdateExpression.js';
 import { VariableDeclarator } from './visitors/VariableDeclarator.js';
+import is_reference from 'is-reference';
 
 /**
  * @type {Visitors}
@@ -397,6 +398,112 @@ export function analyze_component(root, source, options) {
 		source
 	};
 
+	if (!runes) {
+		// every exported `let` or `var` declaration becomes a prop, everything else becomes an export
+		for (const node of instance.ast.body) {
+			if (node.type !== 'ExportNamedDeclaration') continue;
+
+			analysis.needs_props = true;
+
+			if (node.declaration) {
+				if (
+					node.declaration.type === 'FunctionDeclaration' ||
+					node.declaration.type === 'ClassDeclaration'
+				) {
+					analysis.exports.push({
+						name: /** @type {import('estree').Identifier} */ (node.declaration.id).name,
+						alias: null
+					});
+				} else if (node.declaration.type === 'VariableDeclaration') {
+					if (node.declaration.kind === 'const') {
+						for (const declarator of node.declaration.declarations) {
+							for (const node of extract_identifiers(declarator.id)) {
+								analysis.exports.push({ name: node.name, alias: null });
+							}
+						}
+					} else {
+						for (const declarator of node.declaration.declarations) {
+							for (const id of extract_identifiers(declarator.id)) {
+								const binding = /** @type {Binding} */ (instance.scope.get(id.name));
+								binding.kind = 'bindable_prop';
+							}
+						}
+					}
+				}
+			} else {
+				for (const specifier of node.specifiers) {
+					const binding = instance.scope.get(specifier.local.name);
+
+					if (
+						binding &&
+						(binding.declaration_kind === 'var' || binding.declaration_kind === 'let')
+					) {
+						binding.kind = 'bindable_prop';
+
+						if (specifier.exported.name !== specifier.local.name) {
+							binding.prop_alias = specifier.exported.name;
+						}
+					} else {
+						analysis.exports.push({ name: specifier.local.name, alias: specifier.exported.name });
+					}
+				}
+			}
+		}
+
+		// if reassigned/mutated bindings are referenced in `$:` blocks
+		// or the template, turn them into state
+		for (const binding of instance.scope.declarations.values()) {
+			if (binding.kind !== 'normal') continue;
+
+			for (const { node, path } of binding.references) {
+				if (node === binding.node) continue;
+
+				if (binding.updated) {
+					if (
+						path[path.length - 1].type === 'StyleDirective' ||
+						path.some((node) => node.type === 'Fragment') ||
+						(path[1].type === 'LabeledStatement' && path[1].label.name === '$')
+					) {
+						binding.kind = 'state';
+					}
+				}
+			}
+		}
+
+		// more legacy nonsense: if an `each` binding is reassigned/mutated,
+		// treat the expression as being mutated as well
+		walk(/** @type {SvelteNode} */ (template.ast), null, {
+			EachBlock(node) {
+				const scope = /** @type {Scope} */ (template.scopes.get(node));
+
+				for (const binding of scope.declarations.values()) {
+					if (binding.updated) {
+						const state = { scope: /** @type {Scope} */ (scope.parent), scopes: template.scopes };
+
+						walk(node.expression, state, {
+							// @ts-expect-error
+							_: set_scope,
+							Identifier(node, context) {
+								const parent = /** @type {Expression} */ (context.path.at(-1));
+
+								if (is_reference(node, parent)) {
+									const binding = context.state.scope.get(node.name);
+
+									if (binding && binding.kind === 'normal') {
+										binding.kind = 'state';
+										binding.mutated = binding.updated = true;
+									}
+								}
+							}
+						});
+
+						break;
+					}
+				}
+			}
+		});
+	}
+
 	if (root.options) {
 		for (const attribute of root.options.attributes) {
 			if (attribute.name === 'accessors') {

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

@@ -134,7 +134,7 @@ function get_delegated_event(event_name, handler, context) {
 			return unhoisted;
 		}
 
-		if (binding !== null && binding.initial !== null && !binding.mutated && !binding.is_called) {
+		if (binding !== null && binding.initial !== null && !binding.updated && !binding.is_called) {
 			const binding_type = binding.initial.type;
 
 			if (
@@ -188,7 +188,7 @@ function get_delegated_event(event_name, handler, context) {
 				(((!context.state.analysis.runes && binding.kind === 'each') ||
 					// or any normal not reactive bindings that are mutated.
 					binding.kind === 'normal') &&
-					binding.mutated))
+					binding.updated))
 		) {
 			return unhoisted;
 		}

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

@@ -39,7 +39,7 @@ export function BindDirective(node, context) {
 					binding.kind !== 'bindable_prop' &&
 					binding.kind !== 'each' &&
 					binding.kind !== 'store_sub' &&
-					!binding.mutated))
+					!binding.updated)) // TODO wut?
 		) {
 			e.bind_invalid_value(node.expression);
 		}
@@ -80,10 +80,6 @@ export function BindDirective(node, context) {
 				if (references.length > 0) {
 					parent.metadata.contains_group_binding = true;
 
-					for (const binding of parent.metadata.references) {
-						binding.mutated = true;
-					}
-
 					each_blocks.push(parent);
 					ids = ids.filter((id) => !references.includes(id));
 					ids.push(...extract_all_identifiers_from_expression(parent.expression)[1]);

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

@@ -39,37 +39,6 @@ export function ExportNamedDeclaration(node, context) {
 		}
 	}
 
-	if (context.state.ast_type === 'instance' && !context.state.analysis.runes) {
-		context.state.analysis.needs_props = true;
-
-		if (node.declaration) {
-			if (
-				node.declaration.type === 'FunctionDeclaration' ||
-				node.declaration.type === 'ClassDeclaration'
-			) {
-				context.state.analysis.exports.push({
-					name: /** @type {Identifier} */ (node.declaration.id).name,
-					alias: null
-				});
-			} else if (node.declaration.type === 'VariableDeclaration') {
-				if (node.declaration.kind === 'const') {
-					for (const declarator of node.declaration.declarations) {
-						for (const node of extract_identifiers(declarator.id)) {
-							context.state.analysis.exports.push({ name: node.name, alias: null });
-						}
-					}
-				} else {
-					for (const declarator of node.declaration.declarations) {
-						for (const id of extract_identifiers(declarator.id)) {
-							const binding = /** @type {Binding} */ (context.state.scope.get(id.name));
-							binding.kind = 'bindable_prop';
-						}
-					}
-				}
-			}
-		}
-	}
-
 	if (context.state.analysis.runes) {
 		if (node.declaration && context.state.ast_type === 'instance') {
 			if (

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

@@ -17,29 +17,7 @@ export function ExportSpecifier(node, context) {
 			});
 
 			const binding = context.state.scope.get(node.local.name);
-			if (binding) binding.reassigned = true;
+			if (binding) binding.reassigned = binding.updated = true;
-		} else {
-			context.state.analysis.needs_props = true;
-
-			const binding = /** @type {Binding} */ (context.state.scope.get(node.local.name));
-
-			if (
-				binding !== null &&
-				(binding.kind === 'state' ||
-					binding.kind === 'raw_state' ||
-					(binding.kind === 'normal' &&
-						(binding.declaration_kind === 'let' || binding.declaration_kind === 'var')))
-			) {
-				binding.kind = 'bindable_prop';
-				if (node.exported.name !== node.local.name) {
-					binding.prop_alias = node.exported.name;
-				}
-			} else {
-				context.state.analysis.exports.push({
-					name: node.local.name,
-					alias: node.exported.name
-				});
-			}
 		}
 	} else {
 		validate_export(node, context.state.scope, node.local.name);

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

@@ -1,4 +1,5 @@
 /** @import { Expression, Identifier } from 'estree' */
+/** @import { EachBlock } from '#compiler' */
 /** @import { Context } from '../types' */
 import is_reference from 'is-reference';
 import { should_proxy } from '../../3-transform/client/utils.js';
@@ -77,46 +78,6 @@ export function Identifier(node, context) {
 		if (node.name === '$$restProps') {
 			context.state.analysis.uses_rest_props = true;
 		}
-
-		if (
-			binding?.kind === 'normal' &&
-			((binding.scope === context.state.instance_scope &&
-				binding.declaration_kind !== 'function') ||
-				binding.declaration_kind === 'import')
-		) {
-			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')
-			) {
-				binding.kind = 'state';
-			} else if (
-				context.state.reactive_statement &&
-				parent.type === 'AssignmentExpression' &&
-				parent.left === binding.node
-			) {
-				binding.kind = 'derived';
-			}
-		} else if (binding?.kind === 'each' && binding.mutated) {
-			// Ensure that the array is marked as reactive even when only its entries are mutated
-			let i = context.path.length;
-			while (i--) {
-				const ancestor = context.path[i];
-				if (
-					ancestor.type === 'EachBlock' &&
-					context.state.analysis.template.scopes.get(ancestor)?.declarations.get(node.name) ===
-						binding
-				) {
-					for (const binding of ancestor.metadata.references) {
-						if (binding.kind === 'normal') {
-							binding.kind = 'state';
-						}
-					}
-					break;
-				}
-			}
-		}
 	}
 
 	if (binding) {

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

@@ -17,10 +17,6 @@ export function StyleDirective(node, context) {
 		let binding = context.state.scope.get(node.name);
 
 		if (binding) {
-			if (!context.state.analysis.runes && binding.mutated) {
-				binding.kind = 'state';
-			}
-
 			if (binding.kind !== 'normal') {
 				node.metadata.expression.has_state = true;
 			}

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

@@ -235,7 +235,7 @@ export function is_prop_source(binding, state) {
 			binding.initial ||
 			// Until legacy mode is gone, we also need to use the prop source when only mutated is true,
 			// because the parent could be a legacy component which needs coarse-grained reactivity
-			binding.mutated)
+			binding.updated)
 	);
 }
 

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

@@ -114,7 +114,7 @@ export function EachBlock(node, context) {
 	const indirect_dependencies = collect_parent_each_blocks(context).flatMap((block) => {
 		const array = /** @type {Expression} */ (context.visit(block.expression));
 		const transitive_dependencies = build_transitive_dependencies(
-			block.metadata.references,
+			block.metadata.expression.dependencies,
 			context
 		);
 		return [array, ...transitive_dependencies];
@@ -126,7 +126,7 @@ export function EachBlock(node, context) {
 		indirect_dependencies.push(collection);
 
 		const transitive_dependencies = build_transitive_dependencies(
-			each_node_meta.references,
+			each_node_meta.expression.dependencies,
 			context
 		);
 		indirect_dependencies.push(...transitive_dependencies);
@@ -279,7 +279,7 @@ function collect_parent_each_blocks(context) {
 }
 
 /**
- * @param {Binding[]} references
+ * @param {Set<Binding>} references
  * @param {ComponentContext} context
  */
 function build_transitive_dependencies(references, context) {

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

@@ -113,13 +113,14 @@ export class Scope {
 			references: [],
 			legacy_dependencies: [],
 			initial,
+			reassigned: false,
 			mutated: false,
+			updated: false,
 			scope: this,
 			kind,
 			declaration_kind,
 			is_called: false,
 			prop_alias: null,
-			reassigned: false,
 			metadata: null
 		};
 		this.declarations.set(node.name, binding);
@@ -506,17 +507,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
 		},
 
 		EachBlock(node, { state, visit }) {
-			// Array part is still from the scope above
-			/** @type {Set<Identifier>} */
-			const references_within = new Set();
-			const idx = references.length;
 			visit(node.expression);
-			for (let i = idx; i < references.length; i++) {
-				const [scope, { node: id }] = references[i];
-				if (scope === state.scope) {
-					references_within.add(id);
-				}
-			}
 
 			// context and children are a new scope
 			const scope = state.scope.child();
@@ -582,9 +573,6 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
 				index: scope.root.unique('$$index'),
 				item: node.context.type === 'Identifier' ? node.context : b.id('$$item'),
 				declarations: scope.declarations,
-				references: [...references_within]
-					.map((id) => /** @type {Binding} */ (state.scope.get(id.name)))
-					.filter(Boolean),
 				is_controlled: false
 			};
 		},
@@ -671,7 +659,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
 		// the css property
 		StyleDirective(node, { path, state, next }) {
 			if (node.value === true) {
-				state.scope.reference(b.id(node.name), path);
+				state.scope.reference(b.id(node.name), path.concat(node));
 			}
 			next();
 		}
@@ -693,25 +681,19 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
 	}
 
 	for (const [scope, node] of updates) {
-		if (node.type === 'MemberExpression') {
+		for (const expression of unwrap_pattern(node)) {
-			let object = node.object;
+			const left = object(expression);
-			while (object.type === 'MemberExpression') {
+			const binding = left && scope.get(left.name);
-				object = object.object;
-			}
 
-			const binding = scope.get(/** @type {Identifier} */ (object).name);
+			if (binding !== null && left !== binding.node) {
-			if (binding) binding.mutated = true;
+				binding.updated = true;
-		} else {
-			unwrap_pattern(node).forEach((node) => {
-				let id = node.type === 'Identifier' ? node : object(node);
-				if (id === null) return;
 
-				const binding = scope.get(id.name);
+				if (left === expression) {
-				if (binding && id !== binding.node) {
+					binding.reassigned = true;
+				} else {
 					binding.mutated = true;
-					binding.reassigned = node.type === 'Identifier';
 				}
-			});
+			}
 		}
 	}
 

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

@@ -300,6 +300,8 @@ export interface Binding {
 	references: { node: Identifier; path: SvelteNode[] }[];
 	mutated: boolean;
 	reassigned: boolean;
+	/** `true` if mutated _or_ reassigned */
+	updated: boolean;
 	scope: Scope;
 	/** For `legacy_reactive`: its reactive dependencies */
 	legacy_dependencies: Binding[];

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

@@ -404,8 +404,6 @@ export interface EachBlock extends BaseNode {
 		index: Identifier;
 		item: Identifier;
 		declarations: Map<string, Binding>;
-		/** List of bindings that are referenced within the expression */
-		references: Binding[];
 		/**
 		 * Optimization path for each blocks: If the parent isn't a fragment and
 		 * it only has a single child, then we can classify the block as being "controlled".

packages/svelte/types/index.d.ts

@@ -949,6 +949,8 @@ declare module 'svelte/compiler' {
 		references: { node: Identifier; path: SvelteNode[] }[];
 		mutated: boolean;
 		reassigned: boolean;
+		/** `true` if mutated _or_ reassigned */
+		updated: boolean;
 		scope: Scope;
 		/** For `legacy_reactive`: its reactive dependencies */
 		legacy_dependencies: Binding[];
@@ -1859,8 +1861,6 @@ declare module 'svelte/compiler' {
 			index: Identifier;
 			item: Identifier;
 			declarations: Map<string, Binding>;
-			/** List of bindings that are referenced within the expression */
-			references: Binding[];
 			/**
 			 * Optimization path for each blocks: If the parent isn't a fragment and
 			 * it only has a single child, then we can classify the block as being "controlled".