From d07af2a0c576dfaec85f75b3335156bd3c0fbd7b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 14 Aug 2017 10:48:14 -0400 Subject: [PATCH] fix select edge case --- .../dom/visitors/Element/Attribute.ts | 84 +++++++++++++++---- .../helpers-invoked-if-changed/_config.js | 32 +++++-- .../helpers-invoked-if-changed/counter.js | 4 + .../helpers-invoked-if-changed/main.html | 11 ++- 4 files changed, 103 insertions(+), 28 deletions(-) create mode 100644 test/runtime/samples/helpers-invoked-if-changed/counter.js diff --git a/src/generators/dom/visitors/Element/Attribute.ts b/src/generators/dom/visitors/Element/Attribute.ts index b03c379482..80b363bd01 100644 --- a/src/generators/dom/visitors/Element/Attribute.ts +++ b/src/generators/dom/visitors/Element/Attribute.ts @@ -47,10 +47,26 @@ export default function visitAttribute( if (isDynamic) { let value; + const allDependencies = new Set(); + let shouldCache; + let hasChangeableIndex; + if (attribute.value.length === 1) { // single {{tag}} — may be a non-string - const { snippet } = block.contextualise(attribute.value[0].expression); + const { expression } = attribute.value[0]; + const { snippet, dependencies, indexes } = block.contextualise(expression); value = snippet; + dependencies.forEach(d => { + allDependencies.add(d); + }); + + hasChangeableIndex = Array.from(indexes).some(index => block.changeableIndexes.get(index)); + + shouldCache = ( + expression.type !== 'Identifier' || + block.contexts.has(expression.name) || + hasChangeableIndex + ); } else { // '{{foo}} {{bar}}' — treat as string concatenation value = @@ -60,22 +76,35 @@ export default function visitAttribute( if (chunk.type === 'Text') { return stringify(chunk.data); } else { - const { snippet } = block.contextualise(chunk.expression); + const { snippet, dependencies, indexes } = block.contextualise(chunk.expression); + + if (Array.from(indexes).some(index => block.changeableIndexes.get(index))) { + hasChangeableIndex = true; + } + + dependencies.forEach(d => { + allDependencies.add(d); + }); + return `( ${snippet} )`; } }) .join(' + '); + + shouldCache = true; } - const last = block.getUniqueName( + const isSelectValueAttribute = + name === 'value' && state.parentNodeName === 'select'; + + const last = (shouldCache || isSelectValueAttribute) && block.getUniqueName( `${state.parentNode}_${name.replace(/[^a-zA-Z_$]/g, '_')}_value` ); - block.addVariable(last); - const isSelectValueAttribute = - name === 'value' && state.parentNodeName === 'select'; + if (shouldCache || isSelectValueAttribute) block.addVariable(last); let updater; + const init = shouldCache ? `${last} = ${value}` : value; if (isSelectValueAttribute) { // annoying special case @@ -104,27 +133,48 @@ export default function visitAttribute( } `; - block.builders.hydrate.addLine(deindent` - ${last} = ${value} + block.builders.hydrate.addBlock(deindent` + ${last} = ${value}; ${updater} `); + + block.builders.update.addLine(`${last} = ${value};`); } else if (propertyName) { block.builders.hydrate.addLine( - `${state.parentNode}.${propertyName} = ${last} = ${value};` + `${state.parentNode}.${propertyName} = ${init};` ); - updater = `${state.parentNode}.${propertyName} = ${last};`; + updater = `${state.parentNode}.${propertyName} = ${shouldCache || isSelectValueAttribute ? last : value};`; } else { block.builders.hydrate.addLine( - `${method}( ${state.parentNode}, '${name}', ${last} = ${value} );` + `${method}( ${state.parentNode}, '${name}', ${init} );` ); - updater = `${method}( ${state.parentNode}, '${name}', ${last} );`; + updater = `${method}( ${state.parentNode}, '${name}', ${shouldCache || isSelectValueAttribute ? last : value} );`; } - block.builders.update.addBlock(deindent` - if ( ${last} !== ( ${last} = ${value} ) ) { - ${updater} - } - `); + if (allDependencies.size || hasChangeableIndex || isSelectValueAttribute) { + const dependencies = Array.from(allDependencies); + const changedCheck = ( + ( block.hasOutroMethod ? `#outroing || ` : '' ) + + dependencies.map(dependency => `'${dependency}' in changed`).join(' || ') + ); + + const updateCachedValue = `${last} !== ( ${last} = ${value} )`; + + const condition = shouldCache ? + ( dependencies.length ? `( ${changedCheck} ) && ${updateCachedValue}` : updateCachedValue ) : + changedCheck; + + // block.builders.update.addConditionalLine( + // condition, + // updater + // ); + + block.builders.update.addBlock(deindent` + if ( ${condition} ) { + ${updater} + } + `); + } } else { const value = attribute.value === true ? 'true' diff --git a/test/runtime/samples/helpers-invoked-if-changed/_config.js b/test/runtime/samples/helpers-invoked-if-changed/_config.js index bdc2f19fdb..fd8145ae74 100644 --- a/test/runtime/samples/helpers-invoked-if-changed/_config.js +++ b/test/runtime/samples/helpers-invoked-if-changed/_config.js @@ -1,24 +1,38 @@ +import counter from './counter.js'; + export default { data: { x: 1, - y: 2 + y: 2, + z: 3 }, html: `

1

-

2

+

3

`, test(assert, component) { - global.count = 0; + counter.y = counter.z = 0; + + component.set({ x: 4 }); + assert.equal(counter.y, 0); + assert.equal(counter.z, 0); + + component.set({ x: 5, y: 6 }); + assert.equal(counter.y, 1); + assert.equal(counter.z, 0); - component.set({ x: 3 }); - assert.equal(global.count, 0); + component.set({ x: 6, y: 6 }); + assert.equal(counter.y, 1); + assert.equal(counter.z, 0); - component.set({ x: 4, y: 5 }); - assert.equal(global.count, 1); + component.set({ z: 7 }); + assert.equal(counter.y, 1); + assert.equal(counter.z, 1); - component.set({ x: 5, y: 5 }); - assert.equal(global.count, 1); + component.set({ x: 8, z: 7 }); + assert.equal(counter.y, 1); + assert.equal(counter.z, 1); } }; diff --git a/test/runtime/samples/helpers-invoked-if-changed/counter.js b/test/runtime/samples/helpers-invoked-if-changed/counter.js new file mode 100644 index 0000000000..5367ba3c2b --- /dev/null +++ b/test/runtime/samples/helpers-invoked-if-changed/counter.js @@ -0,0 +1,4 @@ +export default { + y: 0, + z: 0 +}; \ No newline at end of file diff --git a/test/runtime/samples/helpers-invoked-if-changed/main.html b/test/runtime/samples/helpers-invoked-if-changed/main.html index bda367155f..1bcd70c4c1 100644 --- a/test/runtime/samples/helpers-invoked-if-changed/main.html +++ b/test/runtime/samples/helpers-invoked-if-changed/main.html @@ -1,11 +1,18 @@

{{x}}

-

{{myHelper(y)}}

+

{{myHelper(z)}}