From 4116b9d8b12548a0512f4245ad7996fd11ed024c Mon Sep 17 00:00:00 2001
From: Rich Harris <richard.a.harris@gmail.com>
Date: Sat, 18 Nov 2017 14:07:21 -0500
Subject: [PATCH 1/3] use <option> children as value attribute, if none exists.
 fixes #928

---
 src/generators/dom/preprocess.ts              | 20 +++++++---
 .../server-side-rendering/preprocess.ts       | 13 ++++++
 .../_config.js                                | 40 +++++++++++++++++++
 .../main.html                                 |  7 ++++
 4 files changed, 75 insertions(+), 5 deletions(-)
 create mode 100644 test/runtime/samples/binding-select-implicit-option-value/_config.js
 create mode 100644 test/runtime/samples/binding-select-implicit-option-value/main.html

diff --git a/src/generators/dom/preprocess.ts b/src/generators/dom/preprocess.ts
index 94b8ea444d..33a305146f 100644
--- a/src/generators/dom/preprocess.ts
+++ b/src/generators/dom/preprocess.ts
@@ -358,6 +358,19 @@ const preprocessors = {
 			}
 		});
 
+		const valueAttribute = node.attributes.find((attribute: Node) => attribute.name === 'value');
+
+		// Treat these the same way:
+		//   <option>{{foo}}</option>
+		//   <option value='{{foo}}'>{{foo}}</option>
+		if (node.name === 'option' && !valueAttribute) {
+			node.attributes.push({
+				type: 'Attribute',
+				name: 'value',
+				value: node.children
+			});
+		}
+
 		// special case — in a case like this...
 		//
 		//   <select bind:value='foo'>
@@ -369,12 +382,9 @@ const preprocessors = {
 		// 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) {
+			if (valueAttribute) {
 				// TODO does this also apply to e.g. `<input type='checkbox' bind:group='foo'>`?
-				const dependencies = block.findDependencies(value.value);
+				const dependencies = block.findDependencies(valueAttribute.value);
 				state.selectBindingDependencies = dependencies;
 				dependencies.forEach((prop: string) => {
 					generator.indirectDependencies.set(prop, new Set());
diff --git a/src/generators/server-side-rendering/preprocess.ts b/src/generators/server-side-rendering/preprocess.ts
index a8a790f754..afddae077c 100644
--- a/src/generators/server-side-rendering/preprocess.ts
+++ b/src/generators/server-side-rendering/preprocess.ts
@@ -69,6 +69,19 @@ const preprocessors = {
 			if (slot && isChildOfComponent(node, generator)) {
 				node.slotted = true;
 			}
+
+			// Treat these the same way:
+			//   <option>{{foo}}</option>
+			//   <option value='{{foo}}'>{{foo}}</option>
+			const valueAttribute = node.attributes.find((attribute: Node) => attribute.name === 'value');
+
+			if (node.name === 'option' && !valueAttribute) {
+				node.attributes.push({
+					type: 'Attribute',
+					name: 'value',
+					value: node.children
+				});
+			}
 		}
 
 		if (node.children.length) {
diff --git a/test/runtime/samples/binding-select-implicit-option-value/_config.js b/test/runtime/samples/binding-select-implicit-option-value/_config.js
new file mode 100644
index 0000000000..40a7364797
--- /dev/null
+++ b/test/runtime/samples/binding-select-implicit-option-value/_config.js
@@ -0,0 +1,40 @@
+export default {
+	data: {
+		values: [1, 2, 3],
+		foo: 2
+	},
+
+	html: `
+		<select>
+			<option value='1'>1</option>
+			<option value='2'>2</option>
+			<option value='3'>3</option>
+		</select>
+
+		<p>foo: 2</p>
+	`,
+
+	test(assert, component, target, window) {
+		const select = target.querySelector('select');
+		const options = [...target.querySelectorAll('option')];
+
+		assert.ok(options[1].selected);
+		assert.equal(component.get('foo'), 2);
+
+		const change = new window.Event('change');
+
+		options[2].selected = true;
+		select.dispatchEvent(change);
+
+		assert.equal(component.get('foo'), 3);
+		assert.htmlEqual( target.innerHTML, `
+			<select>
+				<option value='1'>1</option>
+				<option value='2'>2</option>
+				<option value='3'>3</option>
+			</select>
+
+			<p>foo: 3</p>
+		` );
+	}
+};
diff --git a/test/runtime/samples/binding-select-implicit-option-value/main.html b/test/runtime/samples/binding-select-implicit-option-value/main.html
new file mode 100644
index 0000000000..ec5cc84a8d
--- /dev/null
+++ b/test/runtime/samples/binding-select-implicit-option-value/main.html
@@ -0,0 +1,7 @@
+<select bind:value='foo'>
+	{{#each values as v}}
+		<option>{{v}}</option>
+	{{/each}}
+</select>
+
+<p>foo: {{foo}}</p>
\ No newline at end of file

From cd97521daa76d4d2370ba72c830891266732fa4b Mon Sep 17 00:00:00 2001
From: Rich Harris <richard.a.harris@gmail.com>
Date: Sat, 18 Nov 2017 14:13:31 -0500
Subject: [PATCH 2/3] update tests

---
 .../_config.js                                       |  6 +++---
 .../samples/binding-select-initial-value/_config.js  |  6 +++---
 test/runtime/samples/binding-select-late/_config.js  |  6 +++---
 test/runtime/samples/binding-select/_config.js       | 12 ++++++------
 4 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/test/runtime/samples/binding-select-initial-value-undefined/_config.js b/test/runtime/samples/binding-select-initial-value-undefined/_config.js
index d5944ae815..f625000edb 100644
--- a/test/runtime/samples/binding-select-initial-value-undefined/_config.js
+++ b/test/runtime/samples/binding-select-initial-value-undefined/_config.js
@@ -5,9 +5,9 @@ export default {
 		<p>selected: a</p>
 
 		<select>
-			<option>a</option>
-			<option>b</option>
-			<option>c</option>
+			<option value='a'>a</option>
+			<option value='b'>b</option>
+			<option value='c'>c</option>
 		</select>
 
 		<p>selected: a</p>
diff --git a/test/runtime/samples/binding-select-initial-value/_config.js b/test/runtime/samples/binding-select-initial-value/_config.js
index 99a7129262..7338af165f 100644
--- a/test/runtime/samples/binding-select-initial-value/_config.js
+++ b/test/runtime/samples/binding-select-initial-value/_config.js
@@ -3,9 +3,9 @@ export default {
 		<p>selected: b</p>
 
 		<select>
-			<option>a</option>
-			<option>b</option>
-			<option>c</option>
+			<option value='a'>a</option>
+			<option value='b'>b</option>
+			<option value='c'>c</option>
 		</select>
 
 		<p>selected: b</p>
diff --git a/test/runtime/samples/binding-select-late/_config.js b/test/runtime/samples/binding-select-late/_config.js
index 2fffa0e4ed..04b94613fc 100644
--- a/test/runtime/samples/binding-select-late/_config.js
+++ b/test/runtime/samples/binding-select-late/_config.js
@@ -22,9 +22,9 @@ export default {
 
 		assert.htmlEqual( target.innerHTML, `
 			<select>
-				<option>one</option>
-				<option>two</option>
-				<option>three</option>
+				<option value='one'>one</option>
+				<option value='two'>two</option>
+				<option value='three'>three</option>
 			</select>
 			<p>selected: two</p>
 		` );
diff --git a/test/runtime/samples/binding-select/_config.js b/test/runtime/samples/binding-select/_config.js
index 7fce00f327..8c8eced7bc 100644
--- a/test/runtime/samples/binding-select/_config.js
+++ b/test/runtime/samples/binding-select/_config.js
@@ -3,9 +3,9 @@ export default {
 		<p>selected: one</p>
 
 		<select>
-			<option>one</option>
-			<option>two</option>
-			<option>three</option>
+			<option value='one'>one</option>
+			<option value='two'>two</option>
+			<option value='three'>three</option>
 		</select>
 
 		<p>selected: one</p>
@@ -32,9 +32,9 @@ export default {
 			<p>selected: two</p>
 
 			<select>
-				<option>one</option>
-				<option>two</option>
-				<option>three</option>
+				<option value='one'>one</option>
+				<option value='two'>two</option>
+				<option value='three'>three</option>
 			</select>
 
 			<p>selected: two</p>

From 481f7bc37c07f9d583d3903362644133b32a8e25 Mon Sep 17 00:00:00 2001
From: Rich Harris <richard.a.harris@gmail.com>
Date: Sat, 18 Nov 2017 14:17:07 -0500
Subject: [PATCH 3/3] remove unused code

---
 src/generators/dom/visitors/Element/Element.ts | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/src/generators/dom/visitors/Element/Element.ts b/src/generators/dom/visitors/Element/Element.ts
index 6c7a307fab..f678dec9e8 100644
--- a/src/generators/dom/visitors/Element/Element.ts
+++ b/src/generators/dom/visitors/Element/Element.ts
@@ -190,19 +190,6 @@ export default function visitElement(
 		visitAttributesAndAddProps();
 	}
 
-	// special case – bound <option> without a value attribute
-	if (
-		node.name === 'option' &&
-		!node.attributes.find(
-			(attribute: Node) =>
-				attribute.type === 'Attribute' && attribute.name === 'value'
-		)
-	) {
-		// TODO check it's bound
-		const statement = `${name}.__value = ${name}.textContent;`;
-		node.initialUpdate = node.lateUpdate = statement;
-	}
-
 	if (!childState.namespace && node.canUseInnerHTML && node.children.length > 0) {
 		if (node.children.length === 1 && node.children[0].type === 'Text') {
 			block.builders.create.addLine(
@@ -219,10 +206,6 @@ export default function visitElement(
 		});
 	}
 
-	if (node.lateUpdate) {
-		block.builders.update.addLine(node.lateUpdate);
-	}
-
 	if (node.name === 'select') {
 		visitAttributesAndAddProps();
 	}