Merge pull request #931 from sveltejs/gh-928

Reuse <option> children as value attribute, if there isn't one

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(
+			if (valueAttribute) {
-				(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);
+				const dependencies = block.findDependencies(valueAttribute.value);
 				state.selectBindingDependencies = dependencies;
 				dependencies.forEach((prop: string) => {
 					generator.indirectDependencies.set(prop, new Set());

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();
 	}

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) {

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>
+		` );
+	}
+};

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>

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 value='a'>a</option>
-			<option>b</option>
+			<option value='b'>b</option>
-			<option>c</option>
+			<option value='c'>c</option>
 		</select>
 
 		<p>selected: a</p>

test/runtime/samples/binding-select-initial-value/_config.js

@@ -3,9 +3,9 @@ export default {
 		<p>selected: b</p>
 
 		<select>
-			<option>a</option>
+			<option value='a'>a</option>
-			<option>b</option>
+			<option value='b'>b</option>
-			<option>c</option>
+			<option value='c'>c</option>
 		</select>
 
 		<p>selected: b</p>

test/runtime/samples/binding-select-late/_config.js

@@ -22,9 +22,9 @@ export default {
 
 		assert.htmlEqual( target.innerHTML, `
 			<select>
-				<option>one</option>
+				<option value='one'>one</option>
-				<option>two</option>
+				<option value='two'>two</option>
-				<option>three</option>
+				<option value='three'>three</option>
 			</select>
 			<p>selected: two</p>
 		` );

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

@@ -3,9 +3,9 @@ export default {
 		<p>selected: one</p>
 
 		<select>
-			<option>one</option>
+			<option value='one'>one</option>
-			<option>two</option>
+			<option value='two'>two</option>
-			<option>three</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 value='one'>one</option>
-				<option>two</option>
+				<option value='two'>two</option>
-				<option>three</option>
+				<option value='three'>three</option>
 			</select>
 
 			<p>selected: two</p>