diff --git a/src/compiler/compile/Component.ts b/src/compiler/compile/Component.ts index 68efd7f76332..9f2a62fe9342 100644 --- a/src/compiler/compile/Component.ts +++ b/src/compiler/compile/Component.ts @@ -813,7 +813,7 @@ export default class Component { const variable = this.var_lookup.get(name); if (variable && (variable.subscribable && variable.reassigned)) { - return `$$subscribe_${name}(), $$invalidate('${name}', ${value || name})`; + return `$$subscribe_${name}($$invalidate('${name}', ${value || name}))`; } if (name[0] === '$' && name[1] !== '$') { diff --git a/src/compiler/compile/nodes/shared/Expression.ts b/src/compiler/compile/nodes/shared/Expression.ts index d054416717d1..a5a730c4d199 100644 --- a/src/compiler/compile/nodes/shared/Expression.ts +++ b/src/compiler/compile/nodes/shared/Expression.ts @@ -7,13 +7,12 @@ import { Node } from '../../../interfaces'; import { globals , sanitize } from '../../../utils/names'; import deindent from '../../utils/deindent'; import Wrapper from '../../render_dom/wrappers/shared/Wrapper'; - import TemplateScope from './TemplateScope'; import get_object from '../../utils/get_object'; -import { nodes_match } from '../../../utils/nodes_match'; import Block from '../../render_dom/Block'; import { INode } from '../interfaces'; import is_dynamic from '../../render_dom/wrappers/shared/is_dynamic'; +import { invalidate } from '../../utils/invalidate'; const binary_operators: Record = { '**': 15, @@ -241,7 +240,6 @@ export default class Expression { const { code } = component; let function_expression; - let pending_assignments: Set = new Set(); let dependencies: Set; let contextual_dependencies: Set; @@ -309,16 +307,6 @@ export default class Expression { if (map.has(node)) scope = scope.parent; if (node === function_expression) { - if (pending_assignments.size > 0) { - if (node.type !== 'ArrowFunctionExpression') { - // this should never happen! - throw new Error(`Well that's odd`); - } - - // TOOD optimisation — if this is an event handler, - // the return value doesn't matter - } - const name = component.get_unique_name( sanitize(get_function_name(node, owner)) ); @@ -334,40 +322,11 @@ export default class Expression { args.push(original_params); } - let body = code.slice(node.body.start, node.body.end).trim(); - if (node.body.type !== 'BlockStatement') { - if (pending_assignments.size > 0) { - const dependencies = new Set(); - pending_assignments.forEach(name => { - if (template_scope.names.has(name)) { - template_scope.dependencies_for_name.get(name).forEach(dependency => { - dependencies.add(dependency); - }); - } else { - dependencies.add(name); - } - }); - - const insert = Array.from(dependencies).map(name => component.invalidate(name)).join('; '); - pending_assignments = new Set(); + const body = code.slice(node.body.start, node.body.end).trim(); - component.has_reactive_assignments = true; - - body = deindent` - { - const $$result = ${body}; - ${insert}; - return $$result; - } - `; - } else { - body = `{\n\treturn ${body};\n}`; - } - } - - const fn = deindent` - ${node.async && 'async '}function${node.generator && '*'} ${name}(${args.join(', ')}) ${body} - `; + const fn = node.type === 'FunctionExpression' + ? `${node.async ? 'async ' : ''}function${node.generator ? '*' : ''} ${name}(${args.join(', ')}) ${body}` + : `const ${name} = ${node.async ? 'async ' : ''}(${args.join(', ')}) => ${body};`; if (dependencies.size === 0 && contextual_dependencies.size === 0) { // we can hoist this out of the component completely @@ -421,66 +380,26 @@ export default class Expression { contextual_dependencies = null; } - if (node.type === 'AssignmentExpression') { - const names = node.left.type === 'MemberExpression' - ? [get_object(node.left).name] - : extract_names(node.left); + if (node.type === 'AssignmentExpression' || node.type === 'UpdateExpression') { + const assignee = node.type === 'AssignmentExpression' ? node.left : node.argument; - if (node.operator === '=' && nodes_match(node.left, node.right)) { - const dirty = names.filter(name => { - return !scope.declarations.has(name); - }); - - if (dirty.length) component.has_reactive_assignments = true; + // normally (`a = 1`, `b.c = 2`), there'll be a single name + // (a or b). In destructuring cases (`[d, e] = [e, d]`) there + // may be more, in which case we need to tack the extra ones + // onto the initial function call + const names = new Set(extract_names(assignee)); - code.overwrite(node.start, node.end, dirty.map(n => component.invalidate(n)).join('; ')); - } else { - names.forEach(name => { - if (scope.declarations.has(name)) return; - - const variable = component.var_lookup.get(name); - if (variable && variable.hoistable) return; - - pending_assignments.add(name); - }); - } - } else if (node.type === 'UpdateExpression') { - const { name } = get_object(node.argument); - - if (scope.declarations.has(name)) return; - - const variable = component.var_lookup.get(name); - if (variable && variable.hoistable) return; - - pending_assignments.add(name); - } - - if (/Statement/.test(node.type)) { - if (pending_assignments.size > 0) { - const has_semi = code.original[node.end - 1] === ';'; - - const insert = ( - (has_semi ? ' ' : '; ') + - Array.from(pending_assignments).map(name => component.invalidate(name)).join('; ') - ); - - if (/^(Break|Continue|Return)Statement/.test(node.type)) { - if (node.argument) { - code.overwrite(node.start, node.argument.start, `var $$result = `); - code.appendLeft(node.end, `${insert}; return $$result`); - } else { - code.prependRight(node.start, `${insert}; `); - } - } else if (parent && /(If|For(In|Of)?|While)Statement/.test(parent.type) && node.type !== 'BlockStatement') { - code.prependRight(node.start, '{ '); - code.appendLeft(node.end, `${insert}; }`); + const traced: Set = new Set(); + names.forEach(name => { + const dependencies = template_scope.dependencies_for_name.get(name); + if (dependencies) { + dependencies.forEach(name => traced.add(name)); } else { - code.appendLeft(node.end, `${insert};`); + traced.add(name); } + }); - component.has_reactive_assignments = true; - pending_assignments = new Set(); - } + invalidate(component, scope, code, node, traced); } } }); diff --git a/src/compiler/compile/render_dom/index.ts b/src/compiler/compile/render_dom/index.ts index 4895c9774807..38021169491e 100644 --- a/src/compiler/compile/render_dom/index.ts +++ b/src/compiler/compile/render_dom/index.ts @@ -7,9 +7,8 @@ import { CompileOptions } from '../../interfaces'; import { walk } from 'estree-walker'; import { stringify_props } from '../utils/stringify_props'; import add_to_set from '../utils/add_to_set'; -import get_object from '../utils/get_object'; import { extract_names } from '../utils/scope'; -import { nodes_match } from '../../utils/nodes_match'; +import { invalidate } from '../utils/invalidate'; export default function dom( component: Component, @@ -158,8 +157,6 @@ export default function dom( let scope = component.instance_scope; const map = component.instance_scope_map; - let pending_assignments = new Set(); - walk(component.ast.instance.content, { enter: (node) => { if (map.has(node)) { @@ -167,102 +164,26 @@ export default function dom( } }, - leave(node, parent) { + leave(node) { if (map.has(node)) { scope = scope.parent; } + // TODO dry out — most of this is shared with Expression.ts if (node.type === 'AssignmentExpression' || node.type === 'UpdateExpression') { const assignee = node.type === 'AssignmentExpression' ? node.left : node.argument; - let names = []; - if (assignee.type === 'MemberExpression') { - const left_object_name = get_object(assignee).name; - left_object_name && (names = [left_object_name]); - } else { - names = extract_names(assignee); - } + // normally (`a = 1`, `b.c = 2`), there'll be a single name + // (a or b). In destructuring cases (`[d, e] = [e, d]`) there + // may be more, in which case we need to tack the extra ones + // onto the initial function call + const names = new Set(extract_names(assignee)); - if (node.operator === '=' && nodes_match(node.left, node.right)) { - const dirty = names.filter(name => { - return name[0] === '$' || scope.find_owner(name) === component.instance_scope; - }); - - if (dirty.length) component.has_reactive_assignments = true; - - code.overwrite(node.start, node.end, dirty.map(n => component.invalidate(n)).join('; ')); - } else { - const single = ( - node.type === 'AssignmentExpression' && - assignee.type === 'Identifier' && - parent.type === 'ExpressionStatement' && - assignee.name[0] !== '$' - ); - - names.forEach(name => { - const owner = scope.find_owner(name); - if (owner && owner !== component.instance_scope) return; - - const variable = component.var_lookup.get(name); - if (variable && (variable.hoistable || variable.global || variable.module)) return; - - if (single && !(variable.subscribable && variable.reassigned)) { - if (variable.referenced || variable.is_reactive_dependency || variable.export_name) { - code.prependRight(node.start, `$$invalidate('${name}', `); - code.appendLeft(node.end, `)`); - } - } else { - pending_assignments.add(name); - } - - component.has_reactive_assignments = true; - }); - } - } - - if (pending_assignments.size > 0) { - if (node.type === 'ArrowFunctionExpression') { - const insert = Array.from(pending_assignments).map(name => component.invalidate(name)).join('; '); - pending_assignments = new Set(); - - code.prependRight(node.body.start, `{ const $$result = `); - code.appendLeft(node.body.end, `; ${insert}; return $$result; }`); - - pending_assignments = new Set(); - } - - else if (/Statement/.test(node.type)) { - const insert = Array.from(pending_assignments).map(name => component.invalidate(name)).join('; '); - - if (/^(Break|Continue|Return)Statement/.test(node.type)) { - if (node.argument) { - code.overwrite(node.start, node.argument.start, `var $$result = `); - code.appendLeft(node.argument.end, `; ${insert}; return $$result`); - } else { - code.prependRight(node.start, `${insert}; `); - } - } else if (parent && /(If|For(In|Of)?|While)Statement/.test(parent.type) && node.type !== 'BlockStatement') { - code.prependRight(node.start, '{ '); - code.appendLeft(node.end, `${code.original[node.end - 1] === ';' ? '' : ';'} ${insert}; }`); - } else { - code.appendLeft(node.end, `${code.original[node.end - 1] === ';' ? '' : ';'} ${insert};`); - } - - pending_assignments = new Set(); - } else if (parent && parent.type !== 'ForStatement' && node.type === 'VariableDeclaration') { - const insert = Array.from(pending_assignments).map(name => component.invalidate(name)).join('; '); - - code.appendLeft(node.end, `${code.original[node.end - 1] === ';' ? '' : ';'} ${insert};`); - pending_assignments = new Set(); - } + invalidate(component, scope, code, node, names); } } }); - if (pending_assignments.size > 0) { - throw new Error(`TODO this should not happen!`); - } - component.rewrite_props(({ name, reassigned }) => { const value = `$${name}`; @@ -395,7 +316,7 @@ export default function dom( const store = component.var_lookup.get(name); if (store && store.reassigned) { - return `${$name}, $$unsubscribe_${name} = @noop, $$subscribe_${name} = () => { $$unsubscribe_${name}(); $$unsubscribe_${name} = @subscribe(${name}, $$value => { ${$name} = $$value; $$invalidate('${$name}', ${$name}); }) }`; + return `${$name}, $$unsubscribe_${name} = @noop, $$subscribe_${name} = () => ($$unsubscribe_${name}(), $$unsubscribe_${name} = @subscribe(${name}, $$value => { ${$name} = $$value; $$invalidate('${$name}', ${$name}); }), ${name})`; } return $name; diff --git a/src/compiler/compile/utils/invalidate.ts b/src/compiler/compile/utils/invalidate.ts new file mode 100644 index 000000000000..0c93ce269445 --- /dev/null +++ b/src/compiler/compile/utils/invalidate.ts @@ -0,0 +1,65 @@ +import Component from '../Component'; +import MagicString from 'magic-string'; +import { Node } from '../../interfaces'; +import { nodes_match } from '../../utils/nodes_match'; +import { Scope } from './scope'; + +export function invalidate(component: Component, scope: Scope, code: MagicString, node: Node, names: Set) { + const [head, ...tail] = Array.from(names).filter(name => { + const owner = scope.find_owner(name); + if (owner && owner !== component.instance_scope) return false; + + const variable = component.var_lookup.get(name); + + return variable && ( + !variable.hoistable && + !variable.global && + !variable.module && + ( + variable.referenced || + variable.is_reactive_dependency || + variable.export_name + ) + ); + }); + + if (head) { + component.has_reactive_assignments = true; + + if (node.operator === '=' && nodes_match(node.left, node.right) && tail.length === 0) { + code.overwrite(node.start, node.end, component.invalidate(head)); + } else { + let suffix = ')'; + + if (head[0] === '$') { + code.prependRight(node.start, `${component.helper('set_store_value')}(${head.slice(1)}, `); + } else { + let prefix = `$$invalidate`; + + const variable = component.var_lookup.get(head); + if (variable.subscribable && variable.reassigned) { + prefix = `$$subscribe_${head}($$invalidate`; + suffix += `)`; + } + + code.prependRight(node.start, `${prefix}('${head}', `); + } + + const extra_args = tail.map(name => component.invalidate(name)); + + const pass_value = ( + extra_args.length > 0 || + (node.type === 'AssignmentExpression' && node.left.type !== 'Identifier') || + (node.type === 'UpdateExpression' && !node.prefix) + ); + + if (pass_value) { + extra_args.unshift(head); + } + + suffix = `${extra_args.map(arg => `, ${arg}`).join('')}${suffix}`; + + code.appendLeft(node.end, suffix); + } + } +} \ No newline at end of file diff --git a/src/runtime/internal/Component.ts b/src/runtime/internal/Component.ts index 92e227e57cc6..d5b4f83bee96 100644 --- a/src/runtime/internal/Component.ts +++ b/src/runtime/internal/Component.ts @@ -101,11 +101,12 @@ export function init(component, options, instance, create_fragment, not_equal, p let ready = false; $$.ctx = instance - ? instance(component, props, (key, value) => { + ? instance(component, props, (key, ret, value = ret) => { if ($$.ctx && not_equal($$.ctx[key], $$.ctx[key] = value)) { if ($$.bound[key]) $$.bound[key](value); if (ready) make_dirty(component, key); } + return ret; }) : props; diff --git a/src/runtime/internal/utils.ts b/src/runtime/internal/utils.ts index e457eb050596..d267b73cee20 100644 --- a/src/runtime/internal/utils.ts +++ b/src/runtime/internal/utils.ts @@ -100,3 +100,8 @@ export function once(fn) { export function null_to_empty(value) { return value == null ? '' : value; } + +export function set_store_value(store, ret, value = ret) { + store.set(value); + return ret; +} diff --git a/test/js/samples/action-custom-event-handler/expected.js b/test/js/samples/action-custom-event-handler/expected.js index ea55f1629b2f..34d5ca10aa38 100644 --- a/test/js/samples/action-custom-event-handler/expected.js +++ b/test/js/samples/action-custom-event-handler/expected.js @@ -53,9 +53,7 @@ function foo(node, callback) { function instance($$self, $$props, $$invalidate) { let { bar } = $$props; - function foo_function() { - return handleFoo(bar); - } + const foo_function = () => handleFoo(bar); $$self.$set = $$props => { if ('bar' in $$props) $$invalidate('bar', bar = $$props.bar); diff --git a/test/js/samples/dynamic-import/expected.js b/test/js/samples/dynamic-import/expected.js index 9a6a67bcd913..8692cd89b26f 100644 --- a/test/js/samples/dynamic-import/expected.js +++ b/test/js/samples/dynamic-import/expected.js @@ -46,9 +46,7 @@ function create_fragment(ctx) { }; } -function func() { - return import('./Foo.svelte'); -} +const func = () => import('./Foo.svelte'); class Component extends SvelteComponent { constructor(options) { diff --git a/test/js/samples/event-handler-no-passive/expected.js b/test/js/samples/event-handler-no-passive/expected.js index e8ca72c556d2..69285a29c6c4 100644 --- a/test/js/samples/event-handler-no-passive/expected.js +++ b/test/js/samples/event-handler-no-passive/expected.js @@ -40,9 +40,7 @@ function create_fragment(ctx) { }; } -function touchstart_handler(e) { - return e.preventDefault(); -} +const touchstart_handler = (e) => e.preventDefault(); class Component extends SvelteComponent { constructor(options) { diff --git a/test/js/samples/instrumentation-template-if-no-block/expected.js b/test/js/samples/instrumentation-template-if-no-block/expected.js index 1b8cbb257af8..4591d8c797d0 100644 --- a/test/js/samples/instrumentation-template-if-no-block/expected.js +++ b/test/js/samples/instrumentation-template-if-no-block/expected.js @@ -60,9 +60,9 @@ function create_fragment(ctx) { function instance($$self, $$props, $$invalidate) { let x = 0; - function click_handler() { - if (true) { x += 1; $$invalidate('x', x); } - } + const click_handler = () => { + if (true) $$invalidate('x', x += 1); + }; return { x, click_handler }; } diff --git a/test/js/samples/instrumentation-template-x-equals-x/expected.js b/test/js/samples/instrumentation-template-x-equals-x/expected.js index c9868ace5002..b08130016b89 100644 --- a/test/js/samples/instrumentation-template-x-equals-x/expected.js +++ b/test/js/samples/instrumentation-template-x-equals-x/expected.js @@ -60,7 +60,7 @@ function create_fragment(ctx) { function instance($$self, $$props, $$invalidate) { let things = []; - function click_handler() { things.push(1); $$invalidate('things', things) } + const click_handler = () => { things.push(1); $$invalidate('things', things) }; return { things, click_handler }; } diff --git a/test/runtime/samples/invalidation-in-if-condition/_config.js b/test/runtime/samples/invalidation-in-if-condition/_config.js new file mode 100644 index 000000000000..60b02d993488 --- /dev/null +++ b/test/runtime/samples/invalidation-in-if-condition/_config.js @@ -0,0 +1,18 @@ +export default { + show: 1, + html: ``, + + async test({ assert, target, window }) { + const button = target.querySelector('button'); + const click = new window.MouseEvent('click'); + + await button.dispatchEvent(click); + assert.htmlEqual(target.innerHTML, ``); + + await button.dispatchEvent(click); + assert.htmlEqual(target.innerHTML, ``); + + await button.dispatchEvent(click); + assert.htmlEqual(target.innerHTML, ``); + } +}; \ No newline at end of file diff --git a/test/runtime/samples/invalidation-in-if-condition/main.svelte b/test/runtime/samples/invalidation-in-if-condition/main.svelte new file mode 100644 index 000000000000..810576695ea5 --- /dev/null +++ b/test/runtime/samples/invalidation-in-if-condition/main.svelte @@ -0,0 +1,12 @@ + + + \ No newline at end of file diff --git a/test/runtime/samples/store-resubscribe/_config.js b/test/runtime/samples/store-resubscribe/_config.js index fed8caa805a3..6bde764a558b 100644 --- a/test/runtime/samples/store-resubscribe/_config.js +++ b/test/runtime/samples/store-resubscribe/_config.js @@ -5,7 +5,7 @@ export default { `, - async test({ assert, component, target, window }) { + async test({ assert, target, window }) { const buttons = target.querySelectorAll('button'); const click = new window.MouseEvent('click');