From f124f3c08173e099d2258667627b6c4efc78e8f4 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Mon, 18 Feb 2019 11:01:41 -0500 Subject: [PATCH] unsubscribe and resubscribe when stores are reassigned - fixes #2014 --- src/compile/Component.ts | 16 +++++-- src/compile/nodes/shared/Expression.ts | 6 +-- src/compile/render-dom/index.ts | 46 +++++++++++++++---- .../render-dom/wrappers/Element/index.ts | 4 +- .../wrappers/InlineComponent/index.ts | 4 +- src/compile/render-ssr/index.ts | 2 +- .../samples/store-resubscribe/_config.js | 36 +++++++++++++++ .../samples/store-resubscribe/main.svelte | 8 ++++ 8 files changed, 101 insertions(+), 21 deletions(-) create mode 100644 test/runtime/samples/store-resubscribe/_config.js create mode 100644 test/runtime/samples/store-resubscribe/main.svelte diff --git a/src/compile/Component.ts b/src/compile/Component.ts index cbda325690..c2b8064064 100644 --- a/src/compile/Component.ts +++ b/src/compile/Component.ts @@ -750,7 +750,15 @@ export default class Component { }); } - rewrite_props(get_insert: (name: string) => string) { + invalidate(name, value = name) { + const variable = this.var_lookup.get(name); + if (variable && (variable.subscribable && variable.reassigned)) { + return `$$subscribe_${name}(), $$invalidate('${name}', ${value})`; + } + return `$$invalidate('${name}', ${value})`; + } + + rewrite_props(get_insert: (variable: Var) => string) { const component = this; const { code, instance_scope, instance_scope_map: map, componentOptions } = this; let scope = instance_scope; @@ -788,7 +796,7 @@ export default class Component { } if (variable.subscribable) { - inserts.push(get_insert(name)); + inserts.push(get_insert(variable)); } }); @@ -834,7 +842,7 @@ export default class Component { coalesced_declarations.push({ kind: node.kind, declarators: [declarator], - insert: get_insert(name) + insert: get_insert(variable) }); } else { if (current_group && current_group.kind !== node.kind) { @@ -852,7 +860,7 @@ export default class Component { current_group = null; if (variable.subscribable) { - let insert = get_insert(name); + let insert = get_insert(variable); if (next) { code.overwrite(declarator.end, next.start, `; ${insert}; ${node.kind} `); diff --git a/src/compile/nodes/shared/Expression.ts b/src/compile/nodes/shared/Expression.ts index 818066f21a..59a3dc2d9c 100644 --- a/src/compile/nodes/shared/Expression.ts +++ b/src/compile/nodes/shared/Expression.ts @@ -290,7 +290,7 @@ export default class Expression { if (dirty.length) component.has_reactive_assignments = true; - code.overwrite(node.start, node.end, dirty.map(n => `$$invalidate('${n}', ${n})`).join('; ')); + code.overwrite(node.start, node.end, dirty.map(n => component.invalidate(n)).join('; ')); } else { names.forEach(name => { if (scope.declarations.has(name)) return; @@ -356,7 +356,7 @@ export default class Expression { let body = code.slice(node.body.start, node.body.end).trim(); if (node.body.type !== 'BlockStatement') { if (pending_assignments.size > 0) { - const insert = Array.from(pending_assignments).map(name => `$$invalidate('${name}', ${name})`).join('; '); + const insert = Array.from(pending_assignments).map(name => component.invalidate(name)).join('; '); pending_assignments = new Set(); component.has_reactive_assignments = true; @@ -431,7 +431,7 @@ export default class Expression { const insert = ( (has_semi ? ' ' : '; ') + - Array.from(pending_assignments).map(name => `$$invalidate('${name}', ${name})`).join('; ') + Array.from(pending_assignments).map(name => component.invalidate(name)).join('; ') ); if (/^(Break|Continue|Return)Statement/.test(node.type)) { diff --git a/src/compile/render-dom/index.ts b/src/compile/render-dom/index.ts index 708b235815..067288c0f5 100644 --- a/src/compile/render-dom/index.ts +++ b/src/compile/render-dom/index.ts @@ -79,12 +79,13 @@ export default function dom( ${component.componentOptions.props && deindent` if (!${component.componentOptions.props}) ${component.componentOptions.props} = {}; @assign(${component.componentOptions.props}, $$props); - $$invalidate('${component.componentOptions.props_object}', ${component.componentOptions.props_object}); + ${component.invalidate(component.componentOptions.props_object)}; `} ${writable_props.map(prop => - `if ('${prop.export_name}' in $$props) $$invalidate('${prop.name}', ${prop.name} = $$props.${prop.export_name});`)} + `if ('${prop.export_name}' in $$props) ${component.invalidate(prop.name, `${prop.name} = $$props.${prop.export_name}`)};` + )} ${renderer.slots.size > 0 && - `if ('$$scope' in $$props) $$invalidate('$$scope', $$scope = $$props.$$scope);`} + `if ('$$scope' in $$props) ${component.invalidate('$$scope', `$$scope = $$props.$$scope`)};`} } ` : null; @@ -175,7 +176,7 @@ export default function dom( if (dirty.length) component.has_reactive_assignments = true; - code.overwrite(node.start, node.end, dirty.map(n => `$$invalidate('${n}', ${n})`).join('; ')); + code.overwrite(node.start, node.end, dirty.map(n => component.invalidate(n)).join('; ')); } else { names.forEach(name => { const owner = scope.findOwner(name); @@ -204,7 +205,7 @@ export default function dom( if (pending_assignments.size > 0) { if (node.type === 'ArrowFunctionExpression') { - const insert = Array.from(pending_assignments).map(name => `$$invalidate('${name}', ${name})`).join(';'); + const insert = Array.from(pending_assignments).map(name => component.invalidate(name)).join('; '); pending_assignments = new Set(); code.prependRight(node.body.start, `{ const $$result = `); @@ -214,7 +215,7 @@ export default function dom( } else if (/Statement/.test(node.type)) { - const insert = Array.from(pending_assignments).map(name => `$$invalidate('${name}', ${name})`).join('; '); + const insert = Array.from(pending_assignments).map(name => component.invalidate(name)).join('; '); if (/^(Break|Continue|Return)Statement/.test(node.type)) { if (node.argument) { @@ -240,12 +241,18 @@ export default function dom( throw new Error(`TODO this should not happen!`); } - component.rewrite_props(name => { + component.rewrite_props(({ name, reassigned }) => { const value = `$${name}`; + const callback = `$value => { ${value} = $$value; $$invalidate('${value}', ${value}) }`; + + if (reassigned) { + return `$$subscribe_${name}()`; + } + const subscribe = component.helper('subscribe'); - let insert = `${subscribe}($$self, ${name}, $$value => { ${value} = $$value; $$invalidate('${value}', ${value}) })`; + let insert = `${subscribe}($$self, ${name}, $${callback})`; if (component.compileOptions.dev) { const validate_store = component.helper('validate_store'); insert = `${validate_store}(${name}, '${name}'); ${insert}`; @@ -346,6 +353,13 @@ export default function dom( @subscribe($$self, ${name.slice(1)}, $$value => { ${name} = $$value; $$invalidate('${name}', ${name}); }); `); + const resubscribable_reactive_store_unsubscribers = reactive_stores + .filter(store => { + const variable = component.var_lookup.get(store.name.slice(1)); + return variable.reassigned; + }) + .map(({ name }) => `$$self.$$.on_destroy.push(() => $$unsubscribe_${name.slice(1)}());`); + if (has_definition) { const reactive_declarations = component.reactive_declarations.map(d => { const condition = Array.from(d.dependencies) @@ -371,12 +385,26 @@ export default function dom( return variable.injected; }); + const reactive_store_declarations = reactive_stores.map(variable => { + const $name = variable.name; + const name = $name.slice(1); + + const store = component.var_lookup.get(name); + if (store.reassigned) { + return `${$name}, $$unsubscribe_${name} = @noop, $$subscribe_${name} = () => { $$unsubscribe_${name}(); $$unsubscribe_${name} = ${name}.subscribe($$value => { ${$name} = $$value; $$invalidate('${$name}', ${$name}); }) }` + } + + return $name; + }); + builder.addBlock(deindent` function ${definition}(${args.join(', ')}) { - ${reactive_stores.length > 0 && `let ${reactive_stores.map(store => store.name).join(', ')};`} + ${reactive_store_declarations.length > 0 && `let ${reactive_store_declarations.join(', ')};`} ${reactive_store_subscriptions} + ${resubscribable_reactive_store_unsubscribers} + ${user_code} ${renderer.slots.size && `let { ${[...renderer.slots].map(name => `$$slot_${sanitize(name)}`).join(', ')}, $$scope } = $$props;`} diff --git a/src/compile/render-dom/wrappers/Element/index.ts b/src/compile/render-dom/wrappers/Element/index.ts index fea7737671..607c169732 100644 --- a/src/compile/render-dom/wrappers/Element/index.ts +++ b/src/compile/render-dom/wrappers/Element/index.ts @@ -465,7 +465,7 @@ export default class ElementWrapper extends Wrapper { this.renderer.component.partly_hoisted.push(deindent` function ${handler}(${contextual_dependencies.size > 0 ? `{ ${Array.from(contextual_dependencies).join(', ')} }` : ``}) { ${group.bindings.map(b => b.handler.mutation)} - ${Array.from(dependencies).filter(dep => dep[0] !== '$').map(dep => `$$invalidate('${dep}', ${dep});`)} + ${Array.from(dependencies).filter(dep => dep[0] !== '$').map(dep => `${this.renderer.component.invalidate(dep)};`)} } `); @@ -532,7 +532,7 @@ export default class ElementWrapper extends Wrapper { renderer.component.partly_hoisted.push(deindent` function ${name}(${['$$node', 'check'].concat(args).join(', ')}) { ${handler.snippet ? `if ($$node || (!$$node && ${handler.snippet} === check)) ` : ''}${handler.mutation} - $$invalidate('${object}', ${object}); + ${renderer.component.invalidate(object)}; } `); diff --git a/src/compile/render-dom/wrappers/InlineComponent/index.ts b/src/compile/render-dom/wrappers/InlineComponent/index.ts index a974de5687..e30984ccbc 100644 --- a/src/compile/render-dom/wrappers/InlineComponent/index.ts +++ b/src/compile/render-dom/wrappers/InlineComponent/index.ts @@ -266,7 +266,7 @@ export default class InlineComponentWrapper extends Wrapper { component.partly_hoisted.push(deindent` function ${fn}($$component) { ${lhs} = $$component; - ${object && `$$invalidate('${object}', ${object});`} + ${object && component.invalidate(object)} } `); @@ -341,7 +341,7 @@ export default class InlineComponentWrapper extends Wrapper { const body = deindent` function ${name}(${args.join(', ')}) { ${lhs} = value; - return $$invalidate('${dependencies[0]}', ${dependencies[0]}); + return ${component.invalidate(dependencies[0])} } `; diff --git a/src/compile/render-ssr/index.ts b/src/compile/render-ssr/index.ts index ab13ebc237..68e07465d7 100644 --- a/src/compile/render-ssr/index.ts +++ b/src/compile/render-ssr/index.ts @@ -40,7 +40,7 @@ export default function ssr( let user_code; if (component.javascript) { - component.rewrite_props(name => { + component.rewrite_props(({ name }) => { const value = `$${name}`; const get_store_value = component.helper('get_store_value'); diff --git a/test/runtime/samples/store-resubscribe/_config.js b/test/runtime/samples/store-resubscribe/_config.js new file mode 100644 index 0000000000..fed8caa805 --- /dev/null +++ b/test/runtime/samples/store-resubscribe/_config.js @@ -0,0 +1,36 @@ +export default { + html: ` +

0

+ + + `, + + async test({ assert, component, target, window }) { + const buttons = target.querySelectorAll('button'); + const click = new window.MouseEvent('click'); + + await buttons[0].dispatchEvent(click); + + assert.htmlEqual(target.innerHTML, ` +

1

+ + + `); + + await buttons[1].dispatchEvent(click); + + assert.htmlEqual(target.innerHTML, ` +

0

+ + + `); + + await buttons[0].dispatchEvent(click); + + assert.htmlEqual(target.innerHTML, ` +

1

+ + + `); + } +}; \ No newline at end of file diff --git a/test/runtime/samples/store-resubscribe/main.svelte b/test/runtime/samples/store-resubscribe/main.svelte new file mode 100644 index 0000000000..97239e8c4c --- /dev/null +++ b/test/runtime/samples/store-resubscribe/main.svelte @@ -0,0 +1,8 @@ + + +

{$foo}

+ + \ No newline at end of file