From 6f65554c93424d19f25b8393729446bf424548a0 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 Mar 2018 14:04:26 -0400 Subject: [PATCH 1/4] failing test for #1240 --- .../_config.js | 26 +++++++++++++++++++ .../main.html | 3 +++ 2 files changed, 29 insertions(+) create mode 100644 test/runtime/samples/each-block-deconflict-name-context/_config.js create mode 100644 test/runtime/samples/each-block-deconflict-name-context/main.html diff --git a/test/runtime/samples/each-block-deconflict-name-context/_config.js b/test/runtime/samples/each-block-deconflict-name-context/_config.js new file mode 100644 index 000000000000..c4b9deb32e5e --- /dev/null +++ b/test/runtime/samples/each-block-deconflict-name-context/_config.js @@ -0,0 +1,26 @@ +export default { + data: { + foo: { + bar: ['x', 'y', 'z'] + } + }, + + html: ` + + + + `, + + test(assert, component, target, window) { + const inputs = target.querySelectorAll('input'); + + inputs[1].value = 'w'; + inputs[1].dispatchEvent(new window.MouseEvent('input')); + + assert.deepEqual(component.get(), { + foo: { + bar: ['x', 'w', 'z'] + } + }); + } +}; diff --git a/test/runtime/samples/each-block-deconflict-name-context/main.html b/test/runtime/samples/each-block-deconflict-name-context/main.html new file mode 100644 index 000000000000..08730e1e0192 --- /dev/null +++ b/test/runtime/samples/each-block-deconflict-name-context/main.html @@ -0,0 +1,3 @@ +{{#each foo.bar as bar}} + +{{/each}} From 12c7d1c86d2ba813b5fcfac3a1480473af198109 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 Mar 2018 14:04:52 -0400 Subject: [PATCH 2/4] flawed solution to #1240 --- src/generators/dom/Block.ts | 3 --- src/generators/nodes/EachBlock.ts | 14 +++++--------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/generators/dom/Block.ts b/src/generators/dom/Block.ts index d101287532c4..48e205f64c05 100644 --- a/src/generators/dom/Block.ts +++ b/src/generators/dom/Block.ts @@ -44,7 +44,6 @@ export default class Block { listNames: Map; indexName: string; listName: string; - listAlias: string; builders: { init: CodeBuilder; @@ -121,8 +120,6 @@ export default class Block { .set('state', this.getUniqueName('state')); if (this.key) this.aliases.set('key', this.getUniqueName('key')); - this.listAlias = this.getUniqueName(this.listName); - this.hasUpdateMethod = false; // determined later } diff --git a/src/generators/nodes/EachBlock.ts b/src/generators/nodes/EachBlock.ts index 1864f9695831..b2ac0b403fba 100644 --- a/src/generators/nodes/EachBlock.ts +++ b/src/generators/nodes/EachBlock.ts @@ -45,11 +45,7 @@ export default class EachBlock extends Node { indexes: new Map(block.indexes), changeableIndexes: new Map(block.changeableIndexes), - listName: ( - (this.expression.type === 'MemberExpression' && !this.expression.computed) ? this.expression.property.name : - this.expression.type === 'Identifier' ? this.expression.name : - `each_value` - ), + listName: this.generator.getUniqueName('each_value'), indexName: this.index || `${this.context}_index`, indexNames: new Map(block.indexNames), @@ -75,14 +71,14 @@ export default class EachBlock extends Node { } this.contextProps = [ - `${this.block.listName}: ${this.block.listAlias}`, - `${this.context}: ${this.block.listAlias}[#i]`, + `${this.block.listName}: ${this.block.listName}`, + `${this.context}: ${this.block.listName}[#i]`, `${this.block.indexName}: #i` ]; if (this.destructuredContexts) { for (let i = 0; i < this.destructuredContexts.length; i += 1) { - this.contextProps.push(`${this.destructuredContexts[i]}: ${this.block.listAlias}[#i][${i}]`); + this.contextProps.push(`${this.destructuredContexts[i]}: ${this.block.listName}[#i][${i}]`); } } @@ -117,7 +113,7 @@ export default class EachBlock extends Node { const each = this.var; const create_each_block = this.block.name; - const each_block_value = this.block.listAlias; + const each_block_value = this.block.listName; const iterations = this.iterations; const needsAnchor = this.next ? !this.next.isDomNode() : !parentNode || !this.parent.isDomNode(); From ce607d5ab3bac29f693966a8a4bbce2c39972695 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 Mar 2018 22:13:34 -0400 Subject: [PATCH 3/4] add local variable for list values - fixes #1240 --- src/generators/dom/Block.ts | 26 +++++++------------ src/generators/nodes/Component.ts | 7 ++--- .../deconflict-builtins/expected-bundle.js | 21 ++++++++------- .../samples/deconflict-builtins/expected.js | 21 ++++++++------- .../expected-bundle.js | 21 ++++++++------- .../each-block-changed-check/expected.js | 21 ++++++++------- 6 files changed, 58 insertions(+), 59 deletions(-) diff --git a/src/generators/dom/Block.ts b/src/generators/dom/Block.ts index 48e205f64c05..9297a357dbe3 100644 --- a/src/generators/dom/Block.ts +++ b/src/generators/dom/Block.ts @@ -195,23 +195,17 @@ export default class Block { // TODO `this.contexts` is possibly redundant post-#1122 const initializers = []; - const updaters = []; - this.contexts.forEach((alias, name) => { - // TODO only the ones that are actually used in this block... - const assignment = `${alias} = state.${name}`; - - initializers.push(assignment); - updaters.push(`${assignment};`); - - this.hasUpdateMethod = true; - }); - this.indexNames.forEach((alias, name) => { + this.contexts.forEach((name, context) => { // TODO only the ones that are actually used in this block... - const assignment = `${alias} = state.${alias}`; // TODO this is wrong!!! + const listName = this.listNames.get(context); + const indexName = this.indexNames.get(context); - initializers.push(assignment); - updaters.push(`${assignment};`); + initializers.push( + `${name} = state.${context}`, + `${listName} = state.${listName}`, + `${indexName} = state.${indexName}` + ); this.hasUpdateMethod = true; }); @@ -275,12 +269,12 @@ export default class Block { } if (this.hasUpdateMethod) { - if (this.builders.update.isEmpty() && updaters.length === 0) { + if (this.builders.update.isEmpty() && initializers.length === 0) { properties.addBlock(`p: @noop,`); } else { properties.addBlock(deindent` p: function update(changed, state) { - ${updaters} + ${initializers.map(str => `${str};`)} ${this.builders.update} }, `); diff --git a/src/generators/nodes/Component.ts b/src/generators/nodes/Component.ts index 566b1c83a1c3..e0a78d886c6b 100644 --- a/src/generators/nodes/Component.ts +++ b/src/generators/nodes/Component.ts @@ -161,10 +161,11 @@ export default class Component extends Node { const computed = isComputed(binding.value); const tail = binding.value.type === 'MemberExpression' ? getTailSnippet(binding.value) : ''; + const list = block.listNames.get(key); + const index = block.indexNames.get(key); + setFromChild = deindent` - var list = state.${block.listNames.get(key)}; - var index = ${block.indexNames.get(key)}; - list[index]${tail} = childState.${binding.name}; + ${list}[${index}]${tail} = childState.${binding.name}; ${binding.dependencies .map((name: string) => { diff --git a/test/js/samples/deconflict-builtins/expected-bundle.js b/test/js/samples/deconflict-builtins/expected-bundle.js index 2ecc8aa9cf85..0b3656e86bcc 100644 --- a/test/js/samples/deconflict-builtins/expected-bundle.js +++ b/test/js/samples/deconflict-builtins/expected-bundle.js @@ -204,14 +204,14 @@ var proto = { function create_main_fragment(component, state) { var each_anchor; - var createElement_1 = state.createElement; + var each_value = state.createElement; var each_blocks = []; - for (var i = 0; i < createElement_1.length; i += 1) { + for (var i = 0; i < each_value.length; i += 1) { each_blocks[i] = create_each_block(component, assign({}, state, { - createElement: createElement_1, - node: createElement_1[i], + each_value: each_value, + node: each_value[i], node_index: i })); } @@ -234,13 +234,13 @@ function create_main_fragment(component, state) { }, p: function update(changed, state) { - var createElement_1 = state.createElement; + var each_value = state.createElement; if (changed.createElement) { - for (var i = 0; i < createElement_1.length; i += 1) { + for (var i = 0; i < each_value.length; i += 1) { var each_context = assign({}, state, { - createElement: createElement_1, - node: createElement_1[i], + each_value: each_value, + node: each_value[i], node_index: i }); @@ -257,7 +257,7 @@ function create_main_fragment(component, state) { each_blocks[i].u(); each_blocks[i].d(); } - each_blocks.length = createElement_1.length; + each_blocks.length = each_value.length; } }, @@ -277,7 +277,7 @@ function create_main_fragment(component, state) { // (1:0) {{#each createElement as node}} function create_each_block(component, state) { - var node = state.node, node_index = state.node_index; + var node = state.node, each_value = state.each_value, node_index = state.node_index; var span, text_value = node, text; return { @@ -293,6 +293,7 @@ function create_each_block(component, state) { p: function update(changed, state) { node = state.node; + each_value = state.each_value; node_index = state.node_index; if ((changed.createElement) && text_value !== (text_value = node)) { text.data = text_value; diff --git a/test/js/samples/deconflict-builtins/expected.js b/test/js/samples/deconflict-builtins/expected.js index 82bba044d7a8..3ec9c5c9afa9 100644 --- a/test/js/samples/deconflict-builtins/expected.js +++ b/test/js/samples/deconflict-builtins/expected.js @@ -4,14 +4,14 @@ import { appendNode, assign, createComment, createElement, createText, destroyEa function create_main_fragment(component, state) { var each_anchor; - var createElement_1 = state.createElement; + var each_value = state.createElement; var each_blocks = []; - for (var i = 0; i < createElement_1.length; i += 1) { + for (var i = 0; i < each_value.length; i += 1) { each_blocks[i] = create_each_block(component, assign({}, state, { - createElement: createElement_1, - node: createElement_1[i], + each_value: each_value, + node: each_value[i], node_index: i })); } @@ -34,13 +34,13 @@ function create_main_fragment(component, state) { }, p: function update(changed, state) { - var createElement_1 = state.createElement; + var each_value = state.createElement; if (changed.createElement) { - for (var i = 0; i < createElement_1.length; i += 1) { + for (var i = 0; i < each_value.length; i += 1) { var each_context = assign({}, state, { - createElement: createElement_1, - node: createElement_1[i], + each_value: each_value, + node: each_value[i], node_index: i }); @@ -57,7 +57,7 @@ function create_main_fragment(component, state) { each_blocks[i].u(); each_blocks[i].d(); } - each_blocks.length = createElement_1.length; + each_blocks.length = each_value.length; } }, @@ -77,7 +77,7 @@ function create_main_fragment(component, state) { // (1:0) {{#each createElement as node}} function create_each_block(component, state) { - var node = state.node, node_index = state.node_index; + var node = state.node, each_value = state.each_value, node_index = state.node_index; var span, text_value = node, text; return { @@ -93,6 +93,7 @@ function create_each_block(component, state) { p: function update(changed, state) { node = state.node; + each_value = state.each_value; node_index = state.node_index; if ((changed.createElement) && text_value !== (text_value = node)) { text.data = text_value; diff --git a/test/js/samples/each-block-changed-check/expected-bundle.js b/test/js/samples/each-block-changed-check/expected-bundle.js index 97ad53ad1444..92e32c85d2b4 100644 --- a/test/js/samples/each-block-changed-check/expected-bundle.js +++ b/test/js/samples/each-block-changed-check/expected-bundle.js @@ -206,14 +206,14 @@ var proto = { function create_main_fragment(component, state) { var text, p, text_1; - var comments = state.comments; + var each_value = state.comments; var each_blocks = []; - for (var i = 0; i < comments.length; i += 1) { + for (var i = 0; i < each_value.length; i += 1) { each_blocks[i] = create_each_block(component, assign({}, state, { - comments: comments, - comment: comments[i], + each_value: each_value, + comment: each_value[i], i: i })); } @@ -240,13 +240,13 @@ function create_main_fragment(component, state) { }, p: function update(changed, state) { - var comments = state.comments; + var each_value = state.comments; if (changed.comments || changed.elapsed || changed.time) { - for (var i = 0; i < comments.length; i += 1) { + for (var i = 0; i < each_value.length; i += 1) { var each_context = assign({}, state, { - comments: comments, - comment: comments[i], + each_value: each_value, + comment: each_value[i], i: i }); @@ -263,7 +263,7 @@ function create_main_fragment(component, state) { each_blocks[i].u(); each_blocks[i].d(); } - each_blocks.length = comments.length; + each_blocks.length = each_value.length; } if (changed.foo) { @@ -288,7 +288,7 @@ function create_main_fragment(component, state) { // (1:0) {{#each comments as comment, i}} function create_each_block(component, state) { - var comment = state.comment, i = state.i; + var comment = state.comment, each_value = state.each_value, i = state.i; var div, strong, text, text_1, span, text_2_value = comment.author, text_2, text_3, text_4_value = state.elapsed(comment.time, state.time), text_4, text_5, text_6, raw_value = comment.html, raw_before; return { @@ -329,6 +329,7 @@ function create_each_block(component, state) { p: function update(changed, state) { comment = state.comment; + each_value = state.each_value; i = state.i; if ((changed.comments) && text_2_value !== (text_2_value = comment.author)) { text_2.data = text_2_value; diff --git a/test/js/samples/each-block-changed-check/expected.js b/test/js/samples/each-block-changed-check/expected.js index 15a29e273918..a73d47593b9b 100644 --- a/test/js/samples/each-block-changed-check/expected.js +++ b/test/js/samples/each-block-changed-check/expected.js @@ -4,14 +4,14 @@ import { appendNode, assign, createElement, createText, destroyEach, detachAfter function create_main_fragment(component, state) { var text, p, text_1; - var comments = state.comments; + var each_value = state.comments; var each_blocks = []; - for (var i = 0; i < comments.length; i += 1) { + for (var i = 0; i < each_value.length; i += 1) { each_blocks[i] = create_each_block(component, assign({}, state, { - comments: comments, - comment: comments[i], + each_value: each_value, + comment: each_value[i], i: i })); } @@ -38,13 +38,13 @@ function create_main_fragment(component, state) { }, p: function update(changed, state) { - var comments = state.comments; + var each_value = state.comments; if (changed.comments || changed.elapsed || changed.time) { - for (var i = 0; i < comments.length; i += 1) { + for (var i = 0; i < each_value.length; i += 1) { var each_context = assign({}, state, { - comments: comments, - comment: comments[i], + each_value: each_value, + comment: each_value[i], i: i }); @@ -61,7 +61,7 @@ function create_main_fragment(component, state) { each_blocks[i].u(); each_blocks[i].d(); } - each_blocks.length = comments.length; + each_blocks.length = each_value.length; } if (changed.foo) { @@ -86,7 +86,7 @@ function create_main_fragment(component, state) { // (1:0) {{#each comments as comment, i}} function create_each_block(component, state) { - var comment = state.comment, i = state.i; + var comment = state.comment, each_value = state.each_value, i = state.i; var div, strong, text, text_1, span, text_2_value = comment.author, text_2, text_3, text_4_value = state.elapsed(comment.time, state.time), text_4, text_5, text_6, raw_value = comment.html, raw_before; return { @@ -127,6 +127,7 @@ function create_each_block(component, state) { p: function update(changed, state) { comment = state.comment; + each_value = state.each_value; i = state.i; if ((changed.comments) && text_2_value !== (text_2_value = comment.author)) { text_2.data = text_2_value; From f2ab5545dc6e8a5e025e34a16bf343bfad8a8323 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 15 Mar 2018 22:33:10 -0400 Subject: [PATCH 4/4] tidy up --- src/generators/dom/Block.ts | 7 ------- src/generators/nodes/EachBlock.ts | 20 ++++++++++---------- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/generators/dom/Block.ts b/src/generators/dom/Block.ts index 9297a357dbe3..2e03c45e758f 100644 --- a/src/generators/dom/Block.ts +++ b/src/generators/dom/Block.ts @@ -19,8 +19,6 @@ export interface BlockOptions { changeableIndexes?: Map; indexNames?: Map; listNames?: Map; - indexName?: string; - listName?: string; dependencies?: Set; } @@ -42,8 +40,6 @@ export default class Block { dependencies: Set; indexNames: Map; listNames: Map; - indexName: string; - listName: string; builders: { init: CodeBuilder; @@ -91,9 +87,6 @@ export default class Block { this.indexNames = options.indexNames; this.listNames = options.listNames; - this.indexName = options.indexName; - this.listName = options.listName; - this.builders = { init: new CodeBuilder(), create: new CodeBuilder(), diff --git a/src/generators/nodes/EachBlock.ts b/src/generators/nodes/EachBlock.ts index b2ac0b403fba..297f78617234 100644 --- a/src/generators/nodes/EachBlock.ts +++ b/src/generators/nodes/EachBlock.ts @@ -45,16 +45,16 @@ export default class EachBlock extends Node { indexes: new Map(block.indexes), changeableIndexes: new Map(block.changeableIndexes), - listName: this.generator.getUniqueName('each_value'), - indexName: this.index || `${this.context}_index`, - indexNames: new Map(block.indexNames), listNames: new Map(block.listNames) }); + const listName = this.generator.getUniqueName('each_value'); + const indexName = this.index || this.generator.getUniqueName(`${this.context}_index`); + this.block.contextTypes.set(this.context, 'each'); - this.block.indexNames.set(this.context, this.block.indexName); - this.block.listNames.set(this.context, this.block.listName); + this.block.indexNames.set(this.context, indexName); + this.block.listNames.set(this.context, listName); if (this.index) { this.block.indexes.set(this.index, this.context); this.block.changeableIndexes.set(this.index, this.key) @@ -71,14 +71,14 @@ export default class EachBlock extends Node { } this.contextProps = [ - `${this.block.listName}: ${this.block.listName}`, - `${this.context}: ${this.block.listName}[#i]`, - `${this.block.indexName}: #i` + `${listName}: ${listName}`, + `${this.context}: ${listName}[#i]`, + `${indexName}: #i` ]; if (this.destructuredContexts) { for (let i = 0; i < this.destructuredContexts.length; i += 1) { - this.contextProps.push(`${this.destructuredContexts[i]}: ${this.block.listName}[#i][${i}]`); + this.contextProps.push(`${this.destructuredContexts[i]}: ${listName}[#i][${i}]`); } } @@ -113,7 +113,7 @@ export default class EachBlock extends Node { const each = this.var; const create_each_block = this.block.name; - const each_block_value = this.block.listName; + const each_block_value = this.block.listNames.get(this.context); const iterations = this.iterations; const needsAnchor = this.next ? !this.next.isDomNode() : !parentNode || !this.parent.isDomNode();