From 5af301abb89dbd3ec52742c8bd09f861953f0dc2 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sun, 17 Feb 2019 16:46:52 -0500 Subject: [PATCH 01/13] 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 02/13] 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 03/13] 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 04/13] 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 05/13] 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 06/13] 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 07/13] 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 08/13] 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 From 7c9c8ce40add041544b4d6dc57d364b5c609883f Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Mon, 18 Feb 2019 11:20:23 -0500 Subject: [PATCH 09/13] use event.initCustomEvent instead of new CustomEvent - fixes #2018 --- src/internal/dom.js | 6 ++++++ src/internal/lifecycle.js | 4 +++- src/internal/transitions.js | 7 ++++--- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/internal/dom.js b/src/internal/dom.js index 32c06dc159..b6b5bae83e 100644 --- a/src/internal/dom.js +++ b/src/internal/dom.js @@ -250,3 +250,9 @@ export function addResizeListener(element, fn) { export function toggleClass(element, name, toggle) { element.classList[toggle ? 'add' : 'remove'](name); } + +export function custom_event(type, detail) { + const e = document.createEvent('CustomEvent'); + e.initCustomEvent(type, false, false, detail); + return e; +} \ No newline at end of file diff --git a/src/internal/lifecycle.js b/src/internal/lifecycle.js index 02fc2161e1..1b9123eb14 100644 --- a/src/internal/lifecycle.js +++ b/src/internal/lifecycle.js @@ -1,3 +1,5 @@ +import { custom_event } from './dom'; + export let current_component; export function set_current_component(component) { @@ -34,7 +36,7 @@ export function createEventDispatcher() { if (callbacks) { // TODO are there situations where events could be dispatched // in a server (non-DOM) environment? - const event = new window.CustomEvent(type, { detail }); + const event = custom_event(type, detail); callbacks.slice().forEach(fn => { fn.call(component, event); }); diff --git a/src/internal/transitions.js b/src/internal/transitions.js index cbbdd7da9c..6b2c5ba01e 100644 --- a/src/internal/transitions.js +++ b/src/internal/transitions.js @@ -1,6 +1,7 @@ import { identity as linear, noop, run_all } from './utils.js'; import { loop } from './loop.js'; import { create_rule, delete_rule } from './style_manager.js'; +import { custom_event } from './dom.js'; let promise; @@ -238,14 +239,14 @@ export function create_bidirectional_transition(node, fn, params, intro) { if (b) tick(0, 1); running_program = init(program, duration); - node.dispatchEvent(new window.CustomEvent(`${running_program.b ? 'intro' : 'outro'}start`)); + node.dispatchEvent(custom_event(`${running_program.b ? 'intro' : 'outro'}start`)); loop(now => { if (pending_program && now > pending_program.start) { running_program = init(pending_program, duration); pending_program = null; - node.dispatchEvent(new window.CustomEvent(`${running_program.b ? 'intro' : 'outro'}start`)); + node.dispatchEvent(custom_event(`${running_program.b ? 'intro' : 'outro'}start`)); if (css) { clear_animation(); @@ -256,7 +257,7 @@ export function create_bidirectional_transition(node, fn, params, intro) { if (running_program) { if (now >= running_program.end) { tick(t = running_program.b, 1 - t); - node.dispatchEvent(new window.CustomEvent(`${running_program.b ? 'intro' : 'outro'}end`)); + node.dispatchEvent(custom_event(`${running_program.b ? 'intro' : 'outro'}end`)); if (!pending_program) { // we're done From 248f55a574937d13e7cf8d30cb063a08ccf05873 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Mon, 18 Feb 2019 13:54:27 -0500 Subject: [PATCH 10/13] move warnings out of stats --- src/Stats.ts | 9 --------- src/compile/Component.ts | 14 ++++++++----- src/compile/css/Stylesheet.ts | 38 ++++++++--------------------------- src/compile/index.ts | 14 +++++++------ test/css/index.js | 4 ++-- test/stats/index.js | 4 ++-- test/validator/index.js | 16 +++++++-------- 7 files changed, 36 insertions(+), 63 deletions(-) diff --git a/src/Stats.ts b/src/Stats.ts index a27ff98105..dd721c7e81 100644 --- a/src/Stats.ts +++ b/src/Stats.ts @@ -1,4 +1,3 @@ -import { Warning } from './interfaces'; import Component from './compile/Component'; const now = (typeof process !== 'undefined' && process.hrtime) @@ -31,14 +30,11 @@ export default class Stats { currentChildren: Timing[]; timings: Timing[]; stack: Timing[]; - warnings: Warning[]; constructor() { this.startTime = now(); this.stack = []; this.currentChildren = this.timings = []; - - this.warnings = []; } start(label) { @@ -92,7 +88,6 @@ export default class Stats { return { timings, - warnings: this.warnings, vars: component.vars.filter(variable => !variable.global && !variable.implicit && !variable.internal).map(variable => ({ name: variable.name, export_name: variable.export_name || null, @@ -105,8 +100,4 @@ export default class Stats { })) }; } - - warn(warning) { - this.warnings.push(warning); - } } diff --git a/src/compile/Component.ts b/src/compile/Component.ts index 09f5261c63..4ff1eff92d 100644 --- a/src/compile/Component.ts +++ b/src/compile/Component.ts @@ -11,7 +11,7 @@ import Stylesheet from './css/Stylesheet'; import { test } from '../config'; import Fragment from './nodes/Fragment'; import internal_exports from './internal-exports'; -import { Node, Ast, CompileOptions, Var } from '../interfaces'; +import { Node, Ast, CompileOptions, Var, Warning } from '../interfaces'; import error from '../utils/error'; import getCodeFrame from '../utils/getCodeFrame'; import flattenReference from '../utils/flattenReference'; @@ -40,6 +40,7 @@ childKeys.ExportNamedDeclaration = ['declaration', 'specifiers']; export default class Component { stats: Stats; + warnings: Warning[]; ast: Ast; source: string; @@ -93,11 +94,13 @@ export default class Component { source: string, name: string, compileOptions: CompileOptions, - stats: Stats + stats: Stats, + warnings: Warning[] ) { this.name = name; this.stats = stats; + this.warnings = warnings; this.ast = ast; this.source = source; this.compileOptions = compileOptions; @@ -161,7 +164,7 @@ export default class Component { if (!compileOptions.customElement) this.stylesheet.reify(); - this.stylesheet.warnOnUnusedSelectors(stats); + this.stylesheet.warnOnUnusedSelectors(this); } add_var(variable: Var) { @@ -309,9 +312,10 @@ export default class Component { }; return { - ast: this.ast, js, css, + ast: this.ast, + warnings: this.warnings, stats: this.stats.render(this) }; } @@ -393,7 +397,7 @@ export default class Component { const frame = getCodeFrame(this.source, start.line - 1, start.column); - this.stats.warn({ + this.warnings.push({ code: warning.code, message: warning.message, frame, diff --git a/src/compile/css/Stylesheet.ts b/src/compile/css/Stylesheet.ts index 0a36086765..aa4db3b5de 100644 --- a/src/compile/css/Stylesheet.ts +++ b/src/compile/css/Stylesheet.ts @@ -1,14 +1,11 @@ import MagicString from 'magic-string'; import { walk } from 'estree-walker'; -import { getLocator } from 'locate-character'; import Selector from './Selector'; -import getCodeFrame from '../../utils/getCodeFrame'; import hash from '../../utils/hash'; import removeCSSPrefix from '../../utils/removeCSSPrefix'; import Element from '../nodes/Element'; -import { Node, Ast, Warning } from '../../interfaces'; +import { Node, Ast } from '../../interfaces'; import Component from '../Component'; -import Stats from '../../Stats'; const isKeyframesNode = (node: Node) => removeCSSPrefix(node.name) === 'keyframes' @@ -392,33 +389,14 @@ export default class Stylesheet { }); } - warnOnUnusedSelectors(stats: Stats) { - let locator; - - const handler = (selector: Selector) => { - const pos = selector.node.start; - - if (!locator) locator = getLocator(this.source, { offsetLine: 1 }); - const start = locator(pos); - const end = locator(selector.node.end); - - const frame = getCodeFrame(this.source, start.line - 1, start.column); - const message = `Unused CSS selector`; - - stats.warn({ - code: `css-unused-selector`, - message, - frame, - start, - end, - pos, - filename: this.filename, - toString: () => `${message} (${start.line}:${start.column})\n${frame}`, - }); - }; - + warnOnUnusedSelectors(component: Component) { this.children.forEach(child => { - child.warnOnUnusedSelector(handler); + child.warnOnUnusedSelector((selector: Selector) => { + component.warn(selector.node, { + code: `css-unused-selector`, + message: `Unused CSS selector` + }); + }); }); } } diff --git a/src/compile/index.ts b/src/compile/index.ts index 404d933327..2efbaf53a2 100644 --- a/src/compile/index.ts +++ b/src/compile/index.ts @@ -3,7 +3,7 @@ import Stats from '../Stats'; import parse from '../parse/index'; import renderDOM from './render-dom/index'; import renderSSR from './render-ssr/index'; -import { CompileOptions, Ast } from '../interfaces'; +import { CompileOptions, Ast, Warning } from '../interfaces'; import Component from './Component'; import fuzzymatch from '../utils/fuzzymatch'; @@ -24,7 +24,7 @@ const valid_options = [ 'preserveComments' ]; -function validate_options(options: CompileOptions, stats: Stats) { +function validate_options(options: CompileOptions, warnings: Warning[]) { const { name, filename } = options; Object.keys(options).forEach(key => { @@ -43,7 +43,7 @@ function validate_options(options: CompileOptions, stats: Stats) { if (name && /^[a-z]/.test(name)) { const message = `options.name should be capitalised`; - stats.warn({ + warnings.push({ code: `options-lowercase-name`, message, filename, @@ -74,10 +74,11 @@ export default function compile(source: string, options: CompileOptions = {}) { options = assign({ generate: 'dom', dev: false }, options); const stats = new Stats(); + const warnings = []; let ast: Ast; - validate_options(options, stats); + validate_options(options, warnings); stats.start('parse'); ast = parse(source, options); @@ -89,12 +90,13 @@ export default function compile(source: string, options: CompileOptions = {}) { source, options.name || get_name(options.filename) || 'SvelteComponent', options, - stats + stats, + warnings ); stats.stop('create component'); if (options.generate === false) { - return { ast, stats: stats.render(component), js: null, css: null }; + return { ast, warnings, stats: stats.render(component), js: null, css: null }; } const js = options.generate === 'ssr' diff --git a/test/css/index.js b/test/css/index.js index 684c310526..be2a10bef1 100644 --- a/test/css/index.js +++ b/test/css/index.js @@ -68,8 +68,8 @@ describe('css', () => { assert.equal(dom.css.code, ssr.css.code); - const dom_warnings = dom.stats.warnings.map(normalize_warning); - const ssr_warnings = ssr.stats.warnings.map(normalize_warning); + const dom_warnings = dom.warnings.map(normalize_warning); + const ssr_warnings = ssr.warnings.map(normalize_warning); assert.deepEqual(dom_warnings, ssr_warnings); assert.deepEqual(dom_warnings.map(normalize_warning), expected_warnings); diff --git a/test/stats/index.js b/test/stats/index.js index fe4c2ac55b..e5a3c53eca 100644 --- a/test/stats/index.js +++ b/test/stats/index.js @@ -32,8 +32,8 @@ describe('stats', () => { result = svelte.compile(input, config.options); config.test(assert, result.stats); - if (result.stats.warnings.length || expectedWarnings.length) { - // TODO check warnings are added to stats.warnings + if (result.warnings.length || expectedWarnings.length) { + // TODO check warnings are added to warnings } } catch (e) { error = e; diff --git a/test/validator/index.js b/test/validator/index.js index f68cb8831d..b26b087bb3 100644 --- a/test/validator/index.js +++ b/test/validator/index.js @@ -24,21 +24,19 @@ describe("validate", () => { let error; try { - const { stats } = svelte.compile(input, { + let { warnings } = svelte.compile(input, { dev: config.dev, legacy: config.legacy, generate: false }); - const warnings = stats.warnings.map(w => ({ + assert.deepEqual(warnings.map(w => ({ code: w.code, message: w.message, pos: w.pos, start: w.start, end: w.end - })); - - assert.deepEqual(warnings, expected_warnings); + })), expected_warnings); } catch (e) { error = e; } @@ -78,12 +76,12 @@ describe("validate", () => { }); it("warns if options.name is not capitalised", () => { - const { stats } = svelte.compile("
", { + const { warnings } = svelte.compile("
", { name: "lowercase", generate: false }); - assert.deepEqual(stats.warnings.map(w => ({ + assert.deepEqual(warnings.map(w => ({ code: w.code, message: w.message })), [{ @@ -93,11 +91,11 @@ describe("validate", () => { }); it("does not warn if options.name begins with non-alphabetic character", () => { - const { stats } = svelte.compile("
", { + const { warnings } = svelte.compile("
", { name: "_", generate: false }); - assert.deepEqual(stats.warnings, []); + assert.deepEqual(warnings, []); }); }); From fd39e9505b5ecc423154907198c06d4a77fe17f2 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Mon, 18 Feb 2019 13:56:05 -0500 Subject: [PATCH 11/13] remove unnecessary code --- test/stats/index.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/stats/index.js b/test/stats/index.js index e5a3c53eca..aafc58c926 100644 --- a/test/stats/index.js +++ b/test/stats/index.js @@ -19,8 +19,6 @@ describe('stats', () => { const filename = `test/stats/samples/${dir}/input.svelte`; const input = fs.readFileSync(filename, 'utf-8').replace(/\s+$/, ''); - const expectedWarnings = - tryToLoadJson(`test/stats/samples/${dir}/warnings.json`) || []; const expectedError = tryToLoadJson( `test/stats/samples/${dir}/error.json` ); @@ -31,10 +29,6 @@ describe('stats', () => { try { result = svelte.compile(input, config.options); config.test(assert, result.stats); - - if (result.warnings.length || expectedWarnings.length) { - // TODO check warnings are added to warnings - } } catch (e) { error = e; } From b2f371a3d1f5fb3ab47bda93ae9c3e4046cad917 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Mon, 18 Feb 2019 14:19:53 -0500 Subject: [PATCH 12/13] move vars out of stats --- src/Stats.ts | 34 +--- src/compile/Component.ts | 161 ++++++++++-------- src/compile/index.ts | 12 +- .../samples/duplicate-globals/_config.js | 5 - test/stats/samples/implicit-action/_config.js | 5 - test/stats/samples/implicit/_config.js | 5 - .../samples/template-references/_config.js | 5 - test/stats/samples/undeclared/_config.js | 5 - test/vars/index.js | 60 +++++++ .../vars/samples/duplicate-globals/_config.js | 5 + .../samples/duplicate-globals/input.svelte | 0 .../duplicate-non-hoistable/_config.js | 4 +- .../duplicate-non-hoistable/input.svelte | 0 .../samples/duplicate-vars/_config.js | 4 +- .../samples/duplicate-vars/input.svelte | 0 test/vars/samples/implicit-action/_config.js | 5 + .../samples/implicit-action/input.svelte | 0 .../samples/implicit-reactive/_config.js | 4 +- .../samples/implicit-reactive/input.svelte | 0 test/vars/samples/implicit/_config.js | 5 + .../samples/implicit/input.svelte | 0 .../samples/imports/_config.js | 4 +- .../samples/imports/input.svelte | 0 .../_config.js | 4 +- .../input.svelte | 0 .../samples/mutated-vs-reassigned}/_config.js | 4 +- .../mutated-vs-reassigned/input.svelte | 0 test/{stats => vars}/samples/props/_config.js | 4 +- .../samples/props/input.svelte | 0 .../samples/store-referenced/_config.js | 4 +- .../samples/store-referenced/input.svelte | 0 .../samples/store-unreferenced/_config.js | 4 +- .../samples/store-unreferenced/input.svelte | 0 .../samples/template-references/_config.js | 5 + .../samples/template-references/input.svelte | 0 test/vars/samples/undeclared/_config.js | 5 + .../samples/undeclared/input.svelte | 0 37 files changed, 198 insertions(+), 155 deletions(-) delete mode 100644 test/stats/samples/duplicate-globals/_config.js delete mode 100644 test/stats/samples/implicit-action/_config.js delete mode 100644 test/stats/samples/implicit/_config.js delete mode 100644 test/stats/samples/template-references/_config.js delete mode 100644 test/stats/samples/undeclared/_config.js create mode 100644 test/vars/index.js create mode 100644 test/vars/samples/duplicate-globals/_config.js rename test/{stats => vars}/samples/duplicate-globals/input.svelte (100%) rename test/{stats => vars}/samples/duplicate-non-hoistable/_config.js (78%) rename test/{stats => vars}/samples/duplicate-non-hoistable/input.svelte (100%) rename test/{stats => vars}/samples/duplicate-vars/_config.js (87%) rename test/{stats => vars}/samples/duplicate-vars/input.svelte (100%) create mode 100644 test/vars/samples/implicit-action/_config.js rename test/{stats => vars}/samples/implicit-action/input.svelte (100%) rename test/{stats => vars}/samples/implicit-reactive/_config.js (86%) rename test/{stats => vars}/samples/implicit-reactive/input.svelte (100%) create mode 100644 test/vars/samples/implicit/_config.js rename test/{stats => vars}/samples/implicit/input.svelte (100%) rename test/{stats => vars}/samples/imports/_config.js (90%) rename test/{stats => vars}/samples/imports/input.svelte (100%) rename test/{stats/samples/mutated-vs-reassigned => vars/samples/mutated-vs-reassigned-bindings}/_config.js (86%) rename test/{stats => vars}/samples/mutated-vs-reassigned-bindings/input.svelte (100%) rename test/{stats/samples/mutated-vs-reassigned-bindings => vars/samples/mutated-vs-reassigned}/_config.js (86%) rename test/{stats => vars}/samples/mutated-vs-reassigned/input.svelte (100%) rename test/{stats => vars}/samples/props/_config.js (92%) rename test/{stats => vars}/samples/props/input.svelte (100%) rename test/{stats => vars}/samples/store-referenced/_config.js (87%) rename test/{stats => vars}/samples/store-referenced/input.svelte (100%) rename test/{stats => vars}/samples/store-unreferenced/_config.js (87%) rename test/{stats => vars}/samples/store-unreferenced/input.svelte (100%) create mode 100644 test/vars/samples/template-references/_config.js rename test/{stats => vars}/samples/template-references/input.svelte (100%) create mode 100644 test/vars/samples/undeclared/_config.js rename test/{stats => vars}/samples/undeclared/input.svelte (100%) diff --git a/src/Stats.ts b/src/Stats.ts index dd721c7e81..87c6c2173a 100644 --- a/src/Stats.ts +++ b/src/Stats.ts @@ -1,5 +1,3 @@ -import Component from './compile/Component'; - const now = (typeof process !== 'undefined' && process.hrtime) ? () => { const t = process.hrtime(); @@ -63,41 +61,13 @@ export default class Stats { this.currentChildren = this.currentTiming ? this.currentTiming.children : this.timings; } - render(component: Component) { + render() { const timings = Object.assign({ total: now() - this.startTime }, collapseTimings(this.timings)); - // TODO would be good to have this info even - // if options.generate is false - const imports = component && component.imports.map(node => { - return { - source: node.source.value, - specifiers: node.specifiers.map(specifier => { - return { - name: ( - specifier.type === 'ImportDefaultSpecifier' ? 'default' : - specifier.type === 'ImportNamespaceSpecifier' ? '*' : - specifier.imported.name - ), - as: specifier.local.name - }; - }) - } - }); - return { - timings, - vars: component.vars.filter(variable => !variable.global && !variable.implicit && !variable.internal).map(variable => ({ - name: variable.name, - export_name: variable.export_name || null, - injected: variable.injected || false, - module: variable.module || false, - mutated: variable.mutated || false, - reassigned: variable.reassigned || false, - referenced: variable.referenced || false, - writable: variable.writable || false - })) + timings }; } } diff --git a/src/compile/Component.ts b/src/compile/Component.ts index 4ff1eff92d..0a718966a2 100644 --- a/src/compile/Component.ts +++ b/src/compile/Component.ts @@ -217,106 +217,121 @@ export default class Component { } generate(result: string) { - const { compileOptions, name } = this; - const { format = 'esm' } = compileOptions; + let js = null; + let css = null; - const banner = `/* ${this.file ? `${this.file} ` : ``}generated by Svelte v${"__VERSION__"} */`; + if (result) { + const { compileOptions, name } = this; + const { format = 'esm' } = compileOptions; - // TODO use same regex for both - result = result.replace(compileOptions.generate === 'ssr' ? /(@+|#+)(\w*(?:-\w*)?)/g : /(@+)(\w*(?:-\w*)?)/g, (match: string, sigil: string, name: string) => { - if (sigil === '@') { - if (internal_exports.has(name)) { - if (compileOptions.dev && internal_exports.has(`${name}Dev`)) name = `${name}Dev`; - this.helpers.add(name); - } + const banner = `/* ${this.file ? `${this.file} ` : ``}generated by Svelte v${"__VERSION__"} */`; - return this.alias(name); - } + // TODO use same regex for both + result = result.replace(compileOptions.generate === 'ssr' ? /(@+|#+)(\w*(?:-\w*)?)/g : /(@+)(\w*(?:-\w*)?)/g, (match: string, sigil: string, name: string) => { + if (sigil === '@') { + if (internal_exports.has(name)) { + if (compileOptions.dev && internal_exports.has(`${name}Dev`)) name = `${name}Dev`; + this.helpers.add(name); + } - return sigil.slice(1) + name; - }); + return this.alias(name); + } - const importedHelpers = Array.from(this.helpers) - .sort() - .map(name => { - const alias = this.alias(name); - return { name, alias }; + return sigil.slice(1) + name; }); - const module = wrapModule( - result, - format, - name, - compileOptions, - banner, - compileOptions.sveltePath, - importedHelpers, - this.imports, - this.vars.filter(variable => variable.module && variable.export_name).map(variable => ({ - name: variable.name, - as: variable.export_name - })), - this.source - ); + const importedHelpers = Array.from(this.helpers) + .sort() + .map(name => { + const alias = this.alias(name); + return { name, alias }; + }); - const parts = module.split('✂]'); - const finalChunk = parts.pop(); + const module = wrapModule( + result, + format, + name, + compileOptions, + banner, + compileOptions.sveltePath, + importedHelpers, + this.imports, + this.vars.filter(variable => variable.module && variable.export_name).map(variable => ({ + name: variable.name, + as: variable.export_name + })), + this.source + ); - const compiled = new Bundle({ separator: '' }); + const parts = module.split('✂]'); + const finalChunk = parts.pop(); - function addString(str: string) { - compiled.addSource({ - content: new MagicString(str), - }); - } + const compiled = new Bundle({ separator: '' }); - const { filename } = compileOptions; + function addString(str: string) { + compiled.addSource({ + content: new MagicString(str), + }); + } - // special case — the source file doesn't actually get used anywhere. we need - // to add an empty file to populate map.sources and map.sourcesContent - if (!parts.length) { - compiled.addSource({ - filename, - content: new MagicString(this.source).remove(0, this.source.length), - }); - } + const { filename } = compileOptions; + + // special case — the source file doesn't actually get used anywhere. we need + // to add an empty file to populate map.sources and map.sourcesContent + if (!parts.length) { + compiled.addSource({ + filename, + content: new MagicString(this.source).remove(0, this.source.length), + }); + } - const pattern = /\[✂(\d+)-(\d+)$/; + const pattern = /\[✂(\d+)-(\d+)$/; - parts.forEach((str: string) => { - const chunk = str.replace(pattern, ''); - if (chunk) addString(chunk); + parts.forEach((str: string) => { + const chunk = str.replace(pattern, ''); + if (chunk) addString(chunk); - const match = pattern.exec(str); + const match = pattern.exec(str); - const snippet = this.code.snip(+match[1], +match[2]); + const snippet = this.code.snip(+match[1], +match[2]); - compiled.addSource({ - filename, - content: snippet, + compiled.addSource({ + filename, + content: snippet, + }); }); - }); - addString(finalChunk); + addString(finalChunk); - const css = compileOptions.customElement ? - { code: null, map: null } : - this.stylesheet.render(compileOptions.cssOutputFilename, true); + css = compileOptions.customElement ? + { code: null, map: null } : + this.stylesheet.render(compileOptions.cssOutputFilename, true); - const js = { - code: compiled.toString(), - map: compiled.generateMap({ - includeContent: true, - file: compileOptions.outputFilename, - }) - }; + js = { + code: compiled.toString(), + map: compiled.generateMap({ + includeContent: true, + file: compileOptions.outputFilename, + }) + }; + } return { js, css, ast: this.ast, warnings: this.warnings, - stats: this.stats.render(this) + vars: this.vars.filter(v => !v.global && !v.implicit && !v.internal).map(v => ({ + name: v.name, + export_name: v.export_name || null, + injected: v.injected || false, + module: v.module || false, + mutated: v.mutated || false, + reassigned: v.reassigned || false, + referenced: v.referenced || false, + writable: v.writable || false + })), + stats: this.stats.render() }; } diff --git a/src/compile/index.ts b/src/compile/index.ts index 2efbaf53a2..704d5bd706 100644 --- a/src/compile/index.ts +++ b/src/compile/index.ts @@ -95,13 +95,11 @@ export default function compile(source: string, options: CompileOptions = {}) { ); stats.stop('create component'); - if (options.generate === false) { - return { ast, warnings, stats: stats.render(component), js: null, css: null }; - } - - const js = options.generate === 'ssr' - ? renderSSR(component, options) - : renderDOM(component, options); + const js = options.generate === false + ? null + : options.generate === 'ssr' + ? renderSSR(component, options) + : renderDOM(component, options); return component.generate(js); } \ No newline at end of file diff --git a/test/stats/samples/duplicate-globals/_config.js b/test/stats/samples/duplicate-globals/_config.js deleted file mode 100644 index 71c255d54b..0000000000 --- a/test/stats/samples/duplicate-globals/_config.js +++ /dev/null @@ -1,5 +0,0 @@ -export default { - test(assert, stats) { - assert.deepEqual(stats.vars, []); - }, -}; diff --git a/test/stats/samples/implicit-action/_config.js b/test/stats/samples/implicit-action/_config.js deleted file mode 100644 index 71c255d54b..0000000000 --- a/test/stats/samples/implicit-action/_config.js +++ /dev/null @@ -1,5 +0,0 @@ -export default { - test(assert, stats) { - assert.deepEqual(stats.vars, []); - }, -}; diff --git a/test/stats/samples/implicit/_config.js b/test/stats/samples/implicit/_config.js deleted file mode 100644 index 71c255d54b..0000000000 --- a/test/stats/samples/implicit/_config.js +++ /dev/null @@ -1,5 +0,0 @@ -export default { - test(assert, stats) { - assert.deepEqual(stats.vars, []); - }, -}; diff --git a/test/stats/samples/template-references/_config.js b/test/stats/samples/template-references/_config.js deleted file mode 100644 index 71c255d54b..0000000000 --- a/test/stats/samples/template-references/_config.js +++ /dev/null @@ -1,5 +0,0 @@ -export default { - test(assert, stats) { - assert.deepEqual(stats.vars, []); - }, -}; diff --git a/test/stats/samples/undeclared/_config.js b/test/stats/samples/undeclared/_config.js deleted file mode 100644 index 71c255d54b..0000000000 --- a/test/stats/samples/undeclared/_config.js +++ /dev/null @@ -1,5 +0,0 @@ -export default { - test(assert, stats) { - assert.deepEqual(stats.vars, []); - }, -}; diff --git a/test/vars/index.js b/test/vars/index.js new file mode 100644 index 0000000000..d8da8e4778 --- /dev/null +++ b/test/vars/index.js @@ -0,0 +1,60 @@ +import * as fs from 'fs'; +import * as assert from 'assert'; +import { svelte, loadConfig, tryToLoadJson } from '../helpers.js'; + +describe('vars', () => { + fs.readdirSync('test/vars/samples').forEach(dir => { + if (dir[0] === '.') return; + + // add .solo to a sample directory name to only run that test + const solo = /\.solo/.test(dir); + const skip = /\.skip/.test(dir); + + if (solo && process.env.CI) { + throw new Error('Forgot to remove `solo: true` from test'); + } + + (solo ? it.only : skip ? it.skip : it)(dir, () => { + const config = loadConfig(`./vars/samples/${dir}/_config.js`); + const filename = `test/vars/samples/${dir}/input.svelte`; + const input = fs.readFileSync(filename, 'utf-8').replace(/\s+$/, ''); + + const expectedError = tryToLoadJson( + `test/vars/samples/${dir}/error.json` + ); + + let result; + let error; + + try { + result = svelte.compile(input, config.options); + config.test(assert, result.vars); + } catch (e) { + error = e; + } + + if (error || expectedError) { + if (error && !expectedError) { + throw error; + } + + if (expectedError && !error) { + throw new Error(`Expected an error: ${expectedError.message}`); + } + + assert.equal(error.message, expectedError.message); + assert.deepEqual(error.start, expectedError.start); + assert.deepEqual(error.end, expectedError.end); + assert.equal(error.pos, expectedError.pos); + } + }); + }); + + it('returns a vars object when options.generate is false', () => { + const { vars } = svelte.compile('', { + generate: false + }); + + assert.ok(Array.isArray(vars)); + }); +}); diff --git a/test/vars/samples/duplicate-globals/_config.js b/test/vars/samples/duplicate-globals/_config.js new file mode 100644 index 0000000000..2b84e83f12 --- /dev/null +++ b/test/vars/samples/duplicate-globals/_config.js @@ -0,0 +1,5 @@ +export default { + test(assert, vars) { + assert.deepEqual(vars, []); + }, +}; diff --git a/test/stats/samples/duplicate-globals/input.svelte b/test/vars/samples/duplicate-globals/input.svelte similarity index 100% rename from test/stats/samples/duplicate-globals/input.svelte rename to test/vars/samples/duplicate-globals/input.svelte diff --git a/test/stats/samples/duplicate-non-hoistable/_config.js b/test/vars/samples/duplicate-non-hoistable/_config.js similarity index 78% rename from test/stats/samples/duplicate-non-hoistable/_config.js rename to test/vars/samples/duplicate-non-hoistable/_config.js index 4b598412a9..bb7f52d4db 100644 --- a/test/stats/samples/duplicate-non-hoistable/_config.js +++ b/test/vars/samples/duplicate-non-hoistable/_config.js @@ -1,6 +1,6 @@ export default { - test(assert, stats) { - assert.deepEqual(stats.vars, [ + test(assert, vars) { + assert.deepEqual(vars, [ { name: 'console', injected: false, diff --git a/test/stats/samples/duplicate-non-hoistable/input.svelte b/test/vars/samples/duplicate-non-hoistable/input.svelte similarity index 100% rename from test/stats/samples/duplicate-non-hoistable/input.svelte rename to test/vars/samples/duplicate-non-hoistable/input.svelte diff --git a/test/stats/samples/duplicate-vars/_config.js b/test/vars/samples/duplicate-vars/_config.js similarity index 87% rename from test/stats/samples/duplicate-vars/_config.js rename to test/vars/samples/duplicate-vars/_config.js index f4f71fe021..276ae130a1 100644 --- a/test/stats/samples/duplicate-vars/_config.js +++ b/test/vars/samples/duplicate-vars/_config.js @@ -1,6 +1,6 @@ export default { - test(assert, stats) { - assert.deepEqual(stats.vars, [ + test(assert, vars) { + assert.deepEqual(vars, [ { name: 'foo', injected: false, diff --git a/test/stats/samples/duplicate-vars/input.svelte b/test/vars/samples/duplicate-vars/input.svelte similarity index 100% rename from test/stats/samples/duplicate-vars/input.svelte rename to test/vars/samples/duplicate-vars/input.svelte diff --git a/test/vars/samples/implicit-action/_config.js b/test/vars/samples/implicit-action/_config.js new file mode 100644 index 0000000000..2b84e83f12 --- /dev/null +++ b/test/vars/samples/implicit-action/_config.js @@ -0,0 +1,5 @@ +export default { + test(assert, vars) { + assert.deepEqual(vars, []); + }, +}; diff --git a/test/stats/samples/implicit-action/input.svelte b/test/vars/samples/implicit-action/input.svelte similarity index 100% rename from test/stats/samples/implicit-action/input.svelte rename to test/vars/samples/implicit-action/input.svelte diff --git a/test/stats/samples/implicit-reactive/_config.js b/test/vars/samples/implicit-reactive/_config.js similarity index 86% rename from test/stats/samples/implicit-reactive/_config.js rename to test/vars/samples/implicit-reactive/_config.js index 0137632d2c..fd67e8b249 100644 --- a/test/stats/samples/implicit-reactive/_config.js +++ b/test/vars/samples/implicit-reactive/_config.js @@ -1,6 +1,6 @@ export default { - test(assert, stats) { - assert.deepEqual(stats.vars, [ + test(assert, vars) { + assert.deepEqual(vars, [ { name: 'a', injected: false, diff --git a/test/stats/samples/implicit-reactive/input.svelte b/test/vars/samples/implicit-reactive/input.svelte similarity index 100% rename from test/stats/samples/implicit-reactive/input.svelte rename to test/vars/samples/implicit-reactive/input.svelte diff --git a/test/vars/samples/implicit/_config.js b/test/vars/samples/implicit/_config.js new file mode 100644 index 0000000000..2b84e83f12 --- /dev/null +++ b/test/vars/samples/implicit/_config.js @@ -0,0 +1,5 @@ +export default { + test(assert, vars) { + assert.deepEqual(vars, []); + }, +}; diff --git a/test/stats/samples/implicit/input.svelte b/test/vars/samples/implicit/input.svelte similarity index 100% rename from test/stats/samples/implicit/input.svelte rename to test/vars/samples/implicit/input.svelte diff --git a/test/stats/samples/imports/_config.js b/test/vars/samples/imports/_config.js similarity index 90% rename from test/stats/samples/imports/_config.js rename to test/vars/samples/imports/_config.js index 287c5837ad..90f2468b54 100644 --- a/test/stats/samples/imports/_config.js +++ b/test/vars/samples/imports/_config.js @@ -1,6 +1,6 @@ export default { - test(assert, stats) { - assert.deepEqual(stats.vars, [ + test(assert, vars) { + assert.deepEqual(vars, [ { name: 'x', export_name: null, diff --git a/test/stats/samples/imports/input.svelte b/test/vars/samples/imports/input.svelte similarity index 100% rename from test/stats/samples/imports/input.svelte rename to test/vars/samples/imports/input.svelte diff --git a/test/stats/samples/mutated-vs-reassigned/_config.js b/test/vars/samples/mutated-vs-reassigned-bindings/_config.js similarity index 86% rename from test/stats/samples/mutated-vs-reassigned/_config.js rename to test/vars/samples/mutated-vs-reassigned-bindings/_config.js index a1d027cbb5..21fc14d820 100644 --- a/test/stats/samples/mutated-vs-reassigned/_config.js +++ b/test/vars/samples/mutated-vs-reassigned-bindings/_config.js @@ -1,6 +1,6 @@ export default { - test(assert, stats) { - assert.deepEqual(stats.vars, [ + test(assert, vars) { + assert.deepEqual(vars, [ { name: 'count', export_name: null, diff --git a/test/stats/samples/mutated-vs-reassigned-bindings/input.svelte b/test/vars/samples/mutated-vs-reassigned-bindings/input.svelte similarity index 100% rename from test/stats/samples/mutated-vs-reassigned-bindings/input.svelte rename to test/vars/samples/mutated-vs-reassigned-bindings/input.svelte diff --git a/test/stats/samples/mutated-vs-reassigned-bindings/_config.js b/test/vars/samples/mutated-vs-reassigned/_config.js similarity index 86% rename from test/stats/samples/mutated-vs-reassigned-bindings/_config.js rename to test/vars/samples/mutated-vs-reassigned/_config.js index a1d027cbb5..21fc14d820 100644 --- a/test/stats/samples/mutated-vs-reassigned-bindings/_config.js +++ b/test/vars/samples/mutated-vs-reassigned/_config.js @@ -1,6 +1,6 @@ export default { - test(assert, stats) { - assert.deepEqual(stats.vars, [ + test(assert, vars) { + assert.deepEqual(vars, [ { name: 'count', export_name: null, diff --git a/test/stats/samples/mutated-vs-reassigned/input.svelte b/test/vars/samples/mutated-vs-reassigned/input.svelte similarity index 100% rename from test/stats/samples/mutated-vs-reassigned/input.svelte rename to test/vars/samples/mutated-vs-reassigned/input.svelte diff --git a/test/stats/samples/props/_config.js b/test/vars/samples/props/_config.js similarity index 92% rename from test/stats/samples/props/_config.js rename to test/vars/samples/props/_config.js index cbec831f61..0c5c6f7e2a 100644 --- a/test/stats/samples/props/_config.js +++ b/test/vars/samples/props/_config.js @@ -1,6 +1,6 @@ export default { - test(assert, stats) { - assert.deepEqual(stats.vars, [ + test(assert, vars) { + assert.deepEqual(vars, [ { name: 'name', export_name: 'name', diff --git a/test/stats/samples/props/input.svelte b/test/vars/samples/props/input.svelte similarity index 100% rename from test/stats/samples/props/input.svelte rename to test/vars/samples/props/input.svelte diff --git a/test/stats/samples/store-referenced/_config.js b/test/vars/samples/store-referenced/_config.js similarity index 87% rename from test/stats/samples/store-referenced/_config.js rename to test/vars/samples/store-referenced/_config.js index ecb958df1e..632a9e5b3f 100644 --- a/test/stats/samples/store-referenced/_config.js +++ b/test/vars/samples/store-referenced/_config.js @@ -1,6 +1,6 @@ export default { - test(assert, stats) { - assert.deepEqual(stats.vars, [ + test(assert, vars) { + assert.deepEqual(vars, [ { name: 'foo', export_name: null, diff --git a/test/stats/samples/store-referenced/input.svelte b/test/vars/samples/store-referenced/input.svelte similarity index 100% rename from test/stats/samples/store-referenced/input.svelte rename to test/vars/samples/store-referenced/input.svelte diff --git a/test/stats/samples/store-unreferenced/_config.js b/test/vars/samples/store-unreferenced/_config.js similarity index 87% rename from test/stats/samples/store-unreferenced/_config.js rename to test/vars/samples/store-unreferenced/_config.js index 9ea5784f92..86e467c859 100644 --- a/test/stats/samples/store-unreferenced/_config.js +++ b/test/vars/samples/store-unreferenced/_config.js @@ -1,6 +1,6 @@ export default { - test(assert, stats) { - assert.deepEqual(stats.vars, [ + test(assert, vars) { + assert.deepEqual(vars, [ { name: 'foo', export_name: null, diff --git a/test/stats/samples/store-unreferenced/input.svelte b/test/vars/samples/store-unreferenced/input.svelte similarity index 100% rename from test/stats/samples/store-unreferenced/input.svelte rename to test/vars/samples/store-unreferenced/input.svelte diff --git a/test/vars/samples/template-references/_config.js b/test/vars/samples/template-references/_config.js new file mode 100644 index 0000000000..2b84e83f12 --- /dev/null +++ b/test/vars/samples/template-references/_config.js @@ -0,0 +1,5 @@ +export default { + test(assert, vars) { + assert.deepEqual(vars, []); + }, +}; diff --git a/test/stats/samples/template-references/input.svelte b/test/vars/samples/template-references/input.svelte similarity index 100% rename from test/stats/samples/template-references/input.svelte rename to test/vars/samples/template-references/input.svelte diff --git a/test/vars/samples/undeclared/_config.js b/test/vars/samples/undeclared/_config.js new file mode 100644 index 0000000000..2b84e83f12 --- /dev/null +++ b/test/vars/samples/undeclared/_config.js @@ -0,0 +1,5 @@ +export default { + test(assert, vars) { + assert.deepEqual(vars, []); + }, +}; diff --git a/test/stats/samples/undeclared/input.svelte b/test/vars/samples/undeclared/input.svelte similarity index 100% rename from test/stats/samples/undeclared/input.svelte rename to test/vars/samples/undeclared/input.svelte From 6f394e521aa5ab2f071d3579ca5015d9cb4967a0 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Mon, 18 Feb 2019 21:00:15 -0500 Subject: [PATCH 13/13] error on contextual stores, for now (#2016) --- src/compile/nodes/shared/Expression.ts | 8 ++ .../samples/store-contextual/_config.js | 76 +++++++++++++++++++ .../samples/store-contextual/main.svelte | 6 ++ 3 files changed, 90 insertions(+) create mode 100644 test/runtime/samples/store-contextual/_config.js create mode 100644 test/runtime/samples/store-contextual/main.svelte diff --git a/src/compile/nodes/shared/Expression.ts b/src/compile/nodes/shared/Expression.ts index 818066f21a..a4281eac64 100644 --- a/src/compile/nodes/shared/Expression.ts +++ b/src/compile/nodes/shared/Expression.ts @@ -122,8 +122,16 @@ export default class Expression { const { name, nodes } = flattenReference(node); if (scope.has(name)) return; + if (globalWhitelist.has(name) && !component.var_lookup.has(name)) return; + if (name[0] === '$' && template_scope.names.has(name.slice(1))) { + component.error(node, { + code: `contextual-store`, + message: `Stores must be declared at the top level of the component (this may change in a future version of Svelte)` + }); + } + if (template_scope.is_let(name)) { if (!function_expression) { dependencies.add(name); diff --git a/test/runtime/samples/store-contextual/_config.js b/test/runtime/samples/store-contextual/_config.js new file mode 100644 index 0000000000..c759ede650 --- /dev/null +++ b/test/runtime/samples/store-contextual/_config.js @@ -0,0 +1,76 @@ +import { writable } from '../../../../store.js'; + +const todos = [ + writable({ done: false, text: 'write docs' }), + writable({ done: false, text: 'implement contextual stores' }), + writable({ done: false, text: 'go outside' }) +]; + +export default { + error: `Stores must be declared at the top level of the component (this may change in a future version of Svelte)`, + + props: { + todos + }, + + html: ` + + + + + + `, + + async test({ assert, component, target, window }) { + const inputs = target.querySelectorAll('input'); + const change = new window.MouseEvent('change'); + + inputs[1].checked = true; + await inputs[1].dispatchEvent(change); + + assert.htmlEqual(target.innerHTML, ` + + + + + + `); + + await todos[0].update(todo => ({ done: !todo.done, text: todo.text })); + + assert.htmlEqual(target.innerHTML, ` + + + + + + `); + } +}; \ No newline at end of file diff --git a/test/runtime/samples/store-contextual/main.svelte b/test/runtime/samples/store-contextual/main.svelte new file mode 100644 index 0000000000..f8403eded4 --- /dev/null +++ b/test/runtime/samples/store-contextual/main.svelte @@ -0,0 +1,6 @@ +{#each todos as todo} + +{/each} \ No newline at end of file