Merge pull request #591 from sveltejs/gh-590

On `<select>`, don't try generating prop code until visiting attributes
sveltejs · May 21, 2017 · 2194de9 · 2194de9
2 parents 7c2fd8e + a3860f2
commit 2194de9
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 34 deletions.
diff --git a/src/generators/dom/visitors/Element/Element.js b/src/generators/dom/visitors/Element/Element.js
@@ -46,7 +46,7 @@ export default function visitElement ( generator, block, state, node ) {
 		block.builders.create.addLine( `${generator.helper( 'setAttribute' )}( ${name}, '${generator.cssId}', '' );` );
 	}
 
-	function visitAttributes () {
+	function visitAttributesAndAddProps () {
 		let intro;
 		let outro;
 
@@ -63,6 +63,37 @@ export default function visitElement ( generator, block, state, node ) {
 			});
 
 		if ( intro || outro ) addTransitions( generator, block, childState, node, intro, outro );
+
+		if ( childState.allUsedContexts.length || childState.usesComponent ) {
+			const initialProps = [];
+			const updates = [];
+
+			if ( childState.usesComponent ) {
+				initialProps.push( `component: ${block.component}` );
+			}
+
+			childState.allUsedContexts.forEach( contextName => {
+				if ( contextName === 'state' ) return;
+
+				const listName = block.listNames.get( contextName );
+				const indexName = block.indexNames.get( contextName );
+
+				initialProps.push( `${listName}: ${listName},\n${indexName}: ${indexName}` );
+				updates.push( `${name}._svelte.${listName} = ${listName};\n${name}._svelte.${indexName} = ${indexName};` );
+			});
+
+			if ( initialProps.length ) {
+				block.builders.create.addBlock( deindent`
+					${name}._svelte = {
+						${initialProps.join( ',\n' )}
+					};
+				` );
+			}
+
+			if ( updates.length ) {
+				block.builders.update.addBlock( updates.join( '\n' ) );
+			}
+		}
 	}
 
 	if ( !state.parentNode ) {
@@ -74,7 +105,7 @@ export default function visitElement ( generator, block, state, node ) {
 	if ( node.name !== 'select' ) {
 		// <select> value attributes are an annoying special case — it must be handled
 		// *after* its children have been updated
-		visitAttributes();
+		visitAttributesAndAddProps();
 	}
 
 	// special case – bound <option> without a value attribute
@@ -83,37 +114,6 @@ export default function visitElement ( generator, block, state, node ) {
 		node.initialUpdate = node.lateUpdate = statement;
 	}
 
-	if ( childState.allUsedContexts.length || childState.usesComponent ) {
-		const initialProps = [];
-		const updates = [];
-
-		if ( childState.usesComponent ) {
-			initialProps.push( `component: ${block.component}` );
-		}
-
-		childState.allUsedContexts.forEach( contextName => {
-			if ( contextName === 'state' ) return;
-
-			const listName = block.listNames.get( contextName );
-			const indexName = block.indexNames.get( contextName );
-
-			initialProps.push( `${listName}: ${listName},\n${indexName}: ${indexName}` );
-			updates.push( `${name}._svelte.${listName} = ${listName};\n${name}._svelte.${indexName} = ${indexName};` );
-		});
-
-		if ( initialProps.length ) {
-			block.builders.create.addBlock( deindent`
-				${name}._svelte = {
-					${initialProps.join( ',\n' )}
-				};
-			` );
-		}
-
-		if ( updates.length ) {
-			block.builders.update.addBlock( updates.join( '\n' ) );
-		}
-	}
-
 	node.children.forEach( child => {
 		visit( generator, block, childState, child );
 	});
@@ -123,7 +123,7 @@ export default function visitElement ( generator, block, state, node ) {
 	}
 
 	if ( node.name === 'select' ) {
-		visitAttributes();
+		visitAttributesAndAddProps();
 	}
 
 	if ( node.initialUpdate ) {

diff --git a/test/runtime/samples/select-props/_config.js b/test/runtime/samples/select-props/_config.js
@@ -0,0 +1,15 @@
+export default {
+	test ( assert, component, target, window ) {
+		const selects = document.querySelectorAll( 'select' );
+
+		const event1 = new window.Event( 'change' );
+		selects[0].value = 'b';
+		selects[0].dispatchEvent(event1);
+
+		const event2 = new window.Event( 'change' );
+		selects[1].value = 'b';
+		selects[1].dispatchEvent(event2);
+
+		assert.deepEqual( component.get( 'log' ), [ 1, 2 ] );
+	}
+};
diff --git a/test/runtime/samples/select-props/main.html b/test/runtime/samples/select-props/main.html
@@ -0,0 +1,19 @@
+{{#each foo as bar}}
+	<select on:change='handler(bar)'>
+		<option>a</option>
+		<option>b</option>
+	</select>
+{{/each}}
+
+<script>
+	export default {
+		data: () => ({ foo: [ 1, 2 ], log: [] }),
+		methods: {
+			handler ( bar ) {
+				let { log } = this.get();
+				log.push( bar );
+				this.set( { log } );
+			}
+		}
+	}
+</script>