track mutability in variable objects, not mutable_props

src/compile/Component.ts

@@ -77,7 +77,6 @@ export default class Component {
 	reactive_declarations: Array<{ assignees: Set<string>, dependencies: Set<string>, snippet: string }> = [];
 	reactive_declaration_nodes: Set<Node> = new Set();
 	has_reactive_assignments = false;
-	mutable_props: Set<string> = new Set();
 
 	indirectDependencies: Map<string, Set<string>> = new Map();
 	template_references: Set<string> = new Set();
@@ -152,7 +151,6 @@ export default class Component {
 		if (!ast.instance && !ast.module) {
 			const props = [...this.template_references];
 			this.declarations.push(...props);
-			addToSet(this.mutable_props, this.template_references);
 			addToSet(this.writable_declarations, this.template_references);
 
 			props.forEach(name => {
@@ -163,7 +161,7 @@ export default class Component {
 					imported_as: null,
 					exported_as: name,
 					source: null,
-					mutated: false,
+					mutated: true, // TODO kind of a misnomer... it's *mutable* but not necessarily *mutated*. is that a problem?
 					referenced: true,
 					module: false
 				});
@@ -174,7 +172,9 @@ export default class Component {
 		}
 
 		// tell the root fragment scope about all of the mutable names we know from the script
-		this.mutable_props.forEach(name => this.fragment.scope.mutables.add(name));
+		this.vars.forEach(variable => {
+			if (variable.mutated) this.fragment.scope.mutables.add(name);
+		});
 	}
 
 	add_var(variable: Var) {
@@ -187,7 +187,7 @@ export default class Component {
 
 		if (variable) {
 			variable.referenced = true;
-		} else if (!this.ast.instance) {
+		} else if (!this.ast.instance || name[0] === '$') {
 			this.add_var({
 				name,
 				kind: 'injected',
@@ -485,11 +485,9 @@ export default class Component {
 									exported_as: name,
 									source: null,
 									module: is_module,
-									mutated: false,
+									mutated: !is_module,
 									referenced: false
 								});
-
-								if (!is_module) this.mutable_props.add(name);
 							});
 						});
 					} else {
@@ -512,7 +510,7 @@ export default class Component {
 							exported_as: name,
 							source: null,
 							module: is_module,
-							mutated: false,
+							mutated: !is_module,
 							referenced: false
 						});
 					}
@@ -587,6 +585,33 @@ export default class Component {
 					message: `The $ prefix is reserved, and cannot be used for variable and import names`
 				});
 			}
+
+			if (!/Import/.test(node.type)) {
+				const kind = node.type === 'VariableDeclaration'
+					? node.kind
+					: node.type === 'ClassDeclaration'
+						? 'class'
+						: node.type === 'FunctionDeclaration'
+							? 'function'
+							: null;
+
+				// sanity check
+				if (!kind) throw new Error(`Unknown declaration type ${node.type}`);
+
+				this.add_var({
+					name,
+					kind,
+					import_type: null,
+					imported_as: null,
+					exported_as: null,
+					source: null,
+					module: false,
+					mutated: false,
+					referenced: false
+				});
+
+				this.declarations.push(name);
+			}
 		});
 
 		this.extract_imports(script.content, true);
@@ -666,9 +691,9 @@ export default class Component {
 			});
 		});
 
-		this.track_mutations();
 		this.extract_imports(script.content, false);
 		this.extract_exports(script.content, false);
+		this.track_mutations();
 
 		// TODO remove this, just use component.symbols everywhere
 		this.props = this.vars.filter(variable => !variable.module && variable.exported_as).map(variable => ({
@@ -690,7 +715,9 @@ export default class Component {
 	// TODO merge this with other walks that are independent
 	track_mutations() {
 		const component = this;
-		let { instance_scope: scope, instance_scope_map: map } = this;
+		const { instance_scope, instance_scope_map: map } = this;
+
+		let scope = instance_scope;
 
 		walk(this.ast.instance.content, {
 			enter(node, parent) {
@@ -701,17 +728,26 @@ export default class Component {
 
 				if (node.type === 'AssignmentExpression') {
 					names = node.left.type === 'MemberExpression'
-							? [getObject(node.left).name]
+						? [getObject(node.left).name]
-							: extractNames(node.left);
+						: extractNames(node.left);
 				} else if (node.type === 'UpdateExpression') {
 					names = [getObject(node.argument).name];
 				}
 
 				if (names) {
 					names.forEach(name => {
-						if (scope.has(name)) component.mutable_props.add(name);
+						if (scope.findOwner(name) === instance_scope) {
+							const variable = component.var_lookup.get(name);
+							variable.mutated = true;
+						}
 					});
 				}
+			},
+
+			leave(node) {
+				if (map.has(node)) {
+					scope = scope.parent;
+				}
 			}
 		})
 	}
@@ -758,7 +794,6 @@ export default class Component {
 		const exported = new Set();
 		this.props.forEach(prop => {
 			exported.add(prop.name);
-			this.mutable_props.add(prop.name);
 		});
 
 		const coalesced_declarations = [];
@@ -898,7 +933,7 @@ export default class Component {
 
 		this.ast.instance.content.body.forEach(node => {
 			if (node.type === 'VariableDeclaration') {
-				if (node.declarations.every(d => d.init && d.init.type === 'Literal' && !this.mutable_props.has(d.id.name) && !template_scope.containsMutable([d.id.name]))) {
+				if (node.declarations.every(d => d.init && d.init.type === 'Literal' && !this.var_lookup.get(d.id.name).mutated && !template_scope.containsMutable([d.id.name]))) {
 					node.declarations.forEach(d => {
 						hoistable_names.add(d.id.name);
 					});

src/compile/nodes/Attribute.ts

@@ -34,7 +34,7 @@ export default class Attribute extends Node {
 			this.isSynthetic = false;
 
 			this.expression = new Expression(component, this, scope, info.expression);
-			this.dependencies = this.expression.dynamic_dependencies;
+			this.dependencies = this.expression.dependencies;
 			this.chunks = null;
 
 			this.isDynamic = true; // TODO not necessarily
@@ -59,7 +59,7 @@ export default class Attribute extends Node {
 
 					const expression = new Expression(component, this, scope, node.expression);
 
-					addToSet(this.dependencies, expression.dynamic_dependencies);
+					addToSet(this.dependencies, expression.dependencies);
 					return expression;
 				});
 
@@ -73,6 +73,19 @@ export default class Attribute extends Node {
 		}
 	}
 
+	get_dependencies() {
+		if (this.isSpread) return this.expression.dynamic_dependencies();
+
+		const dependencies = new Set();
+		this.chunks.forEach(chunk => {
+			if (chunk.type === 'Expression') {
+				addToSet(dependencies, chunk.dynamic_dependencies());
+			}
+		});
+
+		return [...dependencies];
+	}
+
 	getValue() {
 		if (this.isTrue) return true;
 		if (this.chunks.length === 0) return `""`;

src/compile/nodes/Binding.ts

@@ -2,6 +2,7 @@ import Node from './shared/Node';
 import getObject from '../../utils/getObject';
 import Expression from './shared/Expression';
 import Component from '../Component';
+import TemplateScope from './shared/TemplateScope';
 
 export default class Binding extends Node {
 	name: string;
@@ -10,7 +11,7 @@ export default class Binding extends Node {
 	obj: string;
 	prop: string;
 
-	constructor(component: Component, parent, scope, info) {
+	constructor(component: Component, parent, scope: TemplateScope, info) {
 		super(component, parent, scope, info);
 
 		if (info.expression.type !== 'Identifier' && info.expression.type !== 'MemberExpression') {
@@ -30,7 +31,15 @@ export default class Binding extends Node {
 		this.isContextual = scope.names.has(name);
 
 		// make sure we track this as a mutable ref
-		scope.setMutable(name);
+		if (this.isContextual) {
+			scope.dependenciesForName.get(name).forEach(name => {
+				const variable = component.var_lookup.get(name);
+				variable.mutated = true;
+			});
+		} else {
+			const variable = component.var_lookup.get(name);
+			variable.mutated = true;
+		}
 
 		if (this.expression.node.type === 'MemberExpression') {
 			prop = `[✂${this.expression.node.property.start}-${this.expression.node.property.end}✂]`;

src/compile/nodes/shared/Expression.ts

@@ -62,6 +62,7 @@ const precedence: Record<string, (node?: Node) => number> = {
 };
 
 export default class Expression {
+	type = 'Expression';
 	component: Component;
 	owner: Wrapper;
 	node: any;
@@ -69,7 +70,6 @@ export default class Expression {
 	references: Set<string>;
 	dependencies: Set<string> = new Set();
 	contextual_dependencies: Set<string> = new Set();
-	dynamic_dependencies: Set<string> = new Set();
 
 	template_scope: TemplateScope;
 	scope: Scope;
@@ -95,7 +95,7 @@ export default class Expression {
 		this.owner = owner;
 		this.is_synthetic = owner.isSynthetic;
 
-		const { dependencies, contextual_dependencies, dynamic_dependencies } = this;
+		const { dependencies, contextual_dependencies } = this;
 
 		let { map, scope } = createScopes(info);
 		this.scope = scope;
@@ -104,27 +104,6 @@ export default class Expression {
 		const expression = this;
 		let function_expression;
 
-		function add_dependency(name, deep = false) {
-			dependencies.add(name);
-
-			if (!function_expression) {
-				// dynamic_dependencies is used to create `if (changed.foo || ...)`
-				// conditions — it doesn't apply if the dependency is inside a
-				// function, and it only applies if the dependency is writable
-				// or a sub-path of a non-writable
-				if (component.ast.instance) {
-					const owner = template_scope.getOwner(name);
-					const is_let = owner && (owner.type === 'InlineComponent' || owner.type === 'Element');
-
-					if (is_let || component.writable_declarations.has(name) || name[0] === '$' || (component.var_lookup.has(name) && deep)) {
-						dynamic_dependencies.add(name);
-					}
-				} else {
-					dynamic_dependencies.add(name);
-				}
-			}
-		}
-
 		// discover dependencies, but don't change the code yet
 		walk(info, {
 			enter(node: any, parent: any, key: string) {
@@ -150,11 +129,15 @@ export default class Expression {
 
 						contextual_dependencies.add(name);
 
-						template_scope.dependenciesForName.get(name).forEach(name => add_dependency(name, true));
+						if (!function_expression) {
+							template_scope.dependenciesForName.get(name).forEach(name => dependencies.add(name));
+						}
 					} else {
-						add_dependency(name, nodes.length > 1);
+						if (!function_expression) {
-						component.add_reference(name);
+							dependencies.add(name);
+						}
 
+						component.add_reference(name);
 						component.warn_if_undefined(nodes[0], template_scope, true);
 					}
 
@@ -162,17 +145,33 @@ export default class Expression {
 				}
 
 				// track any assignments from template expressions as mutable
+				let mutated;
 				if (function_expression) {
 					if (node.type === 'AssignmentExpression') {
-						const names = node.left.type === 'MemberExpression'
+						mutated = node.left.type === 'MemberExpression'
 							? [getObject(node.left).name]
 							: extractNames(node.left);
-						names.forEach(name => template_scope.setMutable(name));
 					} else if (node.type === 'UpdateExpression') {
 						const { name } = getObject(node.argument);
-						template_scope.setMutable(name);
+						mutated = [name];
 					}
 				}
+
+				if (mutated) {
+					mutated.forEach(name => {
+						if (template_scope.names.has(name)) {
+							template_scope.dependenciesForName.get(name).forEach(name => {
+								const variable = component.var_lookup.get(name);
+								if (variable) variable.mutated = true;
+							});
+						} else {
+							component.add_reference(name);
+
+							const variable = component.var_lookup.get(name);
+							if (variable) variable.mutated = true;
+						}
+					});
+				}
 			},
 
 			leave(node) {
@@ -187,6 +186,17 @@ export default class Expression {
 		});
 	}
 
+	dynamic_dependencies() {
+		return Array.from(this.dependencies).filter(name => {
+			const owner = this.template_scope.getOwner(name);
+			const is_let = owner && (owner.type === 'InlineComponent' || owner.type === 'Element');
+			if (is_let) return true;
+
+			const variable = this.component.var_lookup.get(name);
+			return variable.mutated;
+		});
+	}
+
 	getPrecedence() {
 		return this.node.type in precedence ? precedence[this.node.type](this.node) : 0;
 	}

src/compile/nodes/shared/TemplateScope.ts

@@ -31,14 +31,6 @@ export default class TemplateScope {
 		return child;
 	}
 
-	setMutable(name: string) {
-		if (this.names.has(name)) {
-			this.mutables.add(name);
-			if (this.parent && this.dependenciesForName.has(name)) this.dependenciesForName.get(name).forEach(dep => this.parent.setMutable(dep));
-		} else if (this.parent) this.parent.setMutable(name);
-		else this.mutables.add(name);
-	}
-
 	containsMutable(names: Iterable<string>) {
 		for (const name of names) {
 			const owner = this.getOwner(name);

src/compile/render-dom/wrappers/AwaitBlock.ts

@@ -67,7 +67,7 @@ export default class AwaitBlockWrapper extends Wrapper {
 
 		this.cannotUseInnerHTML();
 
-		block.addDependencies(this.node.expression.dynamic_dependencies);
+		block.addDependencies(this.node.expression.dependencies);
 
 		let isDynamic = false;
 		let hasIntros = false;

src/compile/render-dom/wrappers/DebugTag.ts

@@ -45,7 +45,7 @@ export default class DebugTagWrapper extends Wrapper {
 
 			const dependencies = new Set();
 			this.node.expressions.forEach(expression => {
-				addToSet(dependencies, expression.dynamic_dependencies);
+				addToSet(dependencies, expression.dependencies);
 			});
 
 			const condition = [...dependencies].map(d => `changed.${d}`).join(' || ');

src/compile/render-dom/wrappers/EachBlock.ts

@@ -74,8 +74,8 @@ export default class EachBlockWrapper extends Wrapper {
 		super(renderer, block, parent, node);
 		this.cannotUseInnerHTML();
 
-		const { dynamic_dependencies } = node.expression;
+		const { dependencies } = node.expression;
-		block.addDependencies(dynamic_dependencies);
+		block.addDependencies(dependencies);
 
 		this.block = block.child({
 			comment: createDebuggingComment(this.node, this.renderer.component),

src/compile/render-dom/wrappers/Element/Attribute.ts

@@ -160,8 +160,8 @@ export default class AttributeWrapper {
 			}
 
 			// only add an update if mutations are involved (or it's a select?)
-			if (this.node.parent.scope.containsMutable(this.node.dependencies) || isSelectValueAttribute) {
+			const dependencies = this.node.get_dependencies();
-				const dependencies = Array.from(this.node.dependencies);
+			if (dependencies.length > 0 || isSelectValueAttribute) {
 				const changedCheck = (
 					(block.hasOutros ? `!#current || ` : '') +
 					dependencies.map(dependency => `changed.${dependency}`).join(' || ')

src/compile/render-dom/wrappers/Element/Binding.ts

@@ -37,14 +37,14 @@ export default class BindingWrapper {
 		this.node = node;
 		this.parent = parent;
 
-		const { dynamic_dependencies } = this.node.expression;
+		const { dependencies } = this.node.expression;
 
-		block.addDependencies(dynamic_dependencies);
+		block.addDependencies(dependencies);
 
 		// TODO does this also apply to e.g. `<input type='checkbox' bind:group='foo'>`?
 		if (parent.node.name === 'select') {
-			parent.selectBindingDependencies = dynamic_dependencies;
+			parent.selectBindingDependencies = dependencies;
-			dynamic_dependencies.forEach((prop: string) => {
+			dependencies.forEach((prop: string) => {
 				parent.renderer.component.indirectDependencies.set(prop, new Set());
 			});
 		}
@@ -106,7 +106,7 @@ export default class BindingWrapper {
 
 		let updateConditions: string[] = this.needsLock ? [`!${lock}`] : [];
 
-		const dependencyArray = [...this.node.expression.dynamic_dependencies]
+		const dependencyArray = [...this.node.expression.dependencies]
 
 		if (dependencyArray.length === 1) {
 			updateConditions.push(`changed.${dependencyArray[0]}`)

src/compile/render-dom/wrappers/Element/StyleAttribute.ts

@@ -35,7 +35,7 @@ export default class StyleAttributeWrapper extends AttributeWrapper {
 							} else {
 								const snippet = chunk.render();
 
-								addToSet(propDependencies, chunk.dynamic_dependencies);
+								addToSet(propDependencies, chunk.dependencies);
 
 								return chunk.getPrecedence() <= 13 ? `(${snippet})` : snippet;
 							}

src/compile/render-dom/wrappers/Element/index.ts

@@ -173,13 +173,13 @@ export default class ElementWrapper extends Wrapper {
 		// add directive and handler dependencies
 		[node.animation, node.outro, ...node.actions, ...node.classes].forEach(directive => {
 			if (directive && directive.expression) {
-				block.addDependencies(directive.expression.dynamic_dependencies);
+				block.addDependencies(directive.expression.dependencies);
 			}
 		});
 
 		node.handlers.forEach(handler => {
 			if (handler.expression) {
-				block.addDependencies(handler.expression.dynamic_dependencies);
+				block.addDependencies(handler.expression.dependencies);
 			}
 		});
 

src/compile/render-dom/wrappers/IfBlock.ts

@@ -87,7 +87,7 @@ export default class IfBlockWrapper extends Wrapper {
 			this.branches.push(branch);
 
 			blocks.push(branch.block);
-			block.addDependencies(node.expression.dynamic_dependencies);
+			block.addDependencies(node.expression.dependencies);
 
 			if (branch.block.dependencies.size > 0) {
 				isDynamic = true;

src/compile/render-dom/wrappers/InlineComponent/index.ts

@@ -35,7 +35,7 @@ export default class InlineComponentWrapper extends Wrapper {
 		this.cannotUseInnerHTML();
 
 		if (this.node.expression) {
-			block.addDependencies(this.node.expression.dynamic_dependencies);
+			block.addDependencies(this.node.expression.dependencies);
 		}
 
 		this.node.attributes.forEach(attr => {
@@ -52,12 +52,12 @@ export default class InlineComponentWrapper extends Wrapper {
 				(eachBlock as EachBlock).has_binding = true;
 			}
 
-			block.addDependencies(binding.expression.dynamic_dependencies);
+			block.addDependencies(binding.expression.dependencies);
 		});
 
 		this.node.handlers.forEach(handler => {
 			if (handler.expression) {
-				block.addDependencies(handler.expression.dynamic_dependencies);
+				block.addDependencies(handler.expression.dependencies);
 			}
 		});
 
@@ -147,7 +147,10 @@ export default class InlineComponentWrapper extends Wrapper {
 		const fragment_dependencies = new Set();
 		this.slots.forEach(slot => {
 			slot.block.dependencies.forEach(name => {
-				if (renderer.component.mutable_props.has(name)) {
+				const is_let = this.node.lets.some(l => l.names.indexOf(name) !== -1);
+				const variable = renderer.component.var_lookup.get(name);
+
+				if (is_let || variable.mutated) {
 					fragment_dependencies.add(name);
 				}
 			});
@@ -279,7 +282,7 @@ export default class InlineComponentWrapper extends Wrapper {
 			);
 
 			updates.push(deindent`
-				if (!${updating} && ${[...binding.expression.dynamic_dependencies].map((dependency: string) => `changed.${dependency}`).join(' || ')}) {
+				if (!${updating} && ${[...binding.expression.dependencies].map((dependency: string) => `changed.${dependency}`).join(' || ')}) {
 					${name_changes}${quotePropIfNecessary(binding.name)} = ${snippet};
 				}
 			`);

src/compile/render-dom/wrappers/Title.ts

@@ -34,7 +34,7 @@ export default class TitleWrapper extends Wrapper {
 				// single {tag} — may be a non-string
 				const { expression } = this.node.children[0];
 				value = expression.render(block);
-				addToSet(allDependencies, expression.dynamic_dependencies);
+				addToSet(allDependencies, expression.dependencies);
 			} else {
 				// '{foo} {bar}' — treat as string concatenation
 				value =
@@ -46,7 +46,7 @@ export default class TitleWrapper extends Wrapper {
 							} else {
 								const snippet = chunk.expression.render(block);
 
-								chunk.expression.dynamic_dependencies.forEach(d => {
+								chunk.expression.dependencies.forEach(d => {
 									allDependencies.add(d);
 								});
 

src/compile/render-dom/wrappers/shared/Tag.ts

@@ -11,14 +11,14 @@ export default class Tag extends Wrapper {
 		super(renderer, block, parent, node);
 		this.cannotUseInnerHTML();
 
-		block.addDependencies(node.expression.dynamic_dependencies);
+		block.addDependencies(node.expression.dependencies);
 	}
 
 	renameThisMethod(
 		block: Block,
 		update: ((value: string) => string)
 	) {
-		const dependencies = this.node.expression.dynamic_dependencies;
+		const dependencies = this.node.expression.dynamic_dependencies();
 		const snippet = this.node.expression.render(block);
 
 		const value = this.node.shouldCache && block.getUniqueName(`${this.var}_value`);
@@ -26,27 +26,22 @@ export default class Tag extends Wrapper {
 
 		if (this.node.shouldCache) block.addVariable(value, snippet);
 
-		if (dependencies.size) {
+		if (dependencies.length > 0) {
 			const changedCheck = (
 				(block.hasOutros ? `!#current || ` : '') +
-				[...dependencies].map((dependency: string) => `changed.${dependency}`).join(' || ')
+				dependencies.map((dependency: string) => `changed.${dependency}`).join(' || ')
 			);
 
 			const updateCachedValue = `${value} !== (${value} = ${snippet})`;
 
 			const condition =this.node.shouldCache
-				? dependencies.size > 0
+				? `(${changedCheck}) && ${updateCachedValue}`
-					? `(${changedCheck}) && ${updateCachedValue}`
-					: updateCachedValue
 				: changedCheck;
 
-			// only update if there's a mutation involved
+			block.builders.update.addConditional(
-			if (this.node.expression.template_scope.containsMutable(dependencies)) {
+				condition,
-				block.builders.update.addConditional(
+				update(content)
-					condition,
+			);
-					update(content)
-				);
-			}
 		}
 
 		return { init: content };

src/compile/render-dom/wrappers/shared/addActions.ts

@@ -1,4 +1,3 @@
-import Renderer from '../../Renderer';
 import Block from '../../Block';
 import Action from '../../../nodes/Action';
 import Component from '../../../Component';
@@ -15,7 +14,7 @@ export default function addActions(
 
 		if (expression) {
 			snippet = expression.render(block);
-			dependencies = expression.dynamic_dependencies;
+			dependencies = expression.dynamic_dependencies();
 		}
 
 		const name = block.getUniqueName(
@@ -33,10 +32,10 @@ export default function addActions(
 			`${name} = ${fn}.call(null, ${target}${snippet ? `, ${snippet}` : ''}) || {};`
 		);
 
-		if (dependencies && dependencies.size > 0) {
+		if (dependencies && dependencies.length > 0) {
 			let conditional = `typeof ${name}.update === 'function' && `;
-			const deps = [...dependencies].map(dependency => `changed.${dependency}`).join(' || ');
+			const deps = dependencies.map(dependency => `changed.${dependency}`).join(' || ');
-			conditional += dependencies.size > 1 ? `(${deps})` : deps;
+			conditional += dependencies.length > 1 ? `(${deps})` : deps;
 
 			block.builders.update.addConditional(
 				conditional,