Skip to content

Commit

Permalink
Merge pull request #1243 from sveltejs/gh-1240
Browse files Browse the repository at this point in the history
fix #1240
  • Loading branch information
Rich-Harris authored Mar 18, 2018
2 parents b3fa965 + f2ab554 commit 86a151a
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 83 deletions.
36 changes: 10 additions & 26 deletions src/generators/dom/Block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ export interface BlockOptions {
changeableIndexes?: Map<string, boolean>;
indexNames?: Map<string, string>;
listNames?: Map<string, string>;
indexName?: string;
listName?: string;
dependencies?: Set<string>;
}

Expand All @@ -42,9 +40,6 @@ export default class Block {
dependencies: Set<string>;
indexNames: Map<string, string>;
listNames: Map<string, string>;
indexName: string;
listName: string;
listAlias: string;

builders: {
init: CodeBuilder;
Expand Down Expand Up @@ -92,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(),
Expand All @@ -121,8 +113,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
}

Expand Down Expand Up @@ -198,23 +188,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;
});
Expand Down Expand Up @@ -278,12 +262,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}
},
`);
Expand Down
7 changes: 4 additions & 3 deletions src/generators/nodes/Component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
24 changes: 10 additions & 14 deletions src/generators/nodes/EachBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,20 +45,16 @@ 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`
),
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)
Expand All @@ -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.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.listAlias}[#i][${i}]`);
this.contextProps.push(`${this.destructuredContexts[i]}: ${listName}[#i][${i}]`);
}
}

Expand Down Expand Up @@ -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.listNames.get(this.context);
const iterations = this.iterations;

const needsAnchor = this.next ? !this.next.isDomNode() : !parentNode || !this.parent.isDomNode();
Expand Down
21 changes: 11 additions & 10 deletions test/js/samples/deconflict-builtins/expected-bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}));
}
Expand All @@ -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
});

Expand All @@ -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;
}
},

Expand All @@ -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 {
Expand All @@ -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;
Expand Down
21 changes: 11 additions & 10 deletions test/js/samples/deconflict-builtins/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}));
}
Expand All @@ -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
});

Expand All @@ -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;
}
},

Expand All @@ -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 {
Expand All @@ -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;
Expand Down
21 changes: 11 additions & 10 deletions test/js/samples/each-block-changed-check/expected-bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}));
}
Expand All @@ -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
});

Expand All @@ -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) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 86a151a

Please sign in to comment.