From 5af301abb89dbd3ec52742c8bd09f861953f0dc2 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sun, 17 Feb 2019 16:46:52 -0500 Subject: [PATCH 1/8] pedantic code generation stuff --- package.json | 1 + src/utils/__test__.ts | 39 +++++++++++++++++++++++++++++++++++++++ src/utils/deindent.ts | 2 +- 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 21885c6ad5..9bd16c1b10 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ ], "scripts": { "test": "mocha --opts mocha.opts", + "test:unit": "mocha --require sucrase/register --recursive ./**/__test__.ts", "quicktest": "mocha --opts mocha.opts", "precoverage": "c8 mocha --opts mocha.coverage.opts", "coverage": "c8 report --reporter=text-lcov > coverage.lcov && c8 report --reporter=html", diff --git a/src/utils/__test__.ts b/src/utils/__test__.ts index 9e7c9f9937..aac99992a1 100644 --- a/src/utils/__test__.ts +++ b/src/utils/__test__.ts @@ -34,6 +34,45 @@ describe('deindent', () => { assert.equal(deindented, `before\n\tline one\n\tline two\nafter`); }); + + it('removes newlines before an empty expression', () => { + const deindented = deindent` + { + some text + + ${null} + }`; + + assert.equal(deindented, `{\n\tsome text\n}`); + }); + + it('removes newlines after an empty expression', () => { + const deindented = deindent` + { + ${null} + + some text + }`; + + assert.equal(deindented, `{\n\tsome text\n}`); + }); + + it('removes newlines around empty expressions', () => { + const deindented = deindent` + { + ${null} + + some text + + ${null} + + some text + + ${null} + }`; + + assert.equal(deindented, `{\n\tsome text\n\n\tsome text\n}`); + }); }); describe('CodeBuilder', () => { diff --git a/src/utils/deindent.ts b/src/utils/deindent.ts index 665f9c3dc1..8c0d596491 100644 --- a/src/utils/deindent.ts +++ b/src/utils/deindent.ts @@ -39,7 +39,7 @@ export default function deindent( current_indentation = get_current_indentation(result); } - return result.trim().replace(/\t+$/gm, ''); + return result.trim().replace(/\t+$/gm, '').replace(/{\n\n/gm, '{\n'); } function get_current_indentation(str: string) { From 70ddabde7f1f6aeb0b15efc6b4a12b618ad7122e Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sun, 17 Feb 2019 16:47:23 -0500 Subject: [PATCH 2/8] declare $vars up top --- src/compile/render-dom/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/compile/render-dom/index.ts b/src/compile/render-dom/index.ts index 14638553a5..b25a7ba7b8 100644 --- a/src/compile/render-dom/index.ts +++ b/src/compile/render-dom/index.ts @@ -312,7 +312,6 @@ export default function dom( const reactive_store_subscriptions = reactive_stores.length > 0 && reactive_stores .map(({ name }) => deindent` - let ${name}; ${component.compileOptions.dev && `@validate_store(${name.slice(1)}, '${name.slice(1)}');`} $$self.$$.on_destroy.push(${name.slice(1)}.subscribe($$value => { ${name} = $$value; $$invalidate('${name}', ${name}); })); `) @@ -345,6 +344,8 @@ export default function dom( builder.addBlock(deindent` function ${definition}(${args.join(', ')}) { + ${reactive_stores.length > 0 && `let ${reactive_stores.map(store => store.name).join(', ')};`} + ${user_code} ${renderer.slots.size && `let { ${[...renderer.slots].map(name => `$$slot_${sanitize(name)}`).join(', ')}, $$scope } = $$props;`} From 55295a0e33ff01a9e54da0d24ecb8598e88a58d4 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sun, 17 Feb 2019 16:47:31 -0500 Subject: [PATCH 3/8] failing test for #2015 --- .../samples/store-auto-subscribe-immediate/_config.js | 9 +++++++++ .../samples/store-auto-subscribe-immediate/main.svelte | 7 +++++++ 2 files changed, 16 insertions(+) create mode 100644 test/runtime/samples/store-auto-subscribe-immediate/_config.js create mode 100644 test/runtime/samples/store-auto-subscribe-immediate/main.svelte diff --git a/test/runtime/samples/store-auto-subscribe-immediate/_config.js b/test/runtime/samples/store-auto-subscribe-immediate/_config.js new file mode 100644 index 0000000000..9cfd765cde --- /dev/null +++ b/test/runtime/samples/store-auto-subscribe-immediate/_config.js @@ -0,0 +1,9 @@ +export default { + html: ` +

42

+ `, + + async test({ assert, component }) { + assert.equal(component.initial_foo, 42); + } +}; \ No newline at end of file diff --git a/test/runtime/samples/store-auto-subscribe-immediate/main.svelte b/test/runtime/samples/store-auto-subscribe-immediate/main.svelte new file mode 100644 index 0000000000..b809bb81f6 --- /dev/null +++ b/test/runtime/samples/store-auto-subscribe-immediate/main.svelte @@ -0,0 +1,7 @@ + + +

{initial_foo}

\ No newline at end of file From 9757fbfdb88b4a84a856290cea97fd051d182a50 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sun, 17 Feb 2019 20:16:08 -0500 Subject: [PATCH 4/8] move store subscriptions into a helper. broke a bunch of stuff, bear with me --- src/compile/Component.ts | 21 ++++-- src/compile/render-dom/index.ts | 44 ++++++++++-- src/compile/render-ssr/index.ts | 72 ++++++++++++++++--- src/interfaces.ts | 1 + src/internal/utils.js | 4 ++ .../_config.js | 9 +++ .../main.svelte | 7 ++ 7 files changed, 140 insertions(+), 18 deletions(-) create mode 100644 test/runtime/samples/store-auto-subscribe-immediate-multiple-vars/_config.js create mode 100644 test/runtime/samples/store-auto-subscribe-immediate-multiple-vars/main.svelte diff --git a/src/compile/Component.ts b/src/compile/Component.ts index 09f5261c63..96a0b6bedb 100644 --- a/src/compile/Component.ts +++ b/src/compile/Component.ts @@ -183,7 +183,11 @@ export default class Component { writable: true }); - this.add_reference(name.slice(1)); + const subscribable_name = name.slice(1); + this.add_reference(subscribable_name); + + const variable = this.var_lookup.get(subscribable_name); + variable.subscribable = true; } else if (!this.ast.instance) { this.add_var({ name, @@ -213,6 +217,11 @@ export default class Component { return this.aliases.get(name); } + helper(name: string) { + this.helpers.add(name); + return this.alias(name); + } + generate(result: string) { const { compileOptions, name } = this; const { format = 'esm' } = compileOptions; @@ -641,6 +650,9 @@ export default class Component { }); this.add_reference(name.slice(1)); + + const variable = this.var_lookup.get(name.slice(1)); + variable.subscribable = true; } else { this.add_var({ name, @@ -783,8 +795,7 @@ export default class Component { // can't use the @ trick here, because we're // manipulating the underlying magic string - component.helpers.add('exclude_internal_props'); - const exclude_internal_props = component.alias('exclude_internal_props'); + const exclude_internal_props = component.helper('exclude_internal_props'); const suffix = code.original[declarator.end] === ';' ? ` = ${exclude_internal_props}($$props)` @@ -799,7 +810,9 @@ export default class Component { if (variable.export_name) { has_exports = true; - } else { + } + + if (!variable.export_name || variable.subscribable) { has_only_exports = false; } }); diff --git a/src/compile/render-dom/index.ts b/src/compile/render-dom/index.ts index b25a7ba7b8..c13e59ac9b 100644 --- a/src/compile/render-dom/index.ts +++ b/src/compile/render-dom/index.ts @@ -202,6 +202,38 @@ export default function dom( component.has_reactive_assignments = true; } + else if (node.type === 'VariableDeclarator') { + const names = extractNames(node.id); + names.forEach(name => { + const owner = scope.findOwner(name); + if (owner && owner !== component.instance_scope) return; + + const variable = component.var_lookup.get(name); + if (variable && variable.subscribable) { + const value = `$${name}`; + + const subscribe = component.helper('subscribe'); + + const index = parent.declarations.indexOf(node); + const next = parent.declarations[index + 1]; + + let insert = `${subscribe}($$self, ${name}, $$value => { ${value} = $$value; $$invalidate('${value}', ${value}) })`; + if (component.compileOptions.dev) { + const validate_store = component.helper('validate_store'); + insert = `${validate_store}(${name}, '${name}'); ${insert}`; + } + + // initialise store value here + if (next) { + code.overwrite(node.end, next.start, `; ${insert}; ${parent.kind} `); + } else { + // final (or only) declarator + code.appendLeft(node.end, `; ${insert}`); + } + } + }); + } + if (pending_assignments.size > 0) { if (node.type === 'ArrowFunctionExpression') { const insert = Array.from(pending_assignments).map(name => `$$invalidate('${name}', ${name})`).join(';'); @@ -310,12 +342,12 @@ export default function dom( : null ); - const reactive_store_subscriptions = reactive_stores.length > 0 && reactive_stores + const reactive_store_subscriptions = reactive_stores + .filter(store => component.var_lookup.get(store.name).hoistable) .map(({ name }) => deindent` ${component.compileOptions.dev && `@validate_store(${name.slice(1)}, '${name.slice(1)}');`} - $$self.$$.on_destroy.push(${name.slice(1)}.subscribe($$value => { ${name} = $$value; $$invalidate('${name}', ${name}); })); - `) - .join('\n\n'); + @subscribe($$self, ${name.slice(1)}, $$value => { ${name} = $$value; $$invalidate('${name}', ${name}); }); + `); if (has_definition) { const reactive_declarations = component.reactive_declarations.map(d => { @@ -346,6 +378,8 @@ export default function dom( function ${definition}(${args.join(', ')}) { ${reactive_stores.length > 0 && `let ${reactive_stores.map(store => store.name).join(', ')};`} + ${reactive_store_subscriptions} + ${user_code} ${renderer.slots.size && `let { ${[...renderer.slots].map(name => `$$slot_${sanitize(name)}`).join(', ')}, $$scope } = $$props;`} @@ -354,8 +388,6 @@ export default function dom( ${component.partly_hoisted.length > 0 && component.partly_hoisted.join('\n\n')} - ${reactive_store_subscriptions} - ${set && `$$self.$set = ${set};`} ${reactive_declarations.length > 0 && deindent` diff --git a/src/compile/render-ssr/index.ts b/src/compile/render-ssr/index.ts index 0383e8931c..f191c3c569 100644 --- a/src/compile/render-ssr/index.ts +++ b/src/compile/render-ssr/index.ts @@ -3,6 +3,8 @@ import Component from '../Component'; import { CompileOptions } from '../../interfaces'; import { stringify } from '../../utils/stringify'; import Renderer from './Renderer'; +import { walk } from 'estree-walker'; +import { extractNames } from '../../utils/annotateWithScopes'; export default function ssr( component: Component, @@ -22,11 +24,63 @@ export default function ssr( { code: null, map: null } : component.stylesheet.render(options.filename, true); - let user_code; + // insert store values + if (component.ast.instance) { + let scope = component.instance_scope; + let map = component.instance_scope_map; + + walk(component.ast.instance.content, { + enter: (node, parent) => { + if (map.has(node)) { + scope = map.get(node); + } + }, + + leave(node, parent) { + if (map.has(node)) { + scope = scope.parent; + } + + if (node.type === 'VariableDeclarator') { + const names = extractNames(node.id); + names.forEach(name => { + const owner = scope.findOwner(name); + if (owner && owner !== component.instance_scope) return; + + const variable = component.var_lookup.get(name); + if (variable && variable.subscribable) { + const value = `$${name}`; + + const get_store_value = component.helper('get_store_value'); + + const index = parent.declarations.indexOf(node); + const next = parent.declarations[index + 1]; + + let insert = `const ${value} = ${get_store_value}(${name});`; + if (component.compileOptions.dev) { + const validate_store = component.helper('validate_store'); + insert = `${validate_store}(${name}, '${name}'); ${insert}`; + } + + // initialise store value here + if (next) { + component.code.overwrite(node.end, next.start, `; ${insert}; ${parent.kind} `); + } else { + // final (or only) declarator + component.code.appendLeft(node.end, `; ${insert}`); + } + } + }); + } + } + }); + } // TODO remove this, just use component.vars everywhere const props = component.vars.filter(variable => !variable.module && variable.export_name && variable.export_name !== component.componentOptions.props_object); + let user_code; + if (component.javascript) { component.rewrite_props(); user_code = component.javascript; @@ -38,13 +92,15 @@ export default function ssr( } const reactive_stores = component.vars.filter(variable => variable.name[0] === '$'); - const reactive_store_values = reactive_stores.map(({ name }) => { - const assignment = `const ${name} = @get_store_value(${name.slice(1)});`; - - return component.compileOptions.dev - ? `@validate_store(${name.slice(1)}, '${name.slice(1)}'); ${assignment}` - : assignment; - }); + const reactive_store_values = reactive_stores + .filter(store => component.var_lookup.get(store.name).hoistable) + .map(({ name }) => { + const assignment = `const ${name} = @get_store_value(${name.slice(1)});`; + + return component.compileOptions.dev + ? `@validate_store(${name.slice(1)}, '${name.slice(1)}'); ${assignment}` + : assignment; + }); // TODO only do this for props with a default value const parent_bindings = component.javascript diff --git a/src/interfaces.ts b/src/interfaces.ts index 11228a0d55..4ec258c94a 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -89,4 +89,5 @@ export interface Var { internal?: boolean; // event handlers, bindings initialised?: boolean; hoistable?: boolean; + subscribable?: boolean; } \ No newline at end of file diff --git a/src/internal/utils.js b/src/internal/utils.js index 2e93efb44b..d6b1bb6380 100644 --- a/src/internal/utils.js +++ b/src/internal/utils.js @@ -47,6 +47,10 @@ export function validate_store(store, name) { } } +export function subscribe(component, store, callback) { + component.$$.on_destroy.push(store.subscribe(callback)); +} + export function create_slot(definition, ctx, fn) { if (definition) { const slot_ctx = get_slot_context(definition, ctx, fn); diff --git a/test/runtime/samples/store-auto-subscribe-immediate-multiple-vars/_config.js b/test/runtime/samples/store-auto-subscribe-immediate-multiple-vars/_config.js new file mode 100644 index 0000000000..9cfd765cde --- /dev/null +++ b/test/runtime/samples/store-auto-subscribe-immediate-multiple-vars/_config.js @@ -0,0 +1,9 @@ +export default { + html: ` +

42

+ `, + + async test({ assert, component }) { + assert.equal(component.initial_foo, 42); + } +}; \ No newline at end of file diff --git a/test/runtime/samples/store-auto-subscribe-immediate-multiple-vars/main.svelte b/test/runtime/samples/store-auto-subscribe-immediate-multiple-vars/main.svelte new file mode 100644 index 0000000000..e98b6002a1 --- /dev/null +++ b/test/runtime/samples/store-auto-subscribe-immediate-multiple-vars/main.svelte @@ -0,0 +1,7 @@ + + +

{initial_foo}

\ No newline at end of file From 7cde8d06bef5aaed07fde5c66665b702928769b0 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sun, 17 Feb 2019 20:40:34 -0500 Subject: [PATCH 5/8] pick error code --- src/compile/Component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compile/Component.ts b/src/compile/Component.ts index 96a0b6bedb..e2412d74f3 100644 --- a/src/compile/Component.ts +++ b/src/compile/Component.ts @@ -788,7 +788,7 @@ export default class Component { if (declarator.id.type !== 'Identifier') { component.error(declarator, { - code: 'todo', + code: 'destructured-prop', message: `props binding in destructured declaration is not yet supported` }); } From 534f6e542569ad1e53ecdb82f2cba2f257660e86 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sun, 17 Feb 2019 23:01:06 -0500 Subject: [PATCH 6/8] init store values in situ - fixes #2015 --- src/compile/Component.ts | 150 ++++++++++++++++++-------------- src/compile/render-dom/index.ts | 81 +++++++++-------- src/compile/render-ssr/index.ts | 99 ++++++++------------- 3 files changed, 158 insertions(+), 172 deletions(-) diff --git a/src/compile/Component.ts b/src/compile/Component.ts index e2412d74f3..cbda325690 100644 --- a/src/compile/Component.ts +++ b/src/compile/Component.ts @@ -750,7 +750,7 @@ export default class Component { }); } - rewrite_props() { + rewrite_props(get_insert: (name: string) => string) { const component = this; const { code, instance_scope, instance_scope_map: map, componentOptions } = this; let scope = instance_scope; @@ -771,73 +771,97 @@ export default class Component { if (node.type === 'VariableDeclaration') { if (node.kind === 'var' || scope === instance_scope) { - let has_exports = false; - let has_only_exports = true; + node.declarations.forEach((declarator, i) => { + const next = node.declarations[i + 1]; - node.declarations.forEach(declarator => { - extractNames(declarator.id).forEach(name => { - const variable = component.var_lookup.get(name); + if (declarator.id.type !== 'Identifier') { + const inserts = []; - if (name === componentOptions.props_object) { - if (variable.export_name) { - component.error(declarator, { - code: 'exported-options-props', - message: `Cannot export props binding` - }); - } + extractNames(declarator.id).forEach(name => { + const variable = component.var_lookup.get(name); - if (declarator.id.type !== 'Identifier') { + if (variable.export_name || name === componentOptions.props_object) { component.error(declarator, { code: 'destructured-prop', - message: `props binding in destructured declaration is not yet supported` + message: `Cannot declare props in destructured declaration` }); } - // can't use the @ trick here, because we're - // manipulating the underlying magic string - const exclude_internal_props = component.helper('exclude_internal_props'); - - const suffix = code.original[declarator.end] === ';' - ? ` = ${exclude_internal_props}($$props)` - : ` = ${exclude_internal_props}($$props);` + if (variable.subscribable) { + inserts.push(get_insert(name)); + } + }); - if (declarator.id.end === declarator.end) { - code.appendLeft(declarator.end, suffix); + if (inserts.length > 0) { + if (next) { + code.overwrite(declarator.end, next.start, `; ${inserts.join('; ')}; ${node.kind} `); } else { - code.overwrite(declarator.id.end, declarator.end, suffix); + code.appendLeft(declarator.end, `; ${inserts.join('; ')}`); } } + return; + } + + const { name } = declarator.id; + const variable = component.var_lookup.get(name); + + if (name === componentOptions.props_object) { if (variable.export_name) { - has_exports = true; + component.error(declarator, { + code: 'exported-options-props', + message: `Cannot export props binding` + }); } - if (!variable.export_name || variable.subscribable) { - has_only_exports = false; + // can't use the @ trick here, because we're + // manipulating the underlying magic string + const exclude_internal_props = component.helper('exclude_internal_props'); + + const suffix = code.original[declarator.end] === ';' + ? ` = ${exclude_internal_props}($$props)` + : ` = ${exclude_internal_props}($$props);` + + if (declarator.id.end === declarator.end) { + code.appendLeft(declarator.end, suffix); + } else { + code.overwrite(declarator.id.end, declarator.end, suffix); } - }); - }); + } + + if (variable.export_name) { + if (variable.subscribable) { + coalesced_declarations.push({ + kind: node.kind, + declarators: [declarator], + insert: get_insert(name) + }); + } else { + if (current_group && current_group.kind !== node.kind) { + current_group = null; + } + + if (!current_group) { + current_group = { kind: node.kind, declarators: [], insert: null }; + coalesced_declarations.push(current_group); + } - if (has_only_exports) { - if (current_group && current_group[current_group.length - 1].kind !== node.kind) { + current_group.declarators.push(declarator); + } + } else { current_group = null; - } - // rewrite as a group, later - if (!current_group) { - current_group = []; - coalesced_declarations.push(current_group); - } + if (variable.subscribable) { + let insert = get_insert(name); - current_group.push(node); - } else { - if (has_exports) { - // rewrite in place - throw new Error('TODO rewrite prop declaration in place'); + if (next) { + code.overwrite(declarator.end, next.start, `; ${insert}; ${node.kind} `); + } else { + code.appendLeft(declarator.end, `; ${insert}`); + } + } } - - current_group = null; - } + }); } } else { if (node.type !== 'ExportNamedDeclaration') { @@ -858,31 +882,25 @@ export default class Component { let combining = false; - group.forEach(node => { - node.declarations.forEach(declarator => { - const { id, init } = declarator; - - if (id.type === 'Identifier') { - const value = init - ? this.code.slice(id.start, init.end) - : this.code.slice(id.start, id.end); + group.declarators.forEach(declarator => { + const { id } = declarator; - if (combining) { - code.overwrite(c, id.start, ', '); - } else { - code.appendLeft(id.start, '{ '); - combining = true; - } - } else { - throw new Error('TODO destructured declarations'); - } + if (combining) { + code.overwrite(c, id.start, ', '); + } else { + code.appendLeft(id.start, '{ '); + combining = true; + } - c = declarator.end; - }); + c = declarator.end; }); if (combining) { - const suffix = code.original[c] === ';' ? ` } = $$props` : ` } = $$props;`; + const insert = group.insert + ? `; ${group.insert}` + : ''; + + const suffix = code.original[c] === ';' ? ` } = $$props${insert}` : ` } = $$props${insert};`; code.appendLeft(c, suffix); } }); diff --git a/src/compile/render-dom/index.ts b/src/compile/render-dom/index.ts index c13e59ac9b..708b235815 100644 --- a/src/compile/render-dom/index.ts +++ b/src/compile/render-dom/index.ts @@ -202,38 +202,6 @@ export default function dom( component.has_reactive_assignments = true; } - else if (node.type === 'VariableDeclarator') { - const names = extractNames(node.id); - names.forEach(name => { - const owner = scope.findOwner(name); - if (owner && owner !== component.instance_scope) return; - - const variable = component.var_lookup.get(name); - if (variable && variable.subscribable) { - const value = `$${name}`; - - const subscribe = component.helper('subscribe'); - - const index = parent.declarations.indexOf(node); - const next = parent.declarations[index + 1]; - - let insert = `${subscribe}($$self, ${name}, $$value => { ${value} = $$value; $$invalidate('${value}', ${value}) })`; - if (component.compileOptions.dev) { - const validate_store = component.helper('validate_store'); - insert = `${validate_store}(${name}, '${name}'); ${insert}`; - } - - // initialise store value here - if (next) { - code.overwrite(node.end, next.start, `; ${insert}; ${parent.kind} `); - } else { - // final (or only) declarator - code.appendLeft(node.end, `; ${insert}`); - } - } - }); - } - if (pending_assignments.size > 0) { if (node.type === 'ArrowFunctionExpression') { const insert = Array.from(pending_assignments).map(name => `$$invalidate('${name}', ${name})`).join(';'); @@ -272,7 +240,19 @@ export default function dom( throw new Error(`TODO this should not happen!`); } - component.rewrite_props(); + component.rewrite_props(name => { + const value = `$${name}`; + + const subscribe = component.helper('subscribe'); + + let insert = `${subscribe}($$self, ${name}, $$value => { ${value} = $$value; $$invalidate('${value}', ${value}) })`; + if (component.compileOptions.dev) { + const validate_store = component.helper('validate_store'); + insert = `${validate_store}(${name}, '${name}'); ${insert}`; + } + + return insert; + }); } const args = ['$$self']; @@ -333,17 +313,34 @@ export default function dom( addToSet(all_reactive_dependencies, d.dependencies); }); - const user_code = component.javascript || ( - !component.ast.instance && !component.ast.module && (filtered_props.length > 0 || component.componentOptions.props) - ? [ - component.componentOptions.props && `let ${component.componentOptions.props} = $$props;`, - filtered_props.length > 0 && `let { ${filtered_props.map(x => x.name).join(', ')} } = $$props;` - ].filter(Boolean).join('\n') - : null - ); + let user_code; + + if (component.javascript) { + user_code = component.javascript; + } else { + if (!component.ast.instance && !component.ast.module && (filtered_props.length > 0 || component.componentOptions.props)) { + const statements = []; + + if (component.componentOptions.props) statements.push(`let ${component.componentOptions.props} = $$props;`); + if (filtered_props.length > 0) statements.push(`let { ${filtered_props.map(x => x.name).join(', ')} } = $$props;`); + + reactive_stores.forEach(({ name }) => { + if (component.compileOptions.dev) { + statements.push(`${component.compileOptions.dev && `@validate_store(${name.slice(1)}, '${name.slice(1)}');`}`); + } + + statements.push(`@subscribe($$self, ${name.slice(1)}, $$value => { ${name} = $$value; $$invalidate('${name}', ${name}); });`); + }); + + user_code = statements.join('\n'); + } + } const reactive_store_subscriptions = reactive_stores - .filter(store => component.var_lookup.get(store.name).hoistable) + .filter(store => { + const variable = component.var_lookup.get(store.name.slice(1)); + return variable.hoistable; + }) .map(({ name }) => deindent` ${component.compileOptions.dev && `@validate_store(${name.slice(1)}, '${name.slice(1)}');`} @subscribe($$self, ${name.slice(1)}, $$value => { ${name} = $$value; $$invalidate('${name}', ${name}); }); diff --git a/src/compile/render-ssr/index.ts b/src/compile/render-ssr/index.ts index f191c3c569..351b7540d1 100644 --- a/src/compile/render-ssr/index.ts +++ b/src/compile/render-ssr/index.ts @@ -24,57 +24,16 @@ export default function ssr( { code: null, map: null } : component.stylesheet.render(options.filename, true); - // insert store values - if (component.ast.instance) { - let scope = component.instance_scope; - let map = component.instance_scope_map; - - walk(component.ast.instance.content, { - enter: (node, parent) => { - if (map.has(node)) { - scope = map.get(node); - } - }, - - leave(node, parent) { - if (map.has(node)) { - scope = scope.parent; - } - - if (node.type === 'VariableDeclarator') { - const names = extractNames(node.id); - names.forEach(name => { - const owner = scope.findOwner(name); - if (owner && owner !== component.instance_scope) return; - - const variable = component.var_lookup.get(name); - if (variable && variable.subscribable) { - const value = `$${name}`; - - const get_store_value = component.helper('get_store_value'); - - const index = parent.declarations.indexOf(node); - const next = parent.declarations[index + 1]; - - let insert = `const ${value} = ${get_store_value}(${name});`; - if (component.compileOptions.dev) { - const validate_store = component.helper('validate_store'); - insert = `${validate_store}(${name}, '${name}'); ${insert}`; - } - - // initialise store value here - if (next) { - component.code.overwrite(node.end, next.start, `; ${insert}; ${parent.kind} `); - } else { - // final (or only) declarator - component.code.appendLeft(node.end, `; ${insert}`); - } - } - }); - } - } + const reactive_stores = component.vars.filter(variable => variable.name[0] === '$'); + const reactive_store_values = reactive_stores + .filter(store => component.var_lookup.get(store.name).hoistable) + .map(({ name }) => { + const assignment = `const ${name} = @get_store_value(${name.slice(1)});`; + + return component.compileOptions.dev + ? `@validate_store(${name.slice(1)}, '${name.slice(1)}'); ${assignment}` + : assignment; }); - } // TODO remove this, just use component.vars everywhere const props = component.vars.filter(variable => !variable.module && variable.export_name && variable.export_name !== component.componentOptions.props_object); @@ -82,26 +41,38 @@ export default function ssr( let user_code; if (component.javascript) { - component.rewrite_props(); + component.rewrite_props(name => { + const value = `$${name}`; + + const get_store_value = component.helper('get_store_value'); + + let insert = `const ${value} = ${get_store_value}(${name})`; + if (component.compileOptions.dev) { + const validate_store = component.helper('validate_store'); + insert = `${validate_store}(${name}, '${name}'); ${insert}`; + } + + return insert; + }); + user_code = component.javascript; } else if (!component.ast.instance && !component.ast.module && (props.length > 0 || component.componentOptions.props)) { - user_code = [ - component.componentOptions.props && `let ${component.componentOptions.props} = $$props;`, - props.length > 0 && `let { ${props.map(prop => prop.export_name).join(', ')} } = $$props;` - ].filter(Boolean).join('\n'); - } + const statements = []; - const reactive_stores = component.vars.filter(variable => variable.name[0] === '$'); - const reactive_store_values = reactive_stores - .filter(store => component.var_lookup.get(store.name).hoistable) - .map(({ name }) => { - const assignment = `const ${name} = @get_store_value(${name.slice(1)});`; + if (component.componentOptions.props) statements.push(`let ${component.componentOptions.props} = $$props;`); + if (props.length > 0) statements.push(`let { ${props.map(x => x.name).join(', ')} } = $$props;`); - return component.compileOptions.dev - ? `@validate_store(${name.slice(1)}, '${name.slice(1)}'); ${assignment}` - : assignment; + reactive_stores.forEach(({ name }) => { + if (component.compileOptions.dev) { + statements.push(`${component.compileOptions.dev && `@validate_store(${name.slice(1)}, '${name.slice(1)}');`}`); + } + + statements.push(`const ${name} = @get_store_value(${name.slice(1)});`); }); + user_code = statements.join('\n'); + } + // TODO only do this for props with a default value const parent_bindings = component.javascript ? props.map(prop => { From 106ae45dc8d9d39c30627d2d7727de0c1189e1cc Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sun, 17 Feb 2019 23:06:51 -0500 Subject: [PATCH 7/8] update stores before committing in SSR mode --- src/compile/render-ssr/index.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/compile/render-ssr/index.ts b/src/compile/render-ssr/index.ts index 351b7540d1..ab13ebc237 100644 --- a/src/compile/render-ssr/index.ts +++ b/src/compile/render-ssr/index.ts @@ -26,9 +26,8 @@ export default function ssr( const reactive_stores = component.vars.filter(variable => variable.name[0] === '$'); const reactive_store_values = reactive_stores - .filter(store => component.var_lookup.get(store.name).hoistable) .map(({ name }) => { - const assignment = `const ${name} = @get_store_value(${name.slice(1)});`; + const assignment = `${name} = @get_store_value(${name.slice(1)});`; return component.compileOptions.dev ? `@validate_store(${name.slice(1)}, '${name.slice(1)}'); ${assignment}` @@ -46,7 +45,7 @@ export default function ssr( const get_store_value = component.helper('get_store_value'); - let insert = `const ${value} = ${get_store_value}(${name})`; + let insert = `${value} = ${get_store_value}(${name})`; if (component.compileOptions.dev) { const validate_store = component.helper('validate_store'); insert = `${validate_store}(${name}, '${name}'); ${insert}`; @@ -67,7 +66,7 @@ export default function ssr( statements.push(`${component.compileOptions.dev && `@validate_store(${name.slice(1)}, '${name.slice(1)}');`}`); } - statements.push(`const ${name} = @get_store_value(${name.slice(1)});`); + statements.push(`${name} = @get_store_value(${name.slice(1)});`); }); user_code = statements.join('\n'); @@ -110,6 +109,7 @@ export default function ssr( return \`${renderer.code}\`;`; const blocks = [ + reactive_stores.length > 0 && `let ${reactive_stores.map(store => store.name).join(', ')};`, user_code, parent_bindings.join('\n'), css.code && `$$result.css.add(#css);`, From f124f3c08173e099d2258667627b6c4efc78e8f4 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Mon, 18 Feb 2019 11:01:41 -0500 Subject: [PATCH 8/8] 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