Merge pull request #1980 from evs-chris/gh-1952-evschris

First pass at tracking mutation to avoid unnecessary update code - #1952

src/compile/Component.ts

@@ -77,6 +77,7 @@ 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();
@@ -169,6 +170,7 @@ export default class Component {
 		if (!this.instance_script) {
 			const props = [...this.template_references];
 			this.declarations.push(...props);
+			addToSet(this.mutable_props, this.template_references);
 			addToSet(this.writable_declarations, this.template_references);
 			addToSet(this.userVars, this.template_references);
 
@@ -177,6 +179,9 @@ export default class Component {
 				as: name
 			}));
 		}
+
+		// 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));
 	}
 
 	addSourcemapLocations(node: Node) {
@@ -405,6 +410,7 @@ export default class Component {
 						node.declaration.declarations.forEach(declarator => {
 							extractNames(declarator.id).forEach(name => {
 								exports.push({ name, as: name });
+								this.mutable_props.add(name);
 							});
 						});
 					} else {
@@ -537,6 +543,7 @@ export default class Component {
 			this.userVars.add(name);
 		});
 
+		this.track_mutations();
 		this.extract_imports_and_exports(script.content, this.imports, this.props);
 		this.hoist_instance_declarations();
 		this.extract_reactive_declarations();
@@ -544,6 +551,35 @@ export default class Component {
 		this.javascript = this.extract_javascript(script);
 	}
 
+	// TODO merge this with other walks that are independent
+	track_mutations() {
+		const component = this;
+		let { instance_scope: scope, instance_scope_map: map } = this;
+
+		walk(this.instance_script.content, {
+			enter(node, parent) {
+				let names;
+				if (map.has(node)) {
+					scope = map.get(node);
+				}
+
+				if (node.type === 'AssignmentExpression') {
+					names = node.left.type === 'MemberExpression'
+							? [getObject(node.left).name]
+							: 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);
+					});
+				}
+			}
+		})
+	}
+
 	extract_reactive_store_references() {
 		// TODO this pattern happens a lot... can we abstract it
 		// (or better still, do fewer AST walks)?
@@ -586,6 +622,7 @@ 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 = [];

src/compile/nodes/Binding.ts

@@ -29,6 +29,9 @@ export default class Binding extends Node {
 		const { name } = getObject(this.expression.node);
 		this.isContextual = scope.names.has(name);
 
+		// make sure we track this as a mutable ref
+		scope.setMutable(name);
+
 		if (this.expression.node.type === 'MemberExpression') {
 			prop = `[✂${this.expression.node.property.start}-${this.expression.node.property.end}✂]`;
 			if (!this.expression.node.computed) prop = `'${prop}'`;

src/compile/nodes/shared/Expression.ts

@@ -104,15 +104,16 @@ export default class Expression {
 		const expression = this;
 		let function_expression;
 
-		function add_dependency(name) {
+		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.instance_script) {
-					if (component.writable_declarations.has(name) || name[0] === '$') {
+					if (component.writable_declarations.has(name) || name[0] === '$' || (component.userVars.has(name) && deep)) {
 						dynamic_dependencies.add(name);
 					}
 				} else {
@@ -146,9 +147,9 @@ export default class Expression {
 
 						contextual_dependencies.add(name);
 
-						template_scope.dependenciesForName.get(name).forEach(add_dependency);
+						template_scope.dependenciesForName.get(name).forEach(name => add_dependency(name, true));
 					} else {
-						add_dependency(name);
+						add_dependency(name, nodes.length > 1);
 						component.template_references.add(name);
 
 						component.warn_if_undefined(nodes[0], template_scope, true);
@@ -156,6 +157,19 @@ export default class Expression {
 
 					this.skip();
 				}
+
+				// track any assignments from template expressions as mutable
+				if (function_expression) {
+					if (node.type === 'AssignmentExpression') {
+						const names = 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);
+					}
+				}
 			},
 
 			leave(node) {

src/compile/nodes/shared/TemplateScope.ts

@@ -1,10 +1,14 @@
 export default class TemplateScope {
 	names: Set<string>;
 	dependenciesForName: Map<string, string>;
+	mutables: Set<string>;
+	parent?: TemplateScope;
 
 	constructor(parent?: TemplateScope) {
+		this.parent = parent;
 		this.names = new Set(parent ? parent.names : []);
 		this.dependenciesForName = new Map(parent ? parent.dependenciesForName : []);
+		this.mutables = new Set();
 	}
 
 	add(name, dependencies) {
@@ -14,6 +18,24 @@ export default class TemplateScope {
 	}
 
 	child() {
-		return new TemplateScope(this);
+		const child = new TemplateScope(this);
+		return child;
+	}
+
+	setMutable(name: string) {
+		if (this.names.has(name)) this.mutables.add(name);
+		else if (this.parent) this.parent.setMutable(name);
+		else this.mutables.add(name);
+	}
+
+	containsMutable(names: Iterable<string>) {
+		for (const name of names) {
+			if (name[0] === '$') return true;
+			if (this.mutables.has(name)) return true;
+			else if (this.dependenciesForName.has(name) && this.containsMutable(this.dependenciesForName.get(name))) return true;
+		}
+
+		if (this.parent) return this.parent.containsMutable(names);
+		else return false;
 	}
 }

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

@@ -159,7 +159,8 @@ export default class AttributeWrapper {
 				updater = `${method}(${element.var}, "${name}", ${shouldCache ? last : value});`;
 			}
 
-			if (this.node.dependencies.size || isSelectValueAttribute) {
+			// 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 = Array.from(this.node.dependencies);
 				const changedCheck = (
 					(block.hasOutros ? `!#current || ` : '') +

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

@@ -40,10 +40,13 @@ export default class Tag extends Wrapper {
 					: updateCachedValue
 				: changedCheck;
 
-			block.builders.update.addConditional(
+			// only update if there's a mutation involved
-				condition,
+			if (this.node.expression.template_scope.containsMutable(dependencies)) {
-				update(content)
+				block.builders.update.addConditional(
-			);
+					condition,
+					update(content)
+				);
+			}
 		}
 
 		return { init: content };

test/js/samples/non-mutable-reference/expected.js

@@ -0,0 +1,53 @@
+/* generated by Svelte vX.Y.Z */
+import { SvelteComponent as SvelteComponent_1, append, createElement, createText, detachNode, init, insert, noop, run, safe_not_equal } from "svelte/internal";
+
+function create_fragment($$, ctx) {
+	var h1, text0, text1, text2, current;
+
+	return {
+		c() {
+			h1 = createElement("h1");
+			text0 = createText("Hello ");
+			text1 = createText(ctx.name);
+			text2 = createText("!");
+		},
+
+		m(target, anchor) {
+			insert(target, h1, anchor);
+			append(h1, text0);
+			append(h1, text1);
+			append(h1, text2);
+			current = true;
+		},
+
+		p: noop,
+
+		i(target, anchor) {
+			if (current) return;
+			this.m(target, anchor);
+		},
+
+		o: run,
+
+		d(detach) {
+			if (detach) {
+				detachNode(h1);
+			}
+		}
+	};
+}
+
+function instance($$self) {
+	let name = 'world';
+
+	return { name };
+}
+
+class SvelteComponent extends SvelteComponent_1 {
+	constructor(options) {
+		super();
+		init(this, options, instance, create_fragment, safe_not_equal);
+	}
+}
+
+export default SvelteComponent;

test/js/samples/non-mutable-reference/input.html

@@ -0,0 +1,5 @@
+<script>
+	let name = 'world';
+</script>
+
+<h1>Hello {name}!</h1>