From 531354fc39512722b5a926ccebaa1ef0e656a7e1 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 14 Aug 2017 08:13:25 -0400 Subject: [PATCH] only cache values when it makes sense --- src/generators/Generator.ts | 11 +++++++---- src/generators/dom/Block.ts | 3 +++ src/generators/dom/preprocess.ts | 12 +++++++++++- .../dom/visitors/Element/meta/Window.ts | 4 ++-- src/generators/dom/visitors/MustacheTag.ts | 16 ++++++++++++---- src/utils/CodeBuilder.ts | 4 ++-- test/runtime/samples/each-block-keyed/_config.js | 7 +++++-- test/runtime/samples/each-block-keyed/main.html | 4 ++-- 8 files changed, 44 insertions(+), 17 deletions(-) diff --git a/src/generators/Generator.ts b/src/generators/Generator.ts index 002e52bbf9..7798cf3c86 100644 --- a/src/generators/Generator.ts +++ b/src/generators/Generator.ts @@ -125,7 +125,8 @@ export default class Generator { ) { this.addSourcemapLocations(expression); - const usedContexts: string[] = []; + const usedContexts = new Set(); + const usedIndexes = new Set(); const { code, helpers } = this; const { contexts, indexes } = block; @@ -168,12 +169,13 @@ export default class Generator { ); } - if (!~usedContexts.indexOf(name)) usedContexts.push(name); + usedContexts.add(name); } else if (helpers.has(name)) { code.prependRight(node.start, `${self.alias('template')}.helpers.`); } else if (indexes.has(name)) { const context = indexes.get(name); - if (!~usedContexts.indexOf(context)) usedContexts.push(context); + usedContexts.add(context); // TODO is this right? + usedIndexes.add(name); } else { // handle shorthand properties if (parent && parent.type === 'Property' && parent.shorthand) { @@ -193,7 +195,7 @@ export default class Generator { code.prependRight(node.start, `state.`); } - if (!~usedContexts.indexOf('state')) usedContexts.push('state'); + usedContexts.add('state'); } this.skip(); @@ -221,6 +223,7 @@ export default class Generator { return { dependencies: Array.from(dependencies), contexts: usedContexts, + indexes: usedIndexes, snippet: `[✂${expression.start}-${expression.end}✂]`, }; } diff --git a/src/generators/dom/Block.ts b/src/generators/dom/Block.ts index 069fb7f2b8..bf38aebc74 100644 --- a/src/generators/dom/Block.ts +++ b/src/generators/dom/Block.ts @@ -12,6 +12,7 @@ export interface BlockOptions { key?: string; contexts?: Map; indexes?: Map; + changeableIndexes?: Map; contextDependencies?: Map; params?: string[]; indexNames?: Map; @@ -32,6 +33,7 @@ export default class Block { contexts: Map; indexes: Map; + changeableIndexes: Map; contextDependencies: Map; dependencies: Set; params: string[]; @@ -77,6 +79,7 @@ export default class Block { this.contexts = options.contexts; this.indexes = options.indexes; + this.changeableIndexes = options.changeableIndexes; this.contextDependencies = options.contextDependencies; this.dependencies = new Set(); diff --git a/src/generators/dom/preprocess.ts b/src/generators/dom/preprocess.ts index af460756da..9b784bc558 100644 --- a/src/generators/dom/preprocess.ts +++ b/src/generators/dom/preprocess.ts @@ -189,7 +189,10 @@ const preprocessors = { contexts.set(node.context, context); const indexes = new Map(block.indexes); - if (node.index) indexes.set(indexName, node.context); + if (node.index) indexes.set(node.index, node.context); + + const changeableIndexes = new Map(block.changeableIndexes); + if (node.index) changeableIndexes.set(node.index, node.key); const contextDependencies = new Map(block.contextDependencies); contextDependencies.set(node.context, dependencies); @@ -203,6 +206,7 @@ const preprocessors = { contextDependencies, contexts, indexes, + changeableIndexes, listName, indexName, @@ -274,6 +278,11 @@ const preprocessors = { } } }); + } else if (attribute.type === 'EventHandler' && attribute.expression) { + attribute.expression.arguments.forEach((arg: Node) => { + const dependencies = block.findDependencies(arg); + block.addDependencies(dependencies); + }); } else if (attribute.type === 'Binding') { const dependencies = block.findDependencies(attribute.value); block.addDependencies(dependencies); @@ -444,6 +453,7 @@ export default function preprocess( contexts: new Map(), indexes: new Map(), + changeableIndexes: new Map(), contextDependencies: new Map(), params: ['state'], diff --git a/src/generators/dom/visitors/Element/meta/Window.ts b/src/generators/dom/visitors/Element/meta/Window.ts index d6c848c342..8e15ee2512 100644 --- a/src/generators/dom/visitors/Element/meta/Window.ts +++ b/src/generators/dom/visitors/Element/meta/Window.ts @@ -38,8 +38,8 @@ export default function visitWindow( let usesState = false; attribute.expression.arguments.forEach((arg: Node) => { - const { contexts } = block.contextualise(arg, null, true); - if (contexts.length) usesState = true; + const { dependencies } = block.contextualise(arg, null, true); + if (dependencies.length) usesState = true; }); const flattened = flattenReference(attribute.expression.callee); diff --git a/src/generators/dom/visitors/MustacheTag.ts b/src/generators/dom/visitors/MustacheTag.ts index 77c84d3330..91575e58d0 100644 --- a/src/generators/dom/visitors/MustacheTag.ts +++ b/src/generators/dom/visitors/MustacheTag.ts @@ -10,9 +10,15 @@ export default function visitMustacheTag( state: State, node: Node ) { - const { dependencies, snippet } = block.contextualise(node.expression); + const { dependencies, indexes, snippet } = block.contextualise(node.expression); - const shouldCache = node.expression.type !== 'Identifier' || block.contexts.has(node.expression.name); + const hasChangeableIndex = Array.from(indexes).some(index => block.changeableIndexes.get(index)); + + const shouldCache = ( + node.expression.type !== 'Identifier' || + block.contexts.has(node.expression.name) || + hasChangeableIndex + ); const name = node._state.name; const value = shouldCache && block.getUniqueName(`${name}_value`); @@ -30,14 +36,16 @@ export default function visitMustacheTag( true ); - if (dependencies.length) { + if (dependencies.length || hasChangeableIndex) { const changedCheck = ( ( block.hasOutroMethod ? `#outroing || ` : '' ) + dependencies.map(dependency => `'${dependency}' in changed`).join(' || ') ); + const updateCachedValue = `${value} !== ( ${value} = ${snippet} )`; + const condition = shouldCache ? - `( ${changedCheck} ) && ${value} !== ( ${value} = ${snippet} )` : + ( dependencies.length ? `( ${changedCheck} ) && ${updateCachedValue}` : updateCachedValue ) : changedCheck; block.builders.update.addConditionalLine( diff --git a/src/utils/CodeBuilder.ts b/src/utils/CodeBuilder.ts index 92c88ea483..934f29b9a4 100644 --- a/src/utils/CodeBuilder.ts +++ b/src/utils/CodeBuilder.ts @@ -26,10 +26,10 @@ export default class CodeBuilder { this.result += `\n\t${line}`; } else { if (this.lastCondition) { - this.result += `\n}\n\n`; + this.result += `\n}`; } - this.result += `if ( ${condition} ) {\n\t${line}`; + this.result += `${this.last === ChunkType.Block ? '\n\n' : '\n'}if ( ${condition} ) {\n\t${line}`; this.lastCondition = condition; } diff --git a/test/runtime/samples/each-block-keyed/_config.js b/test/runtime/samples/each-block-keyed/_config.js index 35b47f5be8..29cf422fbf 100644 --- a/test/runtime/samples/each-block-keyed/_config.js +++ b/test/runtime/samples/each-block-keyed/_config.js @@ -6,7 +6,10 @@ export default { ] }, - html: '

implement keyed each blocks

implement client-side hydration

', + html: ` +

1: implement keyed each blocks

+

2: implement client-side hydration

+ `, test ( assert, component, target ) { const [ p1, p2 ] = target.querySelectorAll( 'p' ); @@ -16,7 +19,7 @@ export default { { id: 234, description: 'implement client-side hydration' } ] }); - assert.htmlEqual( target.innerHTML, '

implement client-side hydration

' ); + assert.htmlEqual( target.innerHTML, '

1: implement client-side hydration

' ); const [ p3 ] = target.querySelectorAll( 'p' ); diff --git a/test/runtime/samples/each-block-keyed/main.html b/test/runtime/samples/each-block-keyed/main.html index 7d5b90a9f8..da74eaa701 100644 --- a/test/runtime/samples/each-block-keyed/main.html +++ b/test/runtime/samples/each-block-keyed/main.html @@ -1,3 +1,3 @@ -{{#each todos as todo @id}} -

{{todo.description}}

+{{#each todos as todo, i @id}} +

{{i+1}}: {{todo.description}}

{{/each}}