From e12a30ae50ab940e2e7601e05b7438409765c84e Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sat, 14 Sep 2019 23:08:41 -0400 Subject: [PATCH] various --- .../compile/nodes/shared/Expression.ts | 14 +++++++--- .../compile/render_dom/wrappers/EachBlock.ts | 23 +++++----------- .../render_dom/wrappers/Element/index.ts | 3 +-- .../compile/render_dom/wrappers/IfBlock.ts | 7 ++--- .../wrappers/InlineComponent/index.ts | 18 +++++-------- .../compile/render_dom/wrappers/Slot.ts | 6 ++--- .../compile/render_dom/wrappers/Title.ts | 27 +++++++++++-------- test/test.js | 2 ++ 8 files changed, 48 insertions(+), 52 deletions(-) diff --git a/src/compiler/compile/nodes/shared/Expression.ts b/src/compiler/compile/nodes/shared/Expression.ts index 00fa38c890..39c1060728 100644 --- a/src/compiler/compile/nodes/shared/Expression.ts +++ b/src/compiler/compile/nodes/shared/Expression.ts @@ -238,9 +238,11 @@ export default class Expression { let contextual_dependencies: Set; const node = walk(this.node, { - enter(node: any, parent: any, key: string) { - // don't manipulate shorthand props twice - if (key === 'value' && parent.shorthand) return; + enter(node: any, parent: any) { + if (node.type === 'Property' && node.shorthand) { + node.value = JSON.parse(JSON.stringify(node.value)); + node.shorthand = false; + } if (map.has(node)) { scope = map.get(node); @@ -283,7 +285,7 @@ export default class Expression { } }, - leave(node: Node) { + leave(node: Node, parent: Node) { if (map.has(node)) scope = scope.parent; if (node === function_expression) { @@ -366,6 +368,10 @@ export default class Expression { function_expression = null; dependencies = null; contextual_dependencies = null; + + if (parent && parent.type === 'Property') { + parent.method = false; + } } if (node.type === 'AssignmentExpression' || node.type === 'UpdateExpression') { diff --git a/src/compiler/compile/render_dom/wrappers/EachBlock.ts b/src/compiler/compile/render_dom/wrappers/EachBlock.ts index 629e20ff8d..d73d6dcffd 100644 --- a/src/compiler/compile/render_dom/wrappers/EachBlock.ts +++ b/src/compiler/compile/render_dom/wrappers/EachBlock.ts @@ -7,6 +7,7 @@ import FragmentWrapper from './Fragment'; import { b, x } from 'code-red'; import ElseBlock from '../../nodes/ElseBlock'; import { Identifier, Node } from 'estree'; +import { changed } from './shared/changed'; export class ElseBlockWrapper extends Wrapper { node: ElseBlock; @@ -268,7 +269,7 @@ export default class EachBlockWrapper extends Wrapper { if (this.else.block.has_update_method) { block.chunks.update.push(b` if (!${this.vars.data_length} && ${each_block_else}) { - ${each_block_else}.p(changed, #ctx); + ${each_block_else}.p(#changed, #ctx); } else if (!${this.vars.data_length}) { ${each_block_else} = ${this.else.block.name}(#ctx); ${each_block_else}.c(); @@ -473,23 +474,13 @@ export default class EachBlockWrapper extends Wrapper { all_dependencies.add(dependency); }); - let condition; - if (all_dependencies.size > 0) { - // TODO make this more elegant somehow? - const array = Array.from(all_dependencies); - let condition = x`#changed.${array[0]}`; - for (let i = 1; i < array.length; i += 1) { - condition = x`${condition} || #changed.${array[i]}`; - } - } - - const has_transitions = !!(this.block.has_intro_method || this.block.has_outro_method); + if (all_dependencies.size) { + const has_transitions = !!(this.block.has_intro_method || this.block.has_outro_method); - if (condition) { const for_loop_body = this.block.has_update_method ? b` if (${iterations}[#i]) { - ${iterations}[#i].p(changed, child_ctx); + ${iterations}[#i].p(#changed, child_ctx); ${has_transitions && b`@transition_in(${this.vars.iterations}[#i], 1);`} } else { ${iterations}[#i] = ${create_each_block}(child_ctx); @@ -541,7 +532,7 @@ export default class EachBlockWrapper extends Wrapper { for (${this.block.has_update_method ? `` : `#i = ${data_length}`}; #i < ${this.block.has_update_method ? view_length : '#old_length'}; #i += 1) { ${iterations}[#i].d(1); } - ${!fixed_length && `${view_length} = ${data_length};`} + ${!fixed_length && b`${view_length} = ${data_length};`} `; } @@ -562,7 +553,7 @@ export default class EachBlockWrapper extends Wrapper { `; block.chunks.update.push(b` - if (${condition}) { + if (${changed(Array.from(all_dependencies))}) { ${update} } `); diff --git a/src/compiler/compile/render_dom/wrappers/Element/index.ts b/src/compiler/compile/render_dom/wrappers/Element/index.ts index 30e1622a88..aa9ee0c041 100644 --- a/src/compiler/compile/render_dom/wrappers/Element/index.ts +++ b/src/compiler/compile/render_dom/wrappers/Element/index.ts @@ -809,8 +809,7 @@ export default class ElementWrapper extends Wrapper { if ((dependencies && dependencies.size > 0) || this.class_dependencies.length) { const all_dependencies = this.class_dependencies.concat(...dependencies); - const deps = all_dependencies.map(dependency => `changed${quote_prop_if_necessary(dependency)}`).join(' || '); - const condition = all_dependencies.length > 1 ? `(${deps})` : deps; + const condition = changed(all_dependencies); block.chunks.update.push(b` if (${condition}) { diff --git a/src/compiler/compile/render_dom/wrappers/IfBlock.ts b/src/compiler/compile/render_dom/wrappers/IfBlock.ts index 22aa83a5d7..85623896f1 100644 --- a/src/compiler/compile/render_dom/wrappers/IfBlock.ts +++ b/src/compiler/compile/render_dom/wrappers/IfBlock.ts @@ -10,6 +10,7 @@ import { b, x } from 'code-red'; import { walk } from 'estree-walker'; import { is_head } from './shared/is_head'; import { Identifier, Node } from 'estree'; +import { changed } from './shared/changed'; function is_else_if(node: ElseBlock) { return ( @@ -260,7 +261,7 @@ export default class IfBlockWrapper extends Wrapper { ? b` ${snippet && ( dependencies.length > 0 - ? b`if ((${condition} == null) || ${dependencies.map(n => `changed.${n}`).join(' || ')}) ${condition} = !!(${snippet})` + ? b`if ((${condition} == null) || ${changed(dependencies)}) ${condition} = !!(${snippet})` : b`if (${condition} == null) ${condition} = !!(${snippet})` )} if (${condition}) return ${block.name};` @@ -360,7 +361,7 @@ export default class IfBlockWrapper extends Wrapper { function ${select_block_type}(#changed, #ctx) { ${this.branches.map(({ dependencies, condition, snippet }, i) => condition ? b` - ${snippet && b`if ((${condition} == null) || ${dependencies.map(n => `changed.${n}`).join(' || ')}) ${condition} = !!(${snippet})`} + ${snippet && b`if ((${condition} == null) || ${changed(dependencies)}) ${condition} = !!(${snippet})`} if (${condition}) return ${String(i)};` : b`return ${i};`)} ${!has_else && b`return -1;`} @@ -516,7 +517,7 @@ export default class IfBlockWrapper extends Wrapper { `; if (branch.snippet) { - block.chunks.update.push(b`if (${branch.dependencies.map(n => `changed.${n}`).join(' || ')}) ${branch.condition} = ${branch.snippet}`); + block.chunks.update.push(b`if (${changed(branch.dependencies)}) ${branch.condition} = ${branch.snippet}`); } // no `p()` here — we don't want to update outroing nodes, diff --git a/src/compiler/compile/render_dom/wrappers/InlineComponent/index.ts b/src/compiler/compile/render_dom/wrappers/InlineComponent/index.ts index 7f63716d46..188e62f787 100644 --- a/src/compiler/compile/render_dom/wrappers/InlineComponent/index.ts +++ b/src/compiler/compile/render_dom/wrappers/InlineComponent/index.ts @@ -252,7 +252,7 @@ export default class InlineComponentWrapper extends Wrapper { if (attr.expression.node.type !== 'ObjectExpression') { value_object = x`@get_spread_object(${value})`; } - changes.push(condition ? `${condition} && ${value_object}` : value_object); + changes.push(condition ? x`${condition} && ${value_object}` : value_object); } else { const obj = x`{ ${quote_name_if_necessary(name)}: ${attr.get_value(block)} }`; initial_props.push(obj); @@ -404,7 +404,7 @@ export default class InlineComponentWrapper extends Wrapper { let snippet = handler.render(block); if (handler.modifiers.has('once')) snippet = x`@once(${snippet})`; - return `${name}.$on("${handler.name}", ${snippet});`; + return b`${name}.$on("${handler.name}", ${snippet});`; }); if (this.node.name === 'svelte:component') { @@ -418,7 +418,7 @@ export default class InlineComponentWrapper extends Wrapper { function ${switch_props}(#ctx) { ${(this.node.attributes.length || this.node.bindings.length) && b` - ${props && `let ${props} = ${attribute_object};`}`} + ${props && b`let ${props} = ${attribute_object};`}`} ${statements} return ${component_opts}; } @@ -479,6 +479,8 @@ export default class InlineComponentWrapper extends Wrapper { } else { ${name} = null; } + } else if (${switch_value}) { + ${updates.length && b`${name}.$set(${name_changes});`} } `); @@ -486,14 +488,6 @@ export default class InlineComponentWrapper extends Wrapper { if (${name}) @transition_in(${name}.$$.fragment, #local); `); - if (updates.length) { - block.chunks.update.push(b` - else if (${switch_value}) { - ${name}.$set(${name_changes}); - } - `); - } - block.chunks.outro.push( b`if (${name}) @transition_out(${name}.$$.fragment, #local);` ); @@ -501,7 +495,7 @@ export default class InlineComponentWrapper extends Wrapper { block.chunks.destroy.push(b`if (${name}) @destroy_component(${name}, ${parent_node ? null : 'detaching'});`); } else { const expression = this.node.name === 'svelte:self' - ? '__svelte:self__' // TODO conflict-proof this + ? component.name : component.qualify(this.node.name); block.chunks.init.push(b` diff --git a/src/compiler/compile/render_dom/wrappers/Slot.ts b/src/compiler/compile/render_dom/wrappers/Slot.ts index c49ffa2a7d..db53685be0 100644 --- a/src/compiler/compile/render_dom/wrappers/Slot.ts +++ b/src/compiler/compile/render_dom/wrappers/Slot.ts @@ -11,6 +11,7 @@ import { stringify_props } from '../../utils/stringify_props'; import Expression from '../../nodes/shared/Expression'; import is_dynamic from './shared/is_dynamic'; import { Identifier } from 'estree'; +import { changed } from './shared/changed'; export default class SlotWrapper extends Wrapper { node: Slot; @@ -163,11 +164,8 @@ export default class SlotWrapper extends Wrapper { return is_dynamic(variable); }); - let update_conditions = dynamic_dependencies.map(name => `changed.${name}`).join(' || '); - if (dynamic_dependencies.length > 1) update_conditions = `(${update_conditions})`; - block.chunks.update.push(b` - if (${slot} && ${slot}.p && ${update_conditions}) { + if (${slot} && ${slot}.p && ${changed(dynamic_dependencies)}) { ${slot}.p( @get_slot_changes(${slot_definition}, #ctx, changed, ${get_slot_changes}), @get_slot_context(${slot_definition}, #ctx, ${get_slot_context}) diff --git a/src/compiler/compile/render_dom/wrappers/Title.ts b/src/compiler/compile/render_dom/wrappers/Title.ts index b7a0bcaa6b..d8535173fa 100644 --- a/src/compiler/compile/render_dom/wrappers/Title.ts +++ b/src/compiler/compile/render_dom/wrappers/Title.ts @@ -1,4 +1,4 @@ -import { b } from 'code-red'; +import { b, x } from 'code-red'; import Wrapper from './shared/Wrapper'; import Renderer from '../Renderer'; import Block from '../Block'; @@ -7,6 +7,7 @@ import { stringify } from '../../utils/stringify'; import add_to_set from '../../utils/add_to_set'; import Text from '../../nodes/Text'; import { Identifier } from 'estree'; +import { changed } from './shared/changed'; export default class TitleWrapper extends Wrapper { node: Title; @@ -28,7 +29,7 @@ export default class TitleWrapper extends Wrapper { if (is_dynamic) { let value; - const all_dependencies = new Set(); + const all_dependencies: Set = new Set(); // TODO some of this code is repeated in Tag.ts — would be good to // DRY it out if that's possible without introducing crazy indirection @@ -72,22 +73,26 @@ export default class TitleWrapper extends Wrapper { block.chunks.init.push( b`@_document.title = ${init};` ); + const updater = b`@_document.title = ${this.node.should_cache ? last : value};`; if (all_dependencies.size) { const dependencies = Array.from(all_dependencies); - const changed_check = ( - (block.has_outros ? `!#current || ` : '') + - dependencies.map(dependency => `changed.${dependency}`).join(' || ') - ); - const update_cached_value = `${last} !== (${last} = ${value})`; + let condition = changed(dependencies); + + if (block.has_outros) { + condition = x`!#current || ${condition}`; + } - const condition = this.node.should_cache ? - (dependencies.length ? `(${changed_check}) && ${update_cached_value}` : update_cached_value) : - changed_check; + if (this.node.should_cache) { + condition = x`${condition} && (${last} !== (${last} = ${value}))`; + } - block.chunks.update.push(b`if (${condition}) ${updater}`); + block.chunks.update.push(b` + if (${condition}) { + ${updater} + }`); } } else { const value = this.node.children.length > 0 diff --git a/test/test.js b/test/test.js index f18c01c149..9480ae7836 100644 --- a/test/test.js +++ b/test/test.js @@ -6,6 +6,8 @@ require('./setup'); require('./helpers'); require('../internal'); +console.clear(); + glob('*/index.js', { cwd: 'test' }).forEach((file) => { require('./' + file); });