diff --git a/src/compile/Component.ts b/src/compile/Component.ts index c9c985339f..9b7dbbb381 100644 --- a/src/compile/Component.ts +++ b/src/compile/Component.ts @@ -77,6 +77,7 @@ 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(); @@ -169,6 +170,7 @@ export default class Component { if (!this.instance_script) { const props = [...this.template_references]; this.declarations.push(...props); + addToSet(this.mutable_props, this.template_references); addToSet(this.writable_declarations, this.template_references); addToSet(this.userVars, this.template_references); @@ -177,6 +179,9 @@ export default class Component { as: name })); } + + // 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)); } addSourcemapLocations(node: Node) { @@ -405,6 +410,7 @@ export default class Component { node.declaration.declarations.forEach(declarator => { extractNames(declarator.id).forEach(name => { exports.push({ name, as: name }); + this.mutable_props.add(name); }); }); } else { @@ -537,6 +543,7 @@ export default class Component { this.userVars.add(name); }); + this.track_mutations(); this.extract_imports_and_exports(script.content, this.imports, this.props); this.hoist_instance_declarations(); this.extract_reactive_declarations(); @@ -544,6 +551,35 @@ export default class Component { this.javascript = this.extract_javascript(script); } + // TODO merge this with other walks that are independent + track_mutations() { + const component = this; + let { instance_scope: scope, instance_scope_map: map } = this; + + walk(this.instance_script.content, { + enter(node, parent) { + let names; + if (map.has(node)) { + scope = map.get(node); + } + + if (node.type === 'AssignmentExpression') { + names = node.left.type === 'MemberExpression' + ? [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); + }); + } + } + }) + } + extract_reactive_store_references() { // TODO this pattern happens a lot... can we abstract it // (or better still, do fewer AST walks)? @@ -586,6 +622,7 @@ 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 = []; diff --git a/src/compile/nodes/Binding.ts b/src/compile/nodes/Binding.ts index 3d26d5f34c..2432478750 100644 --- a/src/compile/nodes/Binding.ts +++ b/src/compile/nodes/Binding.ts @@ -29,6 +29,9 @@ export default class Binding extends Node { const { name } = getObject(this.expression.node); this.isContextual = scope.names.has(name); + // make sure we track this as a mutable ref + scope.setMutable(name); + if (this.expression.node.type === 'MemberExpression') { prop = `[✂${this.expression.node.property.start}-${this.expression.node.property.end}✂]`; if (!this.expression.node.computed) prop = `'${prop}'`; diff --git a/src/compile/nodes/shared/Expression.ts b/src/compile/nodes/shared/Expression.ts index e45b282bfe..5e223cd53d 100644 --- a/src/compile/nodes/shared/Expression.ts +++ b/src/compile/nodes/shared/Expression.ts @@ -104,15 +104,16 @@ export default class Expression { const expression = this; let function_expression; - function add_dependency(name) { + 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.instance_script) { - if (component.writable_declarations.has(name) || name[0] === '$') { + if (component.writable_declarations.has(name) || name[0] === '$' || (component.userVars.has(name) && deep)) { dynamic_dependencies.add(name); } } else { @@ -146,9 +147,9 @@ export default class Expression { contextual_dependencies.add(name); - template_scope.dependenciesForName.get(name).forEach(add_dependency); + template_scope.dependenciesForName.get(name).forEach(name => add_dependency(name, true)); } else { - add_dependency(name); + add_dependency(name, nodes.length > 1); component.template_references.add(name); component.warn_if_undefined(nodes[0], template_scope, true); @@ -156,6 +157,19 @@ export default class Expression { this.skip(); } + + // track any assignments from template expressions as mutable + if (function_expression) { + if (node.type === 'AssignmentExpression') { + const names = 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); + } + } }, leave(node) { diff --git a/src/compile/nodes/shared/TemplateScope.ts b/src/compile/nodes/shared/TemplateScope.ts index a5a7ec90a2..45a8895505 100644 --- a/src/compile/nodes/shared/TemplateScope.ts +++ b/src/compile/nodes/shared/TemplateScope.ts @@ -1,10 +1,14 @@ export default class TemplateScope { names: Set; dependenciesForName: Map; + mutables: Set; + parent?: TemplateScope; constructor(parent?: TemplateScope) { + this.parent = parent; this.names = new Set(parent ? parent.names : []); this.dependenciesForName = new Map(parent ? parent.dependenciesForName : []); + this.mutables = new Set(); } add(name, dependencies) { @@ -14,6 +18,24 @@ export default class TemplateScope { } child() { - return new TemplateScope(this); + const child = new TemplateScope(this); + return child; + } + + setMutable(name: string) { + if (this.names.has(name)) this.mutables.add(name); + else if (this.parent) this.parent.setMutable(name); + else this.mutables.add(name); + } + + containsMutable(names: Iterable) { + for (const name of names) { + if (name[0] === '$') return true; + if (this.mutables.has(name)) return true; + else if (this.dependenciesForName.has(name) && this.containsMutable(this.dependenciesForName.get(name))) return true; + } + + if (this.parent) return this.parent.containsMutable(names); + else return false; } } \ No newline at end of file diff --git a/src/compile/render-dom/wrappers/Element/Attribute.ts b/src/compile/render-dom/wrappers/Element/Attribute.ts index bec0fa35a4..dbb53b665f 100644 --- a/src/compile/render-dom/wrappers/Element/Attribute.ts +++ b/src/compile/render-dom/wrappers/Element/Attribute.ts @@ -159,7 +159,8 @@ export default class AttributeWrapper { updater = `${method}(${element.var}, "${name}", ${shouldCache ? last : value});`; } - if (this.node.dependencies.size || isSelectValueAttribute) { + // 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 changedCheck = ( (block.hasOutros ? `!#current || ` : '') + diff --git a/src/compile/render-dom/wrappers/shared/Tag.ts b/src/compile/render-dom/wrappers/shared/Tag.ts index 510c2cf756..d44a53ffde 100644 --- a/src/compile/render-dom/wrappers/shared/Tag.ts +++ b/src/compile/render-dom/wrappers/shared/Tag.ts @@ -40,10 +40,13 @@ export default class Tag extends Wrapper { : updateCachedValue : changedCheck; - block.builders.update.addConditional( - condition, - update(content) - ); + // only update if there's a mutation involved + if (this.node.expression.template_scope.containsMutable(dependencies)) { + block.builders.update.addConditional( + condition, + update(content) + ); + } } return { init: content }; diff --git a/test/js/samples/non-mutable-reference/expected.js b/test/js/samples/non-mutable-reference/expected.js new file mode 100644 index 0000000000..57f6f3fea1 --- /dev/null +++ b/test/js/samples/non-mutable-reference/expected.js @@ -0,0 +1,53 @@ +/* generated by Svelte vX.Y.Z */ +import { SvelteComponent as SvelteComponent_1, append, createElement, createText, detachNode, init, insert, noop, run, safe_not_equal } from "svelte/internal"; + +function create_fragment($$, ctx) { + var h1, text0, text1, text2, current; + + return { + c() { + h1 = createElement("h1"); + text0 = createText("Hello "); + text1 = createText(ctx.name); + text2 = createText("!"); + }, + + m(target, anchor) { + insert(target, h1, anchor); + append(h1, text0); + append(h1, text1); + append(h1, text2); + current = true; + }, + + p: noop, + + i(target, anchor) { + if (current) return; + this.m(target, anchor); + }, + + o: run, + + d(detach) { + if (detach) { + detachNode(h1); + } + } + }; +} + +function instance($$self) { + let name = 'world'; + + return { name }; +} + +class SvelteComponent extends SvelteComponent_1 { + constructor(options) { + super(); + init(this, options, instance, create_fragment, safe_not_equal); + } +} + +export default SvelteComponent; \ No newline at end of file diff --git a/test/js/samples/non-mutable-reference/input.html b/test/js/samples/non-mutable-reference/input.html new file mode 100644 index 0000000000..22b3c84db0 --- /dev/null +++ b/test/js/samples/non-mutable-reference/input.html @@ -0,0 +1,5 @@ + + +

Hello {name}!

\ No newline at end of file