From 93e51d6ef3bccc5a3948fc5f0dcfdde9d1f413ca Mon Sep 17 00:00:00 2001
From: Rich Harris <richard.a.harris@gmail.com>
Date: Mon, 10 Apr 2017 13:50:12 -0400
Subject: [PATCH] correctly bind one-way select value attributes (#423)

---
 .../dom/visitors/Element/Attribute.js         | 35 +++++++++++++++++--
 .../dom/visitors/Element/Element.js           | 14 ++++++++
 .../select-one-way-bind-object/_config.js     | 24 +++++++++++++
 .../select-one-way-bind-object/main.html      |  4 +++
 4 files changed, 74 insertions(+), 3 deletions(-)
 create mode 100644 test/runtime/samples/select-one-way-bind-object/_config.js
 create mode 100644 test/runtime/samples/select-one-way-bind-object/main.html

diff --git a/src/generators/dom/visitors/Element/Attribute.js b/src/generators/dom/visitors/Element/Attribute.js
index 235c5b8341..d838860276 100644
--- a/src/generators/dom/visitors/Element/Attribute.js
+++ b/src/generators/dom/visitors/Element/Attribute.js
@@ -46,9 +46,38 @@ export default function visitAttribute ( generator, block, state, node, attribut
 		const last = `last_${state.parentNode}_${name.replace( /-/g, '_')}`;
 		block.builders.create.addLine( `var ${last} = ${value};` );
 
-		const updater = propertyName ?
-			`${state.parentNode}.${propertyName} = ${last};` :
-			`${generator.helper( method )}( ${state.parentNode}, '${name}', ${last} );`;
+		const isSelectValueAttribute = name === 'value' && state.parentNodeName === 'select';
+
+		let updater;
+
+		if ( isSelectValueAttribute ) {
+			// annoying special case
+			const isMultipleSelect = node.name === 'select' && node.attributes.find( attr => attr.name.toLowerCase() === 'multiple' ); // TODO use getStaticAttributeValue
+			const i = block.getUniqueName( 'i' );
+			const option = block.getUniqueName( 'option' );
+
+			const ifStatement = isMultipleSelect ?
+				deindent`
+					${option}.selected = ~${last}.indexOf( ${option}.__value );` :
+				deindent`
+					if ( ${option}.__value === ${last} ) {
+						${option}.selected = true;
+						break;
+					}`;
+
+			updater = deindent`
+				var ${last} = ${last};
+				for ( var ${i} = 0; ${i} < ${state.parentNode}.options.length; ${i} += 1 ) {
+					var ${option} = ${state.parentNode}.options[${i}];
+
+					${ifStatement}
+				}
+			`;
+		} else if ( propertyName ) {
+			updater = `${state.parentNode}.${propertyName} = ${last};`;
+		} else {
+			updater = `${generator.helper( method )}( ${state.parentNode}, '${name}', ${last} );`;
+		}
 
 		block.builders.create.addLine( updater );
 		block.builders.update.addBlock( deindent`
diff --git a/src/generators/dom/visitors/Element/Element.js b/src/generators/dom/visitors/Element/Element.js
index b27f7e5d53..9b451f1348 100644
--- a/src/generators/dom/visitors/Element/Element.js
+++ b/src/generators/dom/visitors/Element/Element.js
@@ -39,6 +39,7 @@ export default function visitElement ( generator, block, state, node ) {
 	const childState = Object.assign( {}, state, {
 		isTopLevel: false,
 		parentNode: name,
+		parentNodeName: node.name,
 		namespace: node.name === 'svg' ? 'http://www.w3.org/2000/svg' : state.namespace,
 		allUsedContexts: []
 	});
@@ -55,9 +56,18 @@ export default function visitElement ( generator, block, state, node ) {
 		block.builders.create.addLine( `${generator.helper( 'setAttribute' )}( ${name}, '${generator.cssId}', '' );` );
 	}
 
+	let selectValueAttribute;
+
 	node.attributes
 		.sort( ( a, b ) => order[ a.type ] - order[ b.type ] )
 		.forEach( attribute => {
+			// <select> value attributes are an annoying special case — it must be handled
+			// *after* its children have been updated
+			if ( attribute.type === 'Attribute' && attribute.name === 'value' && node.name === 'select' ) {
+				selectValueAttribute = attribute;
+				return;
+			}
+
 			visitors[ attribute.type ]( generator, block, childState, node, attribute );
 		});
 
@@ -106,6 +116,10 @@ export default function visitElement ( generator, block, state, node ) {
 	node.children.forEach( child => {
 		visit( generator, block, childState, child );
 	});
+
+	if ( selectValueAttribute ) {
+		visitAttribute( generator, block, childState, node, selectValueAttribute );
+	}
 }
 
 function getRenderStatement ( generator, namespace, name ) {
diff --git a/test/runtime/samples/select-one-way-bind-object/_config.js b/test/runtime/samples/select-one-way-bind-object/_config.js
new file mode 100644
index 0000000000..946eaabceb
--- /dev/null
+++ b/test/runtime/samples/select-one-way-bind-object/_config.js
@@ -0,0 +1,24 @@
+const items = [ {}, {} ];
+
+export default {
+	skip: true, // JSDOM quirks
+
+	'skip-ssr': true,
+
+	data: {
+		foo: items[0],
+		items
+	},
+
+	test ( assert, component, target ) {
+		const options = target.querySelectorAll( 'option' );
+
+		assert.equal( options[0].selected, true );
+		assert.equal( options[1].selected, false );
+
+		component.set( { foo: items[1] } );
+
+		assert.equal( options[0].selected, false );
+		assert.equal( options[1].selected, true );
+	}
+};
diff --git a/test/runtime/samples/select-one-way-bind-object/main.html b/test/runtime/samples/select-one-way-bind-object/main.html
new file mode 100644
index 0000000000..6f74305c53
--- /dev/null
+++ b/test/runtime/samples/select-one-way-bind-object/main.html
@@ -0,0 +1,4 @@
+<select value="{{foo}}">
+	<option value='{{items[0]}}'>a</option>
+	<option value='{{items[1]}}'>b</option>
+</select>