Merge pull request #2062 from sveltejs/gh-2059

inject lets for reactive declarations where necessary

src/compile/Component.ts

@@ -20,7 +20,6 @@ import TemplateScope from './nodes/shared/TemplateScope';
 import fuzzymatch from '../utils/fuzzymatch';
 import { remove_indentation, add_indentation } from '../utils/indentation';
 import getObject from '../utils/getObject';
-import deindent from '../utils/deindent';
 import globalWhitelist from '../utils/globalWhitelist';
 
 type ComponentOptions = {
@@ -66,9 +65,10 @@ 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>, snippet: string }> = [];
+	reactive_declarations: Array<{ assignees: Set<string>, dependencies: Set<string>, node: Node, injected: boolean }> = [];
 	reactive_declaration_nodes: Set<Node> = new Set();
 	has_reactive_assignments = false;
+	injected_reactive_declaration_vars: Set<string> = new Set();
 
 	indirectDependencies: Map<string, Set<string>> = new Map();
 
@@ -554,6 +554,19 @@ export default class Component {
 
 		this.addSourcemapLocations(script.content);
 
+		// inject vars for reactive declarations
+		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;
+
+			if (type === 'Identifier' && !this.var_lookup.has(name)) {
+				this.injected_reactive_declaration_vars.add(name);
+			}
+		});
+
 		let { scope: instance_scope, map, globals } = createScopes(script.content);
 		this.instance_scope = instance_scope;
 		this.instance_scope_map = map;
@@ -589,9 +602,17 @@ export default class Component {
 		});
 
 		globals.forEach(name => {
-			if (this.module_scope && this.module_scope.declarations.has(name)) return;
+			if (this.var_lookup.has(name)) return;
 
-			if (name[0] === '$') {
+			if (this.injected_reactive_declaration_vars.has(name)) {
+				this.add_var({
+					name,
+					injected: true,
+					writable: true,
+					reassigned: true,
+					initialised: true
+				});
+			} else if (name[0] === '$') {
 				this.add_var({
 					name,
 					injected: true,
@@ -1002,12 +1023,12 @@ export default class Component {
 					assignees,
 					dependencies,
 					node,
-					snippet: node.body.type === 'BlockStatement'
-						? `[✂${node.body.start}-${node.end}✂]`
-						: deindent`
-							{
-								[✂${node.body.start}-${node.end}✂]
-							}`
+					injected: (
+						node.body.type === 'ExpressionStatement' &&
+						node.body.expression.type === 'AssignmentExpression' &&
+						node.body.expression.left.type === 'Identifier' &&
+						this.var_lookup.get(node.body.expression.left.name).injected
+					)
 				});
 			}
 		});
@@ -1082,8 +1103,7 @@ export default class Component {
 		}
 
 		if (allow_implicit && !this.ast.instance && !this.ast.module) return;
-		if (this.instance_scope && this.instance_scope.declarations.has(name)) return;
-		if (this.module_scope && this.module_scope.declarations.has(name)) return;
+		if (this.var_lookup.has(name)) return;
 		if (template_scope && template_scope.names.has(name)) return;
 		if (globalWhitelist.has(name)) return;
 

src/compile/render-dom/index.ts

@@ -178,10 +178,11 @@ export default function dom(
 						code.overwrite(node.start, node.end, dirty.map(n => `$$invalidate('${n}', ${n})`).join('; '));
 					} else {
 						names.forEach(name => {
-							if (scope.findOwner(name) !== component.instance_scope) return;
+							const owner = scope.findOwner(name);
+							if (owner && owner !== component.instance_scope) return;
 
 							const variable = component.var_lookup.get(name);
-							if (variable && variable.hoistable) return;
+							if (variable && variable.hoistable || variable.global || variable.module) return;
 
 							pending_assignments.add(name);
 							component.has_reactive_assignments = true;
@@ -312,6 +313,24 @@ export default function dom(
 		.join('\n\n');
 
 	if (has_definition) {
+		const reactive_declarations = component.reactive_declarations.map(d => {
+			const condition = Array.from(d.dependencies).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 deindent`
+				if (${condition}) ${snippet}`
+		});
+
+		const injected = Array.from(component.injected_reactive_declaration_vars).filter(name => {
+			const variable = component.var_lookup.get(name);
+			return variable.injected;
+		});
+
 		builder.addBlock(deindent`
 			function ${definition}(${args.join(', ')}) {
 				${user_code}
@@ -326,10 +345,10 @@ export default function dom(
 
 				${set && `$$self.$set = ${set};`}
 
-				${component.reactive_declarations.length > 0 && deindent`
+				${reactive_declarations.length > 0 && deindent`
+				${injected.length && `let ${injected.join(', ')};`}
 				$$self.$$.update = ($$dirty = { ${Array.from(all_reactive_dependencies).map(n => `${n}: 1`).join(', ')} }) => {
-					${component.reactive_declarations.map(d => deindent`
-					if (${Array.from(d.dependencies).map(n => `$$dirty.${n}`).join(' || ')}) ${d.snippet}`)}
+					${reactive_declarations}
 				};
 				`}
 

src/compile/render-ssr/index.ts

@@ -50,6 +50,11 @@ 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;
+	});
+
 	const main = renderer.has_bindings
 		? deindent`
 			let $$settled;
@@ -60,7 +65,7 @@ export default function ssr(
 
 				${reactive_store_values}
 
-				${component.reactive_declarations.map(d => d.snippet)}
+				${reactive_declarations}
 
 				$$rendered = \`${renderer.code}\`;
 			} while (!$$settled);
@@ -70,7 +75,7 @@ export default function ssr(
 		: deindent`
 			${reactive_store_values}
 
-			${component.reactive_declarations.map(d => d.snippet)}
+			${reactive_declarations}
 
 			return \`${renderer.code}\`;`;
 

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

@@ -0,0 +1,14 @@
+export default {
+	html: `
+		<p>1 + 2 = 3</p>
+		<p>3 * 3 = 9</p>
+	`,
+
+	test({ assert, component, target }) {
+		component.a = 3;
+		assert.htmlEqual(target.innerHTML, `
+			<p>3 + 2 = 5</p>
+			<p>5 * 5 = 25</p>
+		`);
+	}
+};

test/runtime/samples/reactive-values-implicit/main.html

@@ -0,0 +1,10 @@
+<script>
+	export let a = 1;
+	export let b = 2;
+
+	$: c = a + b;
+	$: cSquared = c * c;
+</script>
+
+<p>{a} + {b} = {c}</p>
+<p>{c} * {c} = {cSquared}</p>

test/stats/samples/implicit-reactive/_config.js

@@ -0,0 +1,26 @@
+export default {
+	test(assert, stats) {
+		assert.deepEqual(stats.vars, [
+			{
+				name: 'a',
+				injected: false,
+				export_name: null,
+				module: false,
+				mutated: false,
+				reassigned: false,
+				referenced: true,
+				writable: true
+			},
+			{
+				name: 'b',
+				injected: true,
+				export_name: null,
+				module: false,
+				mutated: false,
+				reassigned: true,
+				referenced: true,
+				writable: true
+			}
+		]);
+	},
+};

test/stats/samples/implicit-reactive/input.html

@@ -0,0 +1,6 @@
+<script>
+	let a = 1;
+	$: b = a + 1;
+</script>
+
+<p>{a} + 1 = {b}</p>