Merge pull request #2415 from sveltejs/gh-2356

Prevent infinite loops with chained bindings

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

@@ -112,7 +112,6 @@ export default class InlineComponentWrapper extends Wrapper {
 
 		const statements: string[] = [];
 		const updates: string[] = [];
-		const postupdates: string[] = [];
 
 		let props;
 		const name_changes = block.get_unique_name(`${name}_changes`);
@@ -311,8 +310,6 @@ export default class InlineComponentWrapper extends Wrapper {
 				}
 			`);
 
-			postupdates.push(updating);
-
 			const contextual_dependencies = Array.from(binding.expression.contextual_dependencies);
 			const dependencies = Array.from(binding.expression.dependencies);
 
@@ -334,9 +331,9 @@ export default class InlineComponentWrapper extends Wrapper {
 
 				block.builders.init.add_block(deindent`
 					function ${name}(${value}) {
-						if (ctx.${name}.call(null, ${value}, ctx)) {
-							${updating} = true;
-						}
+						ctx.${name}.call(null, ${value}, ctx);
+						${updating} = true;
+						@add_flush_callback(() => ${updating} = false);
 					}
 				`);
 
@@ -344,9 +341,9 @@ export default class InlineComponentWrapper extends Wrapper {
 			} else {
 				block.builders.init.add_block(deindent`
 					function ${name}(${value}) {
-						if (ctx.${name}.call(null, ${value})) {
-							${updating} = true;
-						}
+						ctx.${name}.call(null, ${value});
+						${updating} = true;
+						@add_flush_callback(() => ${updating} = false);
 					}
 				`);
 			}
@@ -354,7 +351,7 @@ export default class InlineComponentWrapper extends Wrapper {
 			const body = deindent`
 				function ${name}(${args.join(', ')}) {
 					${lhs} = ${value};
-					return ${component.invalidate(dependencies[0])}
+					${component.invalidate(dependencies[0])};
 				}
 			`;
 
@@ -453,8 +450,6 @@ export default class InlineComponentWrapper extends Wrapper {
 					else if (${switch_value}) {
 						${name}.$set(${name_changes});
 					}
-
-					${postupdates.length > 0 && `${postupdates.join(' = ')} = false;`}
 				`);
 			}
 
@@ -498,7 +493,6 @@ export default class InlineComponentWrapper extends Wrapper {
 				block.builders.update.add_block(deindent`
 					${updates}
 					${name}.$set(${name_changes});
-					${postupdates.length > 0 && `${postupdates.join(' = ')} = false;`}
 				`);
 			}
 

src/internal/Component.js

@@ -85,16 +85,9 @@ export function init(component, options, instance, create_fragment, not_equal, p
 
 	$$.ctx = instance
 		? instance(component, props, (key, value) => {
-			if ($$.bound[key]) $$.bound[key](value);
-
-			if ($$.ctx) {
-				const changed = not_equal(value, $$.ctx[key]);
-				if (ready && changed) {
-					make_dirty(component, key);
-				}
-
-				$$.ctx[key] = value;
-				return changed;
+			if ($$.ctx && not_equal($$.ctx[key], $$.ctx[key] = value)) {
+				if ($$.bound[key]) $$.bound[key](value);
+				if (ready) make_dirty(component, key);
 			}
 		})
 		: props;

src/internal/scheduler.js

@@ -7,6 +7,7 @@ export const intros = { enabled: false };
 let update_promise;
 const binding_callbacks = [];
 const render_callbacks = [];
+const flush_callbacks = [];
 
 export function schedule_update() {
 	if (!update_promise) {
@@ -15,10 +16,6 @@ export function schedule_update() {
 	}
 }
 
-export function add_render_callback(fn) {
-	render_callbacks.push(fn);
-}
-
 export function tick() {
 	schedule_update();
 	return update_promise;
@@ -28,6 +25,14 @@ export function add_binding_callback(fn) {
 	binding_callbacks.push(fn);
 }
 
+export function add_render_callback(fn) {
+	render_callbacks.push(fn);
+}
+
+export function add_flush_callback(fn) {
+	flush_callbacks.push(fn);
+}
+
 export function flush() {
 	const seen_callbacks = new Set();
 
@@ -56,6 +61,10 @@ export function flush() {
 		}
 	} while (dirty_components.length);
 
+	while (flush_callbacks.length) {
+		flush_callbacks.pop()();
+	}
+
 	update_promise = null;
 }
 

test/runtime/samples/component-binding-blowback-d/One.svelte

@@ -0,0 +1,18 @@
+<script>
+	import Two from './Two.svelte';
+
+	export let list;
+	export let i;
+
+	function handle_click() {
+		list = [...list, {}];
+	}
+</script>
+
+{#each list as item, j}
+	<Two bind:value={item.value} {i} {j}/>
+{/each}
+
+<button on:click={handle_click}>
+	click me
+</button>

test/runtime/samples/component-binding-blowback-d/Two.svelte

@@ -0,0 +1,4 @@
+<script>
+	export let i, j;
+	export let value = `${i}:${j}`;
+</script>

test/runtime/samples/component-binding-blowback-d/_config.js

@@ -0,0 +1,23 @@
+export default {
+	html: `
+		<button>click me</button>
+		<button>click me</button>
+
+		<p>{"value":"0:0"}</p>
+		<p></p>
+	`,
+
+	async test({ assert, target, window }) {
+		const button = target.querySelectorAll('button')[1];
+
+		await button.dispatchEvent(new window.Event('click'));
+
+		assert.htmlEqual(target.innerHTML, `
+			<button>click me</button>
+			<button>click me</button>
+
+			<p>{"value":"0:0"}</p>
+			<p>{"value":"1:0"}</p>
+		`);
+	}
+};

test/runtime/samples/component-binding-blowback-d/main.svelte

@@ -0,0 +1,14 @@
+<script>
+	import One from "./One.svelte";
+
+	const obj = {
+		a: [{}],
+		b: []
+	};
+</script>
+
+<One bind:list={obj.a} i={0}/>
+<One bind:list={obj.b} i={1}/>
+
+<p>{obj.a.map(JSON.stringify)}</p>
+<p>{obj.b.map(JSON.stringify)}</p>

test/runtime/samples/component-binding-blowback-e/One.svelte

@@ -0,0 +1,18 @@
+<script>
+	import Two from './Two.svelte';
+
+	export let list;
+	export let i;
+
+	function handle_click() {
+		list = [...list, {}];
+	}
+</script>
+
+{#each list as item, j}
+	<Two bind:value={item.value} {i} {j}/>
+{/each}
+
+<button on:click={handle_click}>
+	click me
+</button>

test/runtime/samples/component-binding-blowback-e/Two.svelte

@@ -0,0 +1,4 @@
+<script>
+	export let i, j;
+	export let value = { i, j };
+</script>

test/runtime/samples/component-binding-blowback-e/_config.js

@@ -0,0 +1,23 @@
+export default {
+	html: `
+		<button>click me</button>
+		<button>click me</button>
+
+		<p>{"value":{"i":0,"j":0}}</p>
+		<p></p>
+	`,
+
+	async test({ assert, target, window }) {
+		const button = target.querySelectorAll('button')[1];
+
+		await button.dispatchEvent(new window.Event('click'));
+
+		assert.htmlEqual(target.innerHTML, `
+			<button>click me</button>
+			<button>click me</button>
+
+			<p>{"value":{"i":0,"j":0}}</p>
+			<p>{"value":{"i":1,"j":0}}</p>
+		`);
+	}
+};

test/runtime/samples/component-binding-blowback-e/main.svelte

@@ -0,0 +1,14 @@
+<script>
+	import One from "./One.svelte";
+
+	const obj = {
+		a: [{}],
+		b: []
+	};
+</script>
+
+<One bind:list={obj.a} i={0}/>
+<One bind:list={obj.b} i={1}/>
+
+<p>{obj.a.map(JSON.stringify)}</p>
+<p>{obj.b.map(JSON.stringify)}</p>

test/runtime/samples/component-binding-blowback-f/One.svelte

@@ -0,0 +1,18 @@
+<script>
+	import Two from './Two.svelte';
+
+	export let list;
+	export let i;
+
+	function handle_click() {
+		list = [...list, {}];
+	}
+</script>
+
+{#each list as item, j}
+	<Two bind:value={item.value} {i} {j}/>
+{/each}
+
+<button on:click={handle_click}>
+	click me
+</button>

test/runtime/samples/component-binding-blowback-f/Two.svelte

@@ -0,0 +1,10 @@
+<script>
+	import { onMount } from 'svelte';
+
+	export let i, j;
+	export let value;
+
+	onMount(() => {
+		value = { i, j };
+	});
+</script>

test/runtime/samples/component-binding-blowback-f/_config.js

@@ -0,0 +1,31 @@
+export default {
+	html: `
+		<button>click me</button>
+		<button>click me</button>
+
+		<p>{"value":{"i":0,"j":0}}</p>
+		<p></p>
+	`,
+
+	ssrHtml: `
+		<button>click me</button>
+		<button>click me</button>
+
+		<p>{}</p>
+		<p></p>
+	`,
+
+	async test({ assert, target, window }) {
+		const button = target.querySelectorAll('button')[1];
+
+		await button.dispatchEvent(new window.Event('click'));
+
+		assert.htmlEqual(target.innerHTML, `
+			<button>click me</button>
+			<button>click me</button>
+
+			<p>{"value":{"i":0,"j":0}}</p>
+			<p>{"value":{"i":1,"j":0}}</p>
+		`);
+	}
+};

test/runtime/samples/component-binding-blowback-f/main.svelte

@@ -0,0 +1,14 @@
+<script>
+	import One from "./One.svelte";
+
+	const obj = {
+		a: [{}],
+		b: []
+	};
+</script>
+
+<One bind:list={obj.a} i={0}/>
+<One bind:list={obj.b} i={1}/>
+
+<p>{obj.a.map(JSON.stringify)}</p>
+<p>{obj.b.map(JSON.stringify)}</p>