From 92e4b7f813bf08efad4b04e7c8c1c4b6b9cd0a57 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 1 Apr 2017 17:10:08 -0400 Subject: [PATCH 1/2] prevent hard-to-reproduce bug with deep two-way bindings --- .../attributes/addComponentBinding.js | 4 +- .../visitors/attributes/addElementBinding.js | 2 +- .../visitors/attributes/binding/getSetter.js | 4 +- .../ComponentSelector.html | 5 ++ .../component-binding-deep-b/Editor.html | 1 + .../component-binding-deep-b/_config.js | 84 +++++++++++++++++++ .../component-binding-deep-b/main.html | 42 ++++++++++ 7 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 test/generator/samples/component-binding-deep-b/ComponentSelector.html create mode 100644 test/generator/samples/component-binding-deep-b/Editor.html create mode 100644 test/generator/samples/component-binding-deep-b/_config.js create mode 100644 test/generator/samples/component-binding-deep-b/main.html diff --git a/src/generators/dom/visitors/attributes/addComponentBinding.js b/src/generators/dom/visitors/attributes/addComponentBinding.js index 5b740aa29f..07ae7fefe8 100644 --- a/src/generators/dom/visitors/attributes/addComponentBinding.js +++ b/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; diff --git a/src/generators/dom/visitors/attributes/addElementBinding.js b/src/generators/dom/visitors/attributes/addElementBinding.js index ee222dc304..a13d254dfb 100644 --- a/src/generators/dom/visitors/attributes/addElementBinding.js +++ b/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; // + {{#each components as component}} + + {{/each}} + \ No newline at end of file diff --git a/test/generator/samples/component-binding-deep-b/Editor.html b/test/generator/samples/component-binding-deep-b/Editor.html new file mode 100644 index 0000000000..34ed70114b --- /dev/null +++ b/test/generator/samples/component-binding-deep-b/Editor.html @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/test/generator/samples/component-binding-deep-b/_config.js b/test/generator/samples/component-binding-deep-b/_config.js new file mode 100644 index 0000000000..b058bdeb43 --- /dev/null +++ b/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: ` + + + + + 
ONE SOURCE\nTWO SOURCE
+ `, + + 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, ` + + + + + 
ONE SOURCE CHANGED\nTWO SOURCE
+ ` ); + + // 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, ` + + + + + 
ONE SOURCE CHANGED\nTWO SOURCE CHANGED
+ ` ); + + component.destroy(); + } +}; diff --git a/test/generator/samples/component-binding-deep-b/main.html b/test/generator/samples/component-binding-deep-b/main.html new file mode 100644 index 0000000000..46cdd2f6d0 --- /dev/null +++ b/test/generator/samples/component-binding-deep-b/main.html @@ -0,0 +1,42 @@ + + + +
+{{compiled}}
+
+ + \ No newline at end of file From cf626ff880ae077bf84fd7cce2f6b1855fa0012c Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 1 Apr 2017 17:19:11 -0400 Subject: [PATCH 2/2] retain binding sourcemaps to the extent possible --- src/utils/flattenReference.js | 5 ++++- test/sourcemaps/samples/binding/test.js | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/utils/flattenReference.js b/src/utils/flattenReference.js index da92d5588f..c4b46e914f 100644 --- a/src/utils/flattenReference.js +++ b/src/utils/flattenReference.js @@ -1,5 +1,7 @@ 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 ); @@ -7,10 +9,11 @@ export default function flatten ( node ) { 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}✂]` }; } diff --git a/test/sourcemaps/samples/binding/test.js b/test/sourcemaps/samples/binding/test.js index a8803c0063..8737930371 100644 --- a/test/sourcemaps/samples/binding/test.js +++ b/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,