Merge pull request #432 from sveltejs/binding-mutation-bug

prevent hard-to-reproduce bug with deep two-way bindings

src/generators/dom/visitors/attributes/addComponentBinding.js

@@ -3,7 +3,7 @@ import flattenReference from '../../../../utils/flattenReference.js';
 import getSetter from './binding/getSetter.js';
 
 export default function createBinding ( generator, node, attribute, current, local ) {
-	const { name } = flattenReference( attribute.value );
+	const { name, keypath } = flattenReference( attribute.value );
 	const { snippet, contexts, dependencies } = generator.contextualise( attribute.value );
 
 	if ( dependencies.length > 1 ) throw new Error( 'An unexpected situation arose. Please raise an issue at https://github.com/sveltejs/svelte/issues — thanks!' );
@@ -35,7 +35,7 @@ export default function createBinding ( generator, node, attribute, current, loc
 		prop
 	});
 
-	const setter = getSetter({ current, name, context: '_context', attribute, dependencies, snippet, value: 'value' });
+	const setter = getSetter({ current, name, keypath, context: '_context', attribute, dependencies, value: 'value' });
 
 	generator.hasComplexBindings = true;
 

src/generators/dom/visitors/attributes/addElementBinding.js

@@ -21,7 +21,7 @@ export default function createBinding ( generator, node, attribute, current, loc
 	const value = getBindingValue( generator, local, node, attribute, isMultipleSelect, bindingGroup, type );
 	const eventName = getBindingEventName( node );
 
-	let setter = getSetter({ current, name, context: '__svelte', attribute, dependencies, snippet, value });
+	let setter = getSetter({ current, name, keypath, context: '__svelte', attribute, dependencies, value });
 	let updateElement;
 
 	// <select> special case

src/generators/dom/visitors/attributes/binding/getSetter.js

@@ -1,6 +1,6 @@
 import deindent from '../../../../../utils/deindent.js';
 
-export default function getSetter ({ current, name, context, attribute, dependencies, snippet, value }) {
+export default function getSetter ({ current, name, keypath, context, attribute, dependencies, value }) {
 	if ( current.contexts.has( name ) ) {
 		const prop = dependencies[0];
 		const tail = attribute.value.type === 'MemberExpression' ? getTailSnippet( attribute.value ) : '';
@@ -17,7 +17,7 @@ export default function getSetter ({ current, name, context, attribute, dependen
 	if ( attribute.value.type === 'MemberExpression' ) {
 		return deindent`
 			var ${name} = ${current.component}.get( '${name}' );
-			${snippet} = ${value};
+			${keypath} = ${value};
 			${current.component}._set({ ${name}: ${name} });
 		`;
 	}

src/utils/flattenReference.js

@@ -1,16 +1,19 @@
 export default function flatten ( node ) {
 	const parts = [];
+	const propEnd = node.end;
+
 	while ( node.type === 'MemberExpression' ) {
 		if ( node.computed ) return null;
 		parts.unshift( node.property.name );
 
 		node = node.object;
 	}
 
+	const propStart = node.end;
 	const name = node.type === 'Identifier' ? node.name : node.type === 'ThisExpression' ? 'this' : null;
 
 	if ( !name ) return null;
 
 	parts.unshift( name );
-	return { name, parts, keypath: parts.join( '.' ) };
+	return { name, parts, keypath: `${name}[✂${propStart}-${propEnd}✂]` };
 }

test/generator/samples/component-binding-deep-b/ComponentSelector.html

@@ -0,0 +1,5 @@
+<select bind:value='selectedComponent'>
+	{{#each components as component}}
+		<option value='{{component}}'>{{component.name}}.html</option>
+	{{/each}}
+</select>

test/generator/samples/component-binding-deep-b/Editor.html

@@ -0,0 +1 @@
+<textarea bind:value='code'></textarea>

test/generator/samples/component-binding-deep-b/_config.js

@@ -0,0 +1,84 @@
+const components = [
+	{
+		name: 'One',
+		source: 'one source'
+	},
+	{
+		name: 'Two',
+		source: 'two source'
+	}
+];
+
+const selectedComponent = components[0];
+
+export default {
+	skip: true, // doesn't reflect real-world bug, maybe a JSDOM quirk
+
+	data: {
+		components,
+		selectedComponent
+	},
+
+	html: `
+		<select>
+			<option value='[object Object]'>One.html</option>
+			<option value='[object Object]'>Two.html</option>
+		</select>
+
+		<textarea></textarea>
+
+		<pre>ONE SOURCE\nTWO SOURCE</pre>
+	`,
+
+	test ( assert, component, target, window ) {
+		const event = new window.MouseEvent( 'input' );
+		const textarea = target.querySelector( 'textarea' );
+
+		textarea.value = 'one source changed';
+		textarea.dispatchEvent( event );
+
+		assert.equal( component.get( 'compiled' ), 'ONE SOURCE CHANGED\nTWO SOURCE' );
+		assert.htmlEqual( target.innerHTML, `
+			<select>
+				<option value='[object Object]'>One.html</option>
+				<option value='[object Object]'>Two.html</option>
+			</select>
+
+			<textarea></textarea>
+
+			<pre>ONE SOURCE CHANGED\nTWO SOURCE</pre>
+		` );
+
+		// const select = target.querySelector( 'select' );
+		// console.log( `select.options[0].selected`, select.options[0].selected )
+		// console.log( `select.options[1].selected`, select.options[1].selected )
+		// console.log( `select.value`, select.value )
+		// console.log( `select.__value`, select.__value )
+		// select.options[1].selected = true;
+		// console.log( `select.options[0].selected`, select.options[0].selected )
+		// console.log( `select.options[1].selected`, select.options[1].selected )
+		// console.log( `select.value`, select.value )
+		// console.log( `select.__value`, select.__value )
+		// select.dispatchEvent( new window.Event( 'change' ) );
+		component.set({ selectedComponent: components[1] });
+
+		assert.equal( textarea.value, 'two source' );
+
+		textarea.value = 'two source changed';
+		textarea.dispatchEvent( event );
+
+		assert.equal( component.get( 'compiled' ), 'ONE SOURCE CHANGED\nTWO SOURCE CHANGED' );
+		assert.htmlEqual( target.innerHTML, `
+			<select>
+				<option value='[object Object]'>One.html</option>
+				<option value='[object Object]'>Two.html</option>
+			</select>
+
+			<textarea></textarea>
+
+			<pre>ONE SOURCE CHANGED\nTWO SOURCE CHANGED</pre>
+		` );
+
+		component.destroy();
+	}
+};

test/generator/samples/component-binding-deep-b/main.html

@@ -0,0 +1,42 @@
+<ComponentSelector :components bind:selectedComponent/>
+<Editor bind:code='selectedComponent.source'/>
+
+<pre>
+{{compiled}}
+</pre>
+
+<script>
+	import Editor from './Editor.html';
+	import ComponentSelector from './ComponentSelector.html';
+
+	export default {
+		components: {
+			ComponentSelector,
+			Editor
+		},
+
+		oncreate () {
+			this.observe( 'components', components => {
+				components.forEach( component => {
+					if ( component === this.get( 'selectedComponent' ) ) return;
+					component.compiled = component.source.toUpperCase();
+				});
+			});
+
+			this.observe( 'selectedComponent', component => {
+				component.compiled = component.source.toUpperCase();
+				this.updateBundle();
+			});
+		},
+
+		methods: {
+			updateBundle () {
+				const components = this.get( 'components' );
+
+				const compiled = components.map( component => component.compiled ).join( '\n' );
+
+				this.set({ compiled });
+			}
+		}
+	}
+</script>

test/sourcemaps/samples/binding/test.js

@@ -1,10 +1,10 @@
 export function test ({ assert, smc, locateInSource, locateInGenerated }) {
-	const expected = locateInSource( 'foo.bar.baz' );
+	const expected = locateInSource( 'bar.baz' );
 
 	let loc;
 	let actual;
 
-	loc = locateInGenerated( 'foo.bar.baz' );
+	loc = locateInGenerated( 'bar.baz' );
 
 	actual = smc.originalPositionFor({
 		line: loc.line + 1,
@@ -18,7 +18,7 @@ export function test ({ assert, smc, locateInSource, locateInGenerated }) {
 		column: expected.column
 	});
 
-	loc = locateInGenerated( 'foo.bar.baz', loc.character + 1 );
+	loc = locateInGenerated( 'bar.baz', loc.character + 1 );
 
 	actual = smc.originalPositionFor({
 		line: loc.line + 1,