diff --git a/src/compile/Component.ts b/src/compile/Component.ts index e0db009036..8261283a87 100644 --- a/src/compile/Component.ts +++ b/src/compile/Component.ts @@ -77,7 +77,6 @@ export default class Component { reactive_declarations: Array<{ assignees: Set, dependencies: Set, snippet: string }> = []; reactive_declaration_nodes: Set = new Set(); has_reactive_assignments = false; - mutable_props: Set = new Set(); indirectDependencies: Map> = new Map(); template_references: Set = new Set(); @@ -152,7 +151,6 @@ export default class Component { if (!ast.instance && !ast.module) { const props = [...this.template_references]; this.declarations.push(...props); - addToSet(this.mutable_props, this.template_references); addToSet(this.writable_declarations, this.template_references); props.forEach(name => { @@ -163,7 +161,7 @@ export default class Component { imported_as: null, exported_as: name, source: null, - mutated: false, + mutated: true, // TODO kind of a misnomer... it's *mutable* but not necessarily *mutated*. is that a problem? referenced: true, module: false }); @@ -174,7 +172,9 @@ export default class Component { } // tell the root fragment scope about all of the mutable names we know from the script - this.mutable_props.forEach(name => this.fragment.scope.mutables.add(name)); + this.vars.forEach(variable => { + if (variable.mutated) this.fragment.scope.mutables.add(name); + }); } add_var(variable: Var) { @@ -187,7 +187,7 @@ export default class Component { if (variable) { variable.referenced = true; - } else if (!this.ast.instance) { + } else if (!this.ast.instance || name[0] === '$') { this.add_var({ name, kind: 'injected', @@ -485,11 +485,9 @@ export default class Component { exported_as: name, source: null, module: is_module, - mutated: false, + mutated: !is_module, referenced: false }); - - if (!is_module) this.mutable_props.add(name); }); }); } else { @@ -512,7 +510,7 @@ export default class Component { exported_as: name, source: null, module: is_module, - mutated: false, + mutated: !is_module, referenced: false }); } @@ -587,6 +585,33 @@ export default class Component { message: `The $ prefix is reserved, and cannot be used for variable and import names` }); } + + if (!/Import/.test(node.type)) { + const kind = node.type === 'VariableDeclaration' + ? node.kind + : node.type === 'ClassDeclaration' + ? 'class' + : node.type === 'FunctionDeclaration' + ? 'function' + : null; + + // sanity check + if (!kind) throw new Error(`Unknown declaration type ${node.type}`); + + this.add_var({ + name, + kind, + import_type: null, + imported_as: null, + exported_as: null, + source: null, + module: false, + mutated: false, + referenced: false + }); + + this.declarations.push(name); + } }); this.extract_imports(script.content, true); @@ -666,9 +691,9 @@ export default class Component { }); }); - this.track_mutations(); this.extract_imports(script.content, false); this.extract_exports(script.content, false); + this.track_mutations(); // TODO remove this, just use component.symbols everywhere this.props = this.vars.filter(variable => !variable.module && variable.exported_as).map(variable => ({ @@ -690,7 +715,9 @@ export default class Component { // TODO merge this with other walks that are independent track_mutations() { const component = this; - let { instance_scope: scope, instance_scope_map: map } = this; + const { instance_scope, instance_scope_map: map } = this; + + let scope = instance_scope; walk(this.ast.instance.content, { enter(node, parent) { @@ -701,17 +728,26 @@ export default class Component { if (node.type === 'AssignmentExpression') { names = node.left.type === 'MemberExpression' - ? [getObject(node.left).name] - : extractNames(node.left); + ? [getObject(node.left).name] + : extractNames(node.left); } else if (node.type === 'UpdateExpression') { names = [getObject(node.argument).name]; } if (names) { names.forEach(name => { - if (scope.has(name)) component.mutable_props.add(name); + if (scope.findOwner(name) === instance_scope) { + const variable = component.var_lookup.get(name); + variable.mutated = true; + } }); } + }, + + leave(node) { + if (map.has(node)) { + scope = scope.parent; + } } }) } @@ -758,7 +794,6 @@ export default class Component { const exported = new Set(); this.props.forEach(prop => { exported.add(prop.name); - this.mutable_props.add(prop.name); }); const coalesced_declarations = []; @@ -898,7 +933,7 @@ export default class Component { this.ast.instance.content.body.forEach(node => { if (node.type === 'VariableDeclaration') { - if (node.declarations.every(d => d.init && d.init.type === 'Literal' && !this.mutable_props.has(d.id.name) && !template_scope.containsMutable([d.id.name]))) { + if (node.declarations.every(d => d.init && d.init.type === 'Literal' && !this.var_lookup.get(d.id.name).mutated && !template_scope.containsMutable([d.id.name]))) { node.declarations.forEach(d => { hoistable_names.add(d.id.name); }); diff --git a/src/compile/nodes/Attribute.ts b/src/compile/nodes/Attribute.ts index 88aaadff26..3e1a08855c 100644 --- a/src/compile/nodes/Attribute.ts +++ b/src/compile/nodes/Attribute.ts @@ -34,7 +34,7 @@ export default class Attribute extends Node { this.isSynthetic = false; this.expression = new Expression(component, this, scope, info.expression); - this.dependencies = this.expression.dynamic_dependencies; + this.dependencies = this.expression.dependencies; this.chunks = null; this.isDynamic = true; // TODO not necessarily @@ -59,7 +59,7 @@ export default class Attribute extends Node { const expression = new Expression(component, this, scope, node.expression); - addToSet(this.dependencies, expression.dynamic_dependencies); + addToSet(this.dependencies, expression.dependencies); return expression; }); @@ -73,6 +73,19 @@ export default class Attribute extends Node { } } + get_dependencies() { + if (this.isSpread) return this.expression.dynamic_dependencies(); + + const dependencies = new Set(); + this.chunks.forEach(chunk => { + if (chunk.type === 'Expression') { + addToSet(dependencies, chunk.dynamic_dependencies()); + } + }); + + return [...dependencies]; + } + getValue() { if (this.isTrue) return true; if (this.chunks.length === 0) return `""`; diff --git a/src/compile/nodes/Binding.ts b/src/compile/nodes/Binding.ts index 2432478750..85fcf7cf2d 100644 --- a/src/compile/nodes/Binding.ts +++ b/src/compile/nodes/Binding.ts @@ -2,6 +2,7 @@ import Node from './shared/Node'; import getObject from '../../utils/getObject'; import Expression from './shared/Expression'; import Component from '../Component'; +import TemplateScope from './shared/TemplateScope'; export default class Binding extends Node { name: string; @@ -10,7 +11,7 @@ export default class Binding extends Node { obj: string; prop: string; - constructor(component: Component, parent, scope, info) { + constructor(component: Component, parent, scope: TemplateScope, info) { super(component, parent, scope, info); if (info.expression.type !== 'Identifier' && info.expression.type !== 'MemberExpression') { @@ -30,7 +31,15 @@ export default class Binding extends Node { this.isContextual = scope.names.has(name); // make sure we track this as a mutable ref - scope.setMutable(name); + if (this.isContextual) { + scope.dependenciesForName.get(name).forEach(name => { + const variable = component.var_lookup.get(name); + variable.mutated = true; + }); + } else { + const variable = component.var_lookup.get(name); + variable.mutated = true; + } if (this.expression.node.type === 'MemberExpression') { prop = `[✂${this.expression.node.property.start}-${this.expression.node.property.end}✂]`; diff --git a/src/compile/nodes/shared/Expression.ts b/src/compile/nodes/shared/Expression.ts index b01a309e04..061ddebeb7 100644 --- a/src/compile/nodes/shared/Expression.ts +++ b/src/compile/nodes/shared/Expression.ts @@ -62,6 +62,7 @@ const precedence: Record number> = { }; export default class Expression { + type = 'Expression'; component: Component; owner: Wrapper; node: any; @@ -69,7 +70,6 @@ export default class Expression { references: Set; dependencies: Set = new Set(); contextual_dependencies: Set = new Set(); - dynamic_dependencies: Set = new Set(); template_scope: TemplateScope; scope: Scope; @@ -95,7 +95,7 @@ export default class Expression { this.owner = owner; this.is_synthetic = owner.isSynthetic; - const { dependencies, contextual_dependencies, dynamic_dependencies } = this; + const { dependencies, contextual_dependencies } = this; let { map, scope } = createScopes(info); this.scope = scope; @@ -104,27 +104,6 @@ export default class Expression { const expression = this; let function_expression; - function add_dependency(name, deep = false) { - dependencies.add(name); - - if (!function_expression) { - // dynamic_dependencies is used to create `if (changed.foo || ...)` - // conditions — it doesn't apply if the dependency is inside a - // function, and it only applies if the dependency is writable - // or a sub-path of a non-writable - if (component.ast.instance) { - const owner = template_scope.getOwner(name); - const is_let = owner && (owner.type === 'InlineComponent' || owner.type === 'Element'); - - if (is_let || component.writable_declarations.has(name) || name[0] === '$' || (component.var_lookup.has(name) && deep)) { - dynamic_dependencies.add(name); - } - } else { - dynamic_dependencies.add(name); - } - } - } - // discover dependencies, but don't change the code yet walk(info, { enter(node: any, parent: any, key: string) { @@ -150,11 +129,15 @@ export default class Expression { contextual_dependencies.add(name); - template_scope.dependenciesForName.get(name).forEach(name => add_dependency(name, true)); + if (!function_expression) { + template_scope.dependenciesForName.get(name).forEach(name => dependencies.add(name)); + } } else { - add_dependency(name, nodes.length > 1); - component.add_reference(name); + if (!function_expression) { + dependencies.add(name); + } + component.add_reference(name); component.warn_if_undefined(nodes[0], template_scope, true); } @@ -162,17 +145,33 @@ export default class Expression { } // track any assignments from template expressions as mutable + let mutated; if (function_expression) { if (node.type === 'AssignmentExpression') { - const names = node.left.type === 'MemberExpression' + mutated = node.left.type === 'MemberExpression' ? [getObject(node.left).name] : extractNames(node.left); - names.forEach(name => template_scope.setMutable(name)); } else if (node.type === 'UpdateExpression') { const { name } = getObject(node.argument); - template_scope.setMutable(name); + mutated = [name]; } } + + if (mutated) { + mutated.forEach(name => { + if (template_scope.names.has(name)) { + template_scope.dependenciesForName.get(name).forEach(name => { + const variable = component.var_lookup.get(name); + if (variable) variable.mutated = true; + }); + } else { + component.add_reference(name); + + const variable = component.var_lookup.get(name); + if (variable) variable.mutated = true; + } + }); + } }, leave(node) { @@ -187,6 +186,17 @@ export default class Expression { }); } + dynamic_dependencies() { + return Array.from(this.dependencies).filter(name => { + const owner = this.template_scope.getOwner(name); + const is_let = owner && (owner.type === 'InlineComponent' || owner.type === 'Element'); + if (is_let) return true; + + const variable = this.component.var_lookup.get(name); + return variable.mutated; + }); + } + getPrecedence() { return this.node.type in precedence ? precedence[this.node.type](this.node) : 0; } diff --git a/src/compile/nodes/shared/TemplateScope.ts b/src/compile/nodes/shared/TemplateScope.ts index df48cc57f9..0a6be650f1 100644 --- a/src/compile/nodes/shared/TemplateScope.ts +++ b/src/compile/nodes/shared/TemplateScope.ts @@ -31,14 +31,6 @@ export default class TemplateScope { return child; } - setMutable(name: string) { - if (this.names.has(name)) { - this.mutables.add(name); - if (this.parent && this.dependenciesForName.has(name)) this.dependenciesForName.get(name).forEach(dep => this.parent.setMutable(dep)); - } else if (this.parent) this.parent.setMutable(name); - else this.mutables.add(name); - } - containsMutable(names: Iterable) { for (const name of names) { const owner = this.getOwner(name); diff --git a/src/compile/render-dom/wrappers/AwaitBlock.ts b/src/compile/render-dom/wrappers/AwaitBlock.ts index a5926fa8d7..22247a14e1 100644 --- a/src/compile/render-dom/wrappers/AwaitBlock.ts +++ b/src/compile/render-dom/wrappers/AwaitBlock.ts @@ -67,7 +67,7 @@ export default class AwaitBlockWrapper extends Wrapper { this.cannotUseInnerHTML(); - block.addDependencies(this.node.expression.dynamic_dependencies); + block.addDependencies(this.node.expression.dependencies); let isDynamic = false; let hasIntros = false; diff --git a/src/compile/render-dom/wrappers/DebugTag.ts b/src/compile/render-dom/wrappers/DebugTag.ts index cec40baaf7..294276a4ea 100644 --- a/src/compile/render-dom/wrappers/DebugTag.ts +++ b/src/compile/render-dom/wrappers/DebugTag.ts @@ -45,7 +45,7 @@ export default class DebugTagWrapper extends Wrapper { const dependencies = new Set(); this.node.expressions.forEach(expression => { - addToSet(dependencies, expression.dynamic_dependencies); + addToSet(dependencies, expression.dependencies); }); const condition = [...dependencies].map(d => `changed.${d}`).join(' || '); diff --git a/src/compile/render-dom/wrappers/EachBlock.ts b/src/compile/render-dom/wrappers/EachBlock.ts index f756a9ce6e..0fb1018ccf 100644 --- a/src/compile/render-dom/wrappers/EachBlock.ts +++ b/src/compile/render-dom/wrappers/EachBlock.ts @@ -74,8 +74,8 @@ export default class EachBlockWrapper extends Wrapper { super(renderer, block, parent, node); this.cannotUseInnerHTML(); - const { dynamic_dependencies } = node.expression; - block.addDependencies(dynamic_dependencies); + const { dependencies } = node.expression; + block.addDependencies(dependencies); this.block = block.child({ comment: createDebuggingComment(this.node, this.renderer.component), diff --git a/src/compile/render-dom/wrappers/Element/Attribute.ts b/src/compile/render-dom/wrappers/Element/Attribute.ts index dbb53b665f..ba4186f360 100644 --- a/src/compile/render-dom/wrappers/Element/Attribute.ts +++ b/src/compile/render-dom/wrappers/Element/Attribute.ts @@ -160,8 +160,8 @@ export default class AttributeWrapper { } // only add an update if mutations are involved (or it's a select?) - if (this.node.parent.scope.containsMutable(this.node.dependencies) || isSelectValueAttribute) { - const dependencies = Array.from(this.node.dependencies); + const dependencies = this.node.get_dependencies(); + if (dependencies.length > 0 || isSelectValueAttribute) { const changedCheck = ( (block.hasOutros ? `!#current || ` : '') + dependencies.map(dependency => `changed.${dependency}`).join(' || ') diff --git a/src/compile/render-dom/wrappers/Element/Binding.ts b/src/compile/render-dom/wrappers/Element/Binding.ts index 710b6e8c6d..952b2bfe08 100644 --- a/src/compile/render-dom/wrappers/Element/Binding.ts +++ b/src/compile/render-dom/wrappers/Element/Binding.ts @@ -37,14 +37,14 @@ export default class BindingWrapper { this.node = node; this.parent = parent; - const { dynamic_dependencies } = this.node.expression; + const { dependencies } = this.node.expression; - block.addDependencies(dynamic_dependencies); + block.addDependencies(dependencies); // TODO does this also apply to e.g. ``? if (parent.node.name === 'select') { - parent.selectBindingDependencies = dynamic_dependencies; - dynamic_dependencies.forEach((prop: string) => { + parent.selectBindingDependencies = dependencies; + dependencies.forEach((prop: string) => { parent.renderer.component.indirectDependencies.set(prop, new Set()); }); } @@ -106,7 +106,7 @@ export default class BindingWrapper { let updateConditions: string[] = this.needsLock ? [`!${lock}`] : []; - const dependencyArray = [...this.node.expression.dynamic_dependencies] + const dependencyArray = [...this.node.expression.dependencies] if (dependencyArray.length === 1) { updateConditions.push(`changed.${dependencyArray[0]}`) diff --git a/src/compile/render-dom/wrappers/Element/StyleAttribute.ts b/src/compile/render-dom/wrappers/Element/StyleAttribute.ts index 7af2a8dc79..2ec4a1c98e 100644 --- a/src/compile/render-dom/wrappers/Element/StyleAttribute.ts +++ b/src/compile/render-dom/wrappers/Element/StyleAttribute.ts @@ -35,7 +35,7 @@ export default class StyleAttributeWrapper extends AttributeWrapper { } else { const snippet = chunk.render(); - addToSet(propDependencies, chunk.dynamic_dependencies); + addToSet(propDependencies, chunk.dependencies); return chunk.getPrecedence() <= 13 ? `(${snippet})` : snippet; } diff --git a/src/compile/render-dom/wrappers/Element/index.ts b/src/compile/render-dom/wrappers/Element/index.ts index cf7286d58d..8f3e92e104 100644 --- a/src/compile/render-dom/wrappers/Element/index.ts +++ b/src/compile/render-dom/wrappers/Element/index.ts @@ -173,13 +173,13 @@ export default class ElementWrapper extends Wrapper { // add directive and handler dependencies [node.animation, node.outro, ...node.actions, ...node.classes].forEach(directive => { if (directive && directive.expression) { - block.addDependencies(directive.expression.dynamic_dependencies); + block.addDependencies(directive.expression.dependencies); } }); node.handlers.forEach(handler => { if (handler.expression) { - block.addDependencies(handler.expression.dynamic_dependencies); + block.addDependencies(handler.expression.dependencies); } }); diff --git a/src/compile/render-dom/wrappers/IfBlock.ts b/src/compile/render-dom/wrappers/IfBlock.ts index c3a8157dbe..ce2d5fda0f 100644 --- a/src/compile/render-dom/wrappers/IfBlock.ts +++ b/src/compile/render-dom/wrappers/IfBlock.ts @@ -87,7 +87,7 @@ export default class IfBlockWrapper extends Wrapper { this.branches.push(branch); blocks.push(branch.block); - block.addDependencies(node.expression.dynamic_dependencies); + block.addDependencies(node.expression.dependencies); if (branch.block.dependencies.size > 0) { isDynamic = true; diff --git a/src/compile/render-dom/wrappers/InlineComponent/index.ts b/src/compile/render-dom/wrappers/InlineComponent/index.ts index 83f41845dc..1b90aa2f69 100644 --- a/src/compile/render-dom/wrappers/InlineComponent/index.ts +++ b/src/compile/render-dom/wrappers/InlineComponent/index.ts @@ -35,7 +35,7 @@ export default class InlineComponentWrapper extends Wrapper { this.cannotUseInnerHTML(); if (this.node.expression) { - block.addDependencies(this.node.expression.dynamic_dependencies); + block.addDependencies(this.node.expression.dependencies); } this.node.attributes.forEach(attr => { @@ -52,12 +52,12 @@ export default class InlineComponentWrapper extends Wrapper { (eachBlock as EachBlock).has_binding = true; } - block.addDependencies(binding.expression.dynamic_dependencies); + block.addDependencies(binding.expression.dependencies); }); this.node.handlers.forEach(handler => { if (handler.expression) { - block.addDependencies(handler.expression.dynamic_dependencies); + block.addDependencies(handler.expression.dependencies); } }); @@ -147,7 +147,10 @@ export default class InlineComponentWrapper extends Wrapper { const fragment_dependencies = new Set(); this.slots.forEach(slot => { slot.block.dependencies.forEach(name => { - if (renderer.component.mutable_props.has(name)) { + const is_let = this.node.lets.some(l => l.names.indexOf(name) !== -1); + const variable = renderer.component.var_lookup.get(name); + + if (is_let || variable.mutated) { fragment_dependencies.add(name); } }); @@ -279,7 +282,7 @@ export default class InlineComponentWrapper extends Wrapper { ); updates.push(deindent` - if (!${updating} && ${[...binding.expression.dynamic_dependencies].map((dependency: string) => `changed.${dependency}`).join(' || ')}) { + if (!${updating} && ${[...binding.expression.dependencies].map((dependency: string) => `changed.${dependency}`).join(' || ')}) { ${name_changes}${quotePropIfNecessary(binding.name)} = ${snippet}; } `); diff --git a/src/compile/render-dom/wrappers/Title.ts b/src/compile/render-dom/wrappers/Title.ts index 733e4c845d..d82d950639 100644 --- a/src/compile/render-dom/wrappers/Title.ts +++ b/src/compile/render-dom/wrappers/Title.ts @@ -34,7 +34,7 @@ export default class TitleWrapper extends Wrapper { // single {tag} — may be a non-string const { expression } = this.node.children[0]; value = expression.render(block); - addToSet(allDependencies, expression.dynamic_dependencies); + addToSet(allDependencies, expression.dependencies); } else { // '{foo} {bar}' — treat as string concatenation value = @@ -46,7 +46,7 @@ export default class TitleWrapper extends Wrapper { } else { const snippet = chunk.expression.render(block); - chunk.expression.dynamic_dependencies.forEach(d => { + chunk.expression.dependencies.forEach(d => { allDependencies.add(d); }); diff --git a/src/compile/render-dom/wrappers/shared/Tag.ts b/src/compile/render-dom/wrappers/shared/Tag.ts index 475c0d8a70..8e72929acd 100644 --- a/src/compile/render-dom/wrappers/shared/Tag.ts +++ b/src/compile/render-dom/wrappers/shared/Tag.ts @@ -11,14 +11,14 @@ export default class Tag extends Wrapper { super(renderer, block, parent, node); this.cannotUseInnerHTML(); - block.addDependencies(node.expression.dynamic_dependencies); + block.addDependencies(node.expression.dependencies); } renameThisMethod( block: Block, update: ((value: string) => string) ) { - const dependencies = this.node.expression.dynamic_dependencies; + const dependencies = this.node.expression.dynamic_dependencies(); const snippet = this.node.expression.render(block); const value = this.node.shouldCache && block.getUniqueName(`${this.var}_value`); @@ -26,27 +26,22 @@ export default class Tag extends Wrapper { if (this.node.shouldCache) block.addVariable(value, snippet); - if (dependencies.size) { + if (dependencies.length > 0) { const changedCheck = ( (block.hasOutros ? `!#current || ` : '') + - [...dependencies].map((dependency: string) => `changed.${dependency}`).join(' || ') + dependencies.map((dependency: string) => `changed.${dependency}`).join(' || ') ); const updateCachedValue = `${value} !== (${value} = ${snippet})`; const condition =this.node.shouldCache - ? dependencies.size > 0 - ? `(${changedCheck}) && ${updateCachedValue}` - : updateCachedValue + ? `(${changedCheck}) && ${updateCachedValue}` : changedCheck; - // only update if there's a mutation involved - if (this.node.expression.template_scope.containsMutable(dependencies)) { - block.builders.update.addConditional( - condition, - update(content) - ); - } + block.builders.update.addConditional( + condition, + update(content) + ); } return { init: content }; diff --git a/src/compile/render-dom/wrappers/shared/addActions.ts b/src/compile/render-dom/wrappers/shared/addActions.ts index 0dc037bb39..d5732e57bc 100644 --- a/src/compile/render-dom/wrappers/shared/addActions.ts +++ b/src/compile/render-dom/wrappers/shared/addActions.ts @@ -1,4 +1,3 @@ -import Renderer from '../../Renderer'; import Block from '../../Block'; import Action from '../../../nodes/Action'; import Component from '../../../Component'; @@ -15,7 +14,7 @@ export default function addActions( if (expression) { snippet = expression.render(block); - dependencies = expression.dynamic_dependencies; + dependencies = expression.dynamic_dependencies(); } const name = block.getUniqueName( @@ -33,10 +32,10 @@ export default function addActions( `${name} = ${fn}.call(null, ${target}${snippet ? `, ${snippet}` : ''}) || {};` ); - if (dependencies && dependencies.size > 0) { + if (dependencies && dependencies.length > 0) { let conditional = `typeof ${name}.update === 'function' && `; - const deps = [...dependencies].map(dependency => `changed.${dependency}`).join(' || '); - conditional += dependencies.size > 1 ? `(${deps})` : deps; + const deps = dependencies.map(dependency => `changed.${dependency}`).join(' || '); + conditional += dependencies.length > 1 ? `(${deps})` : deps; block.builders.update.addConditional( conditional,