Merge pull request #642 from sveltejs/gh-639-b

mark indirect dependencies of <select> bindings

src/generators/Generator.ts

@@ -33,6 +33,7 @@ export default class Generator {
 	code: MagicString;
 
 	bindingGroups: string[];
+	indirectDependencies: Map<string, Set<string>>;
 	expectedProperties: Set<string>;
 	cascade: boolean;
 	css: string;
@@ -64,6 +65,7 @@ export default class Generator {
 		this.importedComponents = new Map();
 
 		this.bindingGroups = [];
+		this.indirectDependencies = new Map();
 
 		// track which properties are needed, so we can provide useful info
 		// in dev mode
@@ -188,8 +190,20 @@ export default class Generator {
 			},
 		});
 
+		const dependencies = new Set(expression._dependencies || []);
+
+		if (expression._dependencies) {
+			expression._dependencies.forEach((prop: string) => {
+				if (this.indirectDependencies.has(prop)) {
+					this.indirectDependencies.get(prop).forEach(dependency => {
+						dependencies.add(dependency);
+					});
+				}
+			});
+		}
+
 		return {
-			dependencies: expression._dependencies, // TODO probably a better way to do this
+			dependencies: Array.from(dependencies),
 			contexts: usedContexts,
 			snippet: `[✂${expression.start}-${expression.end}✂]`,
 		};

src/generators/dom/Block.ts

@@ -174,7 +174,7 @@ export default class Block {
 		);
 	}
 
-	findDependencies(expression) {
+	findDependencies(expression: Node) {
 		return this.generator.findDependencies(
 			this.contextDependencies,
 			this.indexes,

src/generators/dom/interfaces.ts

@@ -8,4 +8,5 @@ export interface State {
 	inEachBlock?: boolean;
 	allUsedContexts?: string[];
 	usesComponent?: boolean;
+	selectBindingDependencies?: string[];
 }

src/generators/dom/preprocess.ts

@@ -217,6 +217,60 @@ const preprocessors = {
 		state: State,
 		node: Node
 	) => {
+		node.attributes.forEach((attribute: Node) => {
+			if (attribute.type === 'Attribute' && attribute.value !== true) {
+				attribute.value.forEach((chunk: Node) => {
+					if (chunk.type !== 'Text') {
+						const dependencies = block.findDependencies(chunk.expression);
+						block.addDependencies(dependencies);
+
+						// special case — <option value='{{foo}}'> — see below
+						if (node.name === 'option' && attribute.name === 'value' && state.selectBindingDependencies) {
+							state.selectBindingDependencies.forEach(prop => {
+								dependencies.forEach((dependency: string) => {
+									generator.indirectDependencies.get(prop).add(dependency);
+								});
+							});
+						}
+					}
+				});
+			} else if (attribute.type === 'Binding') {
+				const dependencies = block.findDependencies(attribute.value);
+				block.addDependencies(dependencies);
+			} else if (attribute.type === 'Transition') {
+				if (attribute.intro)
+					generator.hasIntroTransitions = block.hasIntroMethod = true;
+				if (attribute.outro) {
+					generator.hasOutroTransitions = block.hasOutroMethod = true;
+					block.outros += 1;
+				}
+			}
+		});
+
+		// special case — in a case like this...
+		//
+		//   <select bind:value='foo'>
+		//     <option value='{{bar}}'>bar</option>
+		//     <option value='{{baz}}'>baz</option>
+		//   </option>
+		//
+		// ...we need to know that `foo` depends on `bar` and `baz`,
+		// so that if `foo.qux` changes, we know that we need to
+		// mark `bar` and `baz` as dirty too
+		if (node.name === 'select') {
+			const value = node.attributes.find((attribute: Node) => attribute.name === 'value');
+			if (value) {
+				// TODO does this also apply to e.g. `<input type='checkbox' bind:group='foo'>`?
+				const dependencies = block.findDependencies(value.value);
+				state.selectBindingDependencies = dependencies;
+				dependencies.forEach((prop: string) => {
+					generator.indirectDependencies.set(prop, new Set());
+				});
+			} else {
+				state.selectBindingDependencies = null;
+			}
+		}
+
 		const isComponent =
 			generator.components.has(node.name) || node.name === ':Self';
 
@@ -239,27 +293,6 @@ const preprocessors = {
 			});
 		}
 
-		node.attributes.forEach((attribute: Node) => {
-			if (attribute.type === 'Attribute' && attribute.value !== true) {
-				attribute.value.forEach((chunk: Node) => {
-					if (chunk.type !== 'Text') {
-						const dependencies = block.findDependencies(chunk.expression);
-						block.addDependencies(dependencies);
-					}
-				});
-			} else if (attribute.type === 'Binding') {
-				const dependencies = block.findDependencies(attribute.value);
-				block.addDependencies(dependencies);
-			} else if (attribute.type === 'Transition') {
-				if (attribute.intro)
-					generator.hasIntroTransitions = block.hasIntroMethod = true;
-				if (attribute.outro) {
-					generator.hasOutroTransitions = block.hasOutroMethod = true;
-					block.outros += 1;
-				}
-			}
-		});
-
 		if (node.children.length) {
 			if (isComponent) {
 				const name = block.getUniqueName(

src/generators/dom/visitors/shared/binding/getSetter.ts

@@ -25,7 +25,10 @@ export default function getSetter({
 			${computed && `var state = ${block.component}.get();`}
 			list[index]${tail} = ${value};
 
-			${block.component}._set({ ${prop}: ${block.component}.get( '${prop}' ) });
+			${computed ?
+				`${block.component}._set({ ${dependencies.map((prop: string) => `${prop}: state.${prop}`).join(', ')} });` :
+				`${block.component}._set({ ${dependencies.map((prop: string) => `${prop}: ${block.component}.get( '${prop}' )`).join(', ')} });`
+			}
 		`;
 	}
 
@@ -35,7 +38,7 @@ export default function getSetter({
 		return deindent`
 			var state = ${block.component}.get();
 			${snippet} = ${value};
-			${block.component}._set({ ${name}: state.${name} });
+			${block.component}._set({ ${dependencies.map((prop: string) => `${prop}: state.${prop}`).join(', ')} });
 		`;
 	}
 

test/runtime/samples/binding-indirect/_config.js

@@ -0,0 +1,91 @@
+const tasks = [
+	{ description: 'put your left leg in', done: false },
+	{ description: 'your left leg out', done: false },
+	{ description: 'in, out, in, out', done: false },
+	{ description: 'shake it all about', done: false }
+];
+
+export default {
+	'skip-ssr': true,
+	allowES2015: true,
+
+	data: {
+		tasks,
+		selected: tasks[0]
+	},
+
+	html: `
+		<select>
+			<option value='[object Object]'>put your left leg in</option>
+			<option value='[object Object]'>your left leg out</option>
+			<option value='[object Object]'>in, out, in, out</option>
+			<option value='[object Object]'>shake it all about</option>
+		</select>
+
+		<label>
+			<input type='checkbox'> put your left leg in
+		</label>
+
+		<h2>Pending tasks</h2>
+		<p>put your left leg in</p>
+		<p>your left leg out</p>
+		<p>in, out, in, out</p>
+		<p>shake it all about</p>
+	`,
+
+	test(assert, component, target, window) {
+		const input = target.querySelector('input');
+		const select = target.querySelector('select');
+		const options = target.querySelectorAll('option');
+
+		const change = new window.Event('change');
+
+		input.checked = true;
+		input.dispatchEvent(change);
+
+		assert.ok(component.get('tasks')[0].done);
+		assert.htmlEqual(target.innerHTML, `
+			<select>
+				<option value='[object Object]'>put your left leg in</option>
+				<option value='[object Object]'>your left leg out</option>
+				<option value='[object Object]'>in, out, in, out</option>
+				<option value='[object Object]'>shake it all about</option>
+			</select>
+
+			<label>
+				<input type='checkbox'> put your left leg in
+			</label>
+
+			<h2>Pending tasks</h2>
+			<p>your left leg out</p>
+			<p>in, out, in, out</p>
+			<p>shake it all about</p>
+		`);
+
+		options[1].selected = true;
+		select.dispatchEvent(change);
+		assert.equal(component.get('selected'), tasks[1]);
+		assert.ok(!input.checked);
+
+		input.checked = true;
+		input.dispatchEvent(change);
+
+		assert.ok(component.get('tasks')[1].done);
+		assert.htmlEqual(target.innerHTML, `
+			<select>
+				<option value='[object Object]'>put your left leg in</option>
+				<option value='[object Object]'>your left leg out</option>
+				<option value='[object Object]'>in, out, in, out</option>
+				<option value='[object Object]'>shake it all about</option>
+			</select>
+
+			<label>
+				<input type='checkbox'> your left leg out
+			</label>
+
+			<h2>Pending tasks</h2>
+			<p>in, out, in, out</p>
+			<p>shake it all about</p>
+		`);
+	}
+};

test/runtime/samples/binding-indirect/main.html

@@ -0,0 +1,14 @@
+<select bind:value='selected'>
+	{{#each tasks as task}}
+		<option value='{{task}}'>{{task.description}}</option>
+	{{/each}}
+</select>
+
+<label>
+	<input type='checkbox' bind:checked='selected.done'> {{selected.description}}
+</label>
+
+<h2>Pending tasks</h2>
+{{#each tasks.filter(t => !t.done) as task}}
+	<p>{{task.description}}</p>
+{{/each}}