Merge pull request #2398 from sveltejs/gh-2285

allow reactive declarations without dependencies

src/compile/Component.ts

@@ -5,7 +5,7 @@ import Stats from '../Stats';
 import { globals, reserved } from '../utils/names';
 import { namespaces, valid_namespaces } from '../utils/namespaces';
 import create_module from './create_module';
-import { create_scopes, extract_names, Scope } from './utils/scope';
+import { create_scopes, extract_names, Scope, extract_identifiers } from './utils/scope';
 import Stylesheet from './css/Stylesheet';
 import { test } from '../config';
 import Fragment from './nodes/Fragment';
@@ -19,6 +19,7 @@ import TemplateScope from './nodes/shared/TemplateScope';
 import fuzzymatch from '../utils/fuzzymatch';
 import { remove_indentation, add_indentation } from '../utils/indentation';
 import get_object from './utils/get_object';
+import unwrap_parens from './utils/unwrap_parens';
 
 type ComponentOptions = {
 	namespace?: string;
@@ -93,7 +94,7 @@ export default class Component {
 	node_for_declaration: Map<string, Node> = new Map();
 	partly_hoisted: string[] = [];
 	fully_hoisted: string[] = [];
-	reactive_declarations: Array<{ assignees: Set<string>, dependencies: Set<string>, node: Node, injected: boolean }> = [];
+	reactive_declarations: Array<{ assignees: Set<string>, dependencies: Set<string>, node: Node, declaration: Node }> = [];
 	reactive_declaration_nodes: Set<Node> = new Set();
 	has_reactive_assignments = false;
 	injected_reactive_declaration_vars: Set<string> = new Set();
@@ -590,13 +591,15 @@ export default class Component {
 		script.content.body.forEach(node => {
 			if (node.type !== 'LabeledStatement') return;
 			if (node.body.type !== 'ExpressionStatement') return;
-			if (node.body.expression.type !== 'AssignmentExpression') return;
 
-			const { type, name } = node.body.expression.left;
+			const expression = unwrap_parens(node.body.expression);
+			if (expression.type !== 'AssignmentExpression') return;
 
-			if (type === 'Identifier' && !this.var_lookup.has(name) && name[0] !== '$') {
-				this.injected_reactive_declaration_vars.add(name);
-			}
+			extract_names(expression.left).forEach(name => {
+				if (!this.var_lookup.has(name) && name[0] !== '$') {
+					this.injected_reactive_declaration_vars.add(name);
+				}
+			});
 		});
 
 		let { scope: instance_scope, map, globals } = create_scopes(script.content);
@@ -1058,9 +1061,10 @@ export default class Component {
 						}
 
 						if (node.type === 'AssignmentExpression') {
-							const identifier = get_object(node.left)
-							assignee_nodes.add(identifier);
-							assignees.add(identifier.name);
+							extract_identifiers(get_object(node.left)).forEach(node => {
+								assignee_nodes.add(node);
+								assignees.add(node.name);
+							});
 						} else if (node.type === 'UpdateExpression') {
 							const identifier = get_object(node.argument);
 							assignees.add(identifier.name);
@@ -1090,18 +1094,10 @@ export default class Component {
 
 				add_indentation(this.code, node.body, 2);
 
-				unsorted_reactive_declarations.push({
-					assignees,
-					dependencies,
-					node,
-					injected: (
-						node.body.type === 'ExpressionStatement' &&
-						node.body.expression.type === 'AssignmentExpression' &&
-						node.body.expression.left.type === 'Identifier' &&
-						node.body.expression.left.name[0] !== '$' &&
-						this.var_lookup.get(node.body.expression.left.name).injected
-					)
-				});
+				const expression = node.body.expression && unwrap_parens(node.body.expression);
+				const declaration = expression && expression.left;
+
+				unsorted_reactive_declarations.push({ assignees, dependencies, node, declaration });
 			}
 		});
 
@@ -1134,13 +1130,6 @@ export default class Component {
 
 			seen.add(declaration);
 
-			if (declaration.dependencies.size === 0) {
-				this.error(declaration.node, {
-					code: 'invalid-reactive-declaration',
-					message: 'Invalid reactive declaration — must depend on local state'
-				});
-			}
-
 			declaration.dependencies.forEach(name => {
 				if (declaration.assignees.has(name)) return;
 				const earlier_declarations = lookup.get(name);

src/compile/render-dom/index.ts

@@ -188,14 +188,26 @@ export default function dom(
 
 						code.overwrite(node.start, node.end, dirty.map(n => component.invalidate(n)).join('; '));
 					} else {
+						const single = (
+							node.left.type === 'Identifier' &&
+							parent.type === 'ExpressionStatement' &&
+							node.left.name[0] !== '$'
+						);
+
 						names.forEach(name => {
 							const owner = scope.find_owner(name);
 							if (owner && owner !== component.instance_scope) return;
 
 							const variable = component.var_lookup.get(name);
 							if (variable && (variable.hoistable || variable.global || variable.module)) return;
 
-							pending_assignments.add(name);
+							if (single) {
+								code.prependRight(node.start, `$$invalidate('${name}', `);
+								code.appendLeft(node.end, `)`);
+							} else {
+								pending_assignments.add(name);
+							}
+
 							component.has_reactive_assignments = true;
 						});
 					}
@@ -347,28 +359,34 @@ export default function dom(
 		.map(({ name }) => `$$self.$$.on_destroy.push(() => $$unsubscribe_${name.slice(1)}());`);
 
 	if (has_definition) {
-		const reactive_declarations = component.reactive_declarations.map(d => {
-			const condition = Array.from(d.dependencies)
-				.filter(n => {
-					if (n === '$$props') return false;
-					const variable = component.var_lookup.get(n);
-					return variable && variable.writable;
-				})
-				.map(n => `$$dirty.${n}`).join(' || ');
-
-			const snippet = d.node.body.type === 'BlockStatement'
-				? `[✂${d.node.body.start}-${d.node.end}✂]`
-				: deindent`
-					{
-						[✂${d.node.body.start}-${d.node.end}✂]
-					}`;
-
-			return condition
-				? deindent`
-					if (${condition}) ${snippet}`
-				: deindent`
-					${snippet}`
-		});
+		const reactive_declarations = [];
+		const fixed_reactive_declarations = []; // not really 'reactive' but whatever
+
+		component.reactive_declarations
+			.forEach(d => {
+				let uses_props;
+
+				const condition = Array.from(d.dependencies)
+					.filter(n => {
+						if (n === '$$props') {
+							uses_props = true;
+							return false;
+						}
+
+						const variable = component.var_lookup.get(n);
+						return variable && variable.writable;
+					})
+					.map(n => `$$dirty.${n}`).join(' || ');
+
+				let snippet = `[✂${d.node.body.start}-${d.node.end}✂]`;
+				if (condition) snippet = `if (${condition}) { ${snippet} }`;
+
+				if (condition || uses_props) {
+					reactive_declarations.push(snippet);
+				} else {
+					fixed_reactive_declarations.push(snippet);
+				}
+			});
 
 		const injected = Array.from(component.injected_reactive_declaration_vars).filter(name => {
 			const variable = component.var_lookup.get(name);
@@ -410,6 +428,8 @@ export default function dom(
 				$$self.$$.update = ($$dirty = { ${Array.from(all_reactive_dependencies).map(n => `${n}: 1`).join(', ')} }) => {
 					${reactive_declarations}
 				};
+
+				${fixed_reactive_declarations}
 				`}
 
 				return ${stringify_props(filtered_declarations)};

src/compile/render-ssr/index.ts

@@ -3,6 +3,7 @@ import Component from '../Component';
 import { CompileOptions } from '../../interfaces';
 import { stringify } from '../utils/stringify';
 import Renderer from './Renderer';
+import { extract_names } from '../utils/scope';
 
 export default function ssr(
 	component: Component,
@@ -63,8 +64,32 @@ export default function ssr(
 		: [];
 
 	const reactive_declarations = component.reactive_declarations.map(d => {
-		const snippet = `[✂${d.node.body.start}-${d.node.end}✂]`;
-		return d.injected ? `let ${snippet}` : snippet;
+		let snippet = `[✂${d.node.body.start}-${d.node.end}✂]`;
+
+		if (d.declaration) {
+			const declared = extract_names(d.declaration);
+			const injected = declared.filter(name => {
+				return name[0] !== '$' && component.var_lookup.get(name).injected;
+			});
+
+			const self_dependencies = injected.filter(name => d.dependencies.has(name));
+
+			if (injected.length) {
+				// in some cases we need to do `let foo; [expression]`, in
+				// others we can do `let [expression]`
+				const separate = (
+					self_dependencies.length > 0 ||
+					declared.length > injected.length ||
+					d.node.body.expression.type === 'ParenthesizedExpression'
+				);
+
+				snippet = separate
+					? `let ${injected.join(', ')}; ${snippet}`
+					: `let ${snippet}`;
+			}
+		}
+
+		return snippet;
 	});
 
 	const main = renderer.has_bindings

src/compile/utils/get_object.ts

@@ -1,7 +1,8 @@
 import { Node } from '../../interfaces';
+import unwrap_parens from './unwrap_parens';
 
 export default function get_object(node: Node) {
-	while (node.type === 'ParenthesizedExpression') node = node.expression;
+	node = unwrap_parens(node);
 	while (node.type === 'MemberExpression') node = node.object;
 	return node;
 }

src/compile/utils/scope.ts

@@ -101,37 +101,41 @@ export class Scope {
 }
 
 export function extract_names(param: Node) {
-	const names: string[] = [];
-	extractors[param.type](names, param);
-	return names;
+	return extract_identifiers(param).map(node => node.name);
+}
+
+export function extract_identifiers(param: Node) {
+	const nodes: Node[] = [];
+	extractors[param.type](nodes, param);
+	return nodes;
 }
 
 const extractors = {
-	Identifier(names: string[], param: Node) {
-		names.push(param.name);
+	Identifier(nodes: Node[], param: Node) {
+		nodes.push(param);
 	},
 
-	ObjectPattern(names: string[], param: Node) {
+	ObjectPattern(nodes: Node[], param: Node) {
 		param.properties.forEach((prop: Node) => {
 			if (prop.type === 'RestElement') {
-				names.push(prop.argument.name);
+				nodes.push(prop.argument);
 			} else {
-				extractors[prop.value.type](names, prop.value);
+				extractors[prop.value.type](nodes, prop.value);
 			}
 		});
 	},
 
-	ArrayPattern(names: string[], param: Node) {
+	ArrayPattern(nodes: Node[], param: Node) {
 		param.elements.forEach((element: Node) => {
-			if (element) extractors[element.type](names, element);
+			if (element) extractors[element.type](nodes, element);
 		});
 	},
 
-	RestElement(names: string[], param: Node) {
-		extractors[param.argument.type](names, param.argument);
+	RestElement(nodes: Node[], param: Node) {
+		extractors[param.argument.type](nodes, param.argument);
 	},
 
-	AssignmentPattern(names: string[], param: Node) {
-		extractors[param.left.type](names, param.left);
+	AssignmentPattern(nodes: Node[], param: Node) {
+		extractors[param.left.type](nodes, param.left);
 	}
 };

src/compile/utils/unwrap_parens.ts

@@ -0,0 +1,6 @@
+import { Node } from '../../interfaces';
+
+export default function unwrap_parens(node: Node) {
+	while (node.type === 'ParenthesizedExpression') node = node.expression;
+	return node;
+}

test/js/samples/dev-warning-missing-data-computed/expected.js

@@ -70,9 +70,7 @@ function instance($$self, $$props, $$invalidate) {
 	};
 
 	$$self.$$.update = ($$dirty = { foo: 1 }) => {
-		if ($$dirty.foo) {
-			bar = foo * 2; $$invalidate('bar', bar);
-		}
+		if ($$dirty.foo) { $$invalidate('bar', bar = foo * 2); }
 	};
 
 	return { foo, bar };

test/js/samples/instrumentation-script-if-no-block/expected.js

@@ -61,7 +61,7 @@ function instance($$self, $$props, $$invalidate) {
 	let x = 0;
 
 	function foo() {
-		if (true) { x += 1; $$invalidate('x', x); }
+		if (true) $$invalidate('x', x += 1);
 	}
 
 	return { x, foo };

test/js/samples/reactive-values-non-topologically-ordered/expected.js

@@ -28,12 +28,8 @@ function instance($$self, $$props, $$invalidate) {
 	};
 
 	$$self.$$.update = ($$dirty = { x: 1, b: 1 }) => {
-		if ($$dirty.x) {
-			b = x; $$invalidate('b', b);
-		}
-		if ($$dirty.b) {
-			a = b; $$invalidate('a', a);
-		}
+		if ($$dirty.x) { $$invalidate('b', b = x); }
+		if ($$dirty.b) { $$invalidate('a', a = b); }
 	};
 
 	return { x };

test/js/samples/reactive-values-non-writable-dependencies/expected.js

@@ -27,9 +27,7 @@ function instance($$self, $$props, $$invalidate) {
 	let max;
 
 	$$self.$$.update = ($$dirty = { a: 1, b: 1 }) => {
-		if ($$dirty.a || $$dirty.b) {
-			max = Math.max(a, b); $$invalidate('max', max);
-		}
+		if ($$dirty.a || $$dirty.b) { $$invalidate('max', max = Math.max(a, b)); }
 	};
 
 	return {};

test/runtime/samples/props-reactive/_config.js

@@ -1,6 +1,4 @@
 export default {
-	show: 1,
-
 	props: {
 		a: 1,
 		b: 2,

test/runtime/samples/reactive-values-implicit-destructured/_config.js

@@ -0,0 +1,25 @@
+export default {
+	props: {
+		coords: [0, 0],
+		numbers: { answer: 42 }
+	},
+
+	html: `
+		<p>0,0</p>
+		<p>42</p>
+	`,
+
+	test({ assert, component, target }) {
+		component.coords = [1, 2];
+		assert.htmlEqual(target.innerHTML, `
+			<p>1,2</p>
+			<p>42</p>
+		`);
+
+		component.numbers = { answer: 43 };
+		assert.htmlEqual(target.innerHTML, `
+			<p>1,2</p>
+			<p>43</p>
+		`);
+	}
+};

test/runtime/samples/reactive-values-implicit-destructured/main.svelte

@@ -0,0 +1,10 @@
+<script>
+	export let coords;
+	export let numbers;
+
+	$: [x, y] = coords;
+	$: ({ answer } = numbers);
+</script>
+
+<p>{x},{y}</p>
+<p>{answer}</p>