fix bind:this with elements in each block, add tests for #2806

src/compiler/compile/render_dom/wrappers/Element/index.ts

@@ -19,6 +19,7 @@ import add_event_handlers from '../shared/add_event_handlers';
 import add_actions from '../shared/add_actions';
 import create_debugging_comment from '../shared/create_debugging_comment';
 import { get_context_merger } from '../shared/get_context_merger';
+import bind_this from '../shared/bind_this';
 
 const events = [
 	{
@@ -540,38 +541,9 @@ export default class ElementWrapper extends Wrapper {
 
 		const this_binding = this.bindings.find(b => b.node.name === 'this');
 		if (this_binding) {
-			const name = renderer.component.get_unique_name(`${this.var}_binding`);
+			const binding_callback = bind_this(renderer.component, block, this_binding.node, this.var);
 
-			renderer.component.add_var({
+			block.builders.mount.add_line(binding_callback);
-				name,
-				internal: true,
-				referenced: true
-			});
-
-			const { handler, object } = this_binding;
-
-			const args = [];
-			for (const arg of handler.contextual_dependencies) {
-				args.push(arg);
-				block.add_variable(arg, `ctx.${arg}`);
-			}
-
-			renderer.component.partly_hoisted.push(deindent`
-				function ${name}(${['$$node', 'check'].concat(args).join(', ')}) {
-					${handler.snippet ? `if ($$node || (!$$node && ${handler.snippet} === check)) ` : ''}${handler.mutation}
-					${renderer.component.invalidate(object)};
-				}
-			`);
-
-			block.builders.mount.add_line(`@add_binding_callback(() => ctx.${name}(${[this.var, 'null'].concat(args).join(', ')}));`);
-			block.builders.destroy.add_line(`ctx.${name}(${['null', this.var].concat(args).join(', ')});`);
-			block.builders.update.add_line(deindent`
-				if (changed.items) {
-					ctx.${name}(${['null', this.var].concat(args).join(', ')});
-					${args.map(a => `${a} = ctx.${a}`).join(', ')};
-					ctx.${name}(${[this.var, 'null'].concat(args).join(', ')});
-				}`
-			);
 		}
 	}
 

src/compiler/compile/render_dom/wrappers/InlineComponent/index.ts

@@ -14,6 +14,7 @@ import create_debugging_comment from '../shared/create_debugging_comment';
 import { get_context_merger } from '../shared/get_context_merger';
 import EachBlock from '../../../nodes/EachBlock';
 import TemplateScope from '../../../nodes/shared/TemplateScope';
+import bind_this from '../shared/bind_this';
 
 export default class InlineComponentWrapper extends Wrapper {
 	var: string;
@@ -252,41 +253,7 @@ export default class InlineComponentWrapper extends Wrapper {
 			component.has_reactive_assignments = true;
 
 			if (binding.name === 'this') {
-				const fn = component.get_unique_name(`${this.var}_binding`);
+				return bind_this(component, block, binding, this.var);
-
-				component.add_var({
-					name: fn,
-					internal: true,
-					referenced: true
-				});
-
-				let lhs;
-				let object;
-
-				if (binding.is_contextual && binding.expression.node.type === 'Identifier') {
-					// bind:x={y} — we can't just do `y = x`, we need to
-					// to `array[index] = x;
-					const { name } = binding.expression.node;
-					const { snippet } = block.bindings.get(name);
-					lhs = snippet;
-
-					// TODO we need to invalidate... something
-				} else {
-					object = flatten_reference(binding.expression.node).name;
-					lhs = component.source.slice(binding.expression.node.start, binding.expression.node.end).trim();
-				}
-
-				const contextual_dependencies = [...binding.expression.contextual_dependencies];
-
-				component.partly_hoisted.push(deindent`
-					function ${fn}(${['$$component', ...contextual_dependencies].join(', ')}) {
-						${lhs} = $$component;
-						${object && component.invalidate(object)}
-					}
-				`);
-
-				block.builders.destroy.add_line(`ctx.${fn}(${['null', ...contextual_dependencies.map(name => `ctx.${name}`)].join(', ')});`);
-				return `@add_binding_callback(() => ctx.${fn}(${[this.var, ...contextual_dependencies.map(name => `ctx.${name}`)].join(', ')}));`;
 			}
 
 			const name = component.get_unique_name(`${this.var}_${binding.name}_binding`);

src/compiler/compile/render_dom/wrappers/shared/bind_this.ts

@@ -0,0 +1,43 @@
+import flatten_reference from '../../../utils/flatten_reference';
+import deindent from '../../../utils/deindent';
+import Component from '../../../Component';
+import Block from '../../Block';
+import Binding from '../../../nodes/Binding';
+
+export default function bind_this(component: Component, block: Block, binding: Binding, variable: string) {
+	const fn = component.get_unique_name(`${variable}_binding`);
+
+	component.add_var({
+		name: fn,
+		internal: true,
+		referenced: true
+	});
+
+	let lhs;
+	let object;
+
+	if (binding.is_contextual && binding.expression.node.type === 'Identifier') {
+		// bind:x={y} — we can't just do `y = x`, we need to
+		// to `array[index] = x;
+		const { name } = binding.expression.node;
+		const { snippet } = block.bindings.get(name);
+		lhs = snippet;
+
+		// TODO we need to invalidate... something
+	} else {
+		object = flatten_reference(binding.expression.node).name;
+		lhs = component.source.slice(binding.expression.node.start, binding.expression.node.end).trim();
+	}
+
+	const contextual_dependencies = [...binding.expression.contextual_dependencies];
+
+	component.partly_hoisted.push(deindent`
+		function ${fn}(${['$$value', ...contextual_dependencies].join(', ')}) {
+			${lhs} = $$value;
+			${object && component.invalidate(object)}
+		}
+	`);
+
+	block.builders.destroy.add_line(`ctx.${fn}(${['null', ...contextual_dependencies.map(name => `ctx.${name}`)].join(', ')});`);
+	return `@add_binding_callback(() => ctx.${fn}(${[variable, ...contextual_dependencies.map(name => `ctx.${name}`)].join(', ')}));`;
+}

test/runtime/samples/binding-this-each-block-property-component/Foo.svelte

@@ -0,0 +1,7 @@
+<script>
+	export function isFoo() {
+		return true;
+	}
+</script>
+
+<p><slot></slot></p>

test/runtime/samples/binding-this-each-block-property-component/_config.js

@@ -0,0 +1,12 @@
+export default {
+	html: ``,
+
+	async test({ assert, component, target }) {
+		component.visible = true;
+		assert.htmlEqual(target.innerHTML, `
+			<p>a</p>
+		`);
+
+		assert.ok(component.items[0].ref.isFoo());
+	}
+};

test/runtime/samples/binding-this-each-block-property-component/main.svelte

@@ -0,0 +1,13 @@
+<script>
+	import Foo from './Foo.svelte';
+
+	export let visible = false;
+
+	export let items = [{ value: 'a', ref: null }];
+</script>
+
+{#if visible}
+	{#each items as item}
+		<Foo bind:this={item.ref}>{item.value}</Foo>
+	{/each}
+{/if}

test/runtime/samples/binding-this-each-block-property/_config.js

@@ -0,0 +1,12 @@
+export default {
+	html: ``,
+
+	async test({ assert, component, target }) {
+		component.visible = true;
+		assert.htmlEqual(target.innerHTML, `
+			<div>a</div>
+		`);
+
+		assert.equal(component.items[0].ref, target.querySelector('div'));
+	}
+};

test/runtime/samples/binding-this-each-block-property/main.svelte

@@ -0,0 +1,11 @@
+<script>
+	export let visible = false;
+
+	export let items = [{ value: 'a', ref: null }];
+</script>
+
+{#if visible}
+	{#each items as item}
+		<div bind:this={item.ref}>{item.value}</div>
+	{/each}
+{/if}