From e15d13bf91cd7c2a5fe339307884ec8054790519 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Tue, 1 Jan 2019 18:11:41 -0500 Subject: [PATCH] allow reactive store references anywhere in script - fixes #1889 --- package-lock.json | 18 ++----- package.json | 2 +- src/compile/Component.ts | 48 +++++++++++++++---- .../store-auto-subscribe-in-script/_config.js | 26 ++++++++++ .../store-auto-subscribe-in-script/main.html | 9 ++++ 5 files changed, 81 insertions(+), 22 deletions(-) create mode 100644 test/runtime/samples/store-auto-subscribe-in-script/_config.js create mode 100644 test/runtime/samples/store-auto-subscribe-in-script/main.html diff --git a/package-lock.json b/package-lock.json index b435177e9d..ce46cbb8b8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "svelte", - "version": "3.0.0-alpha12", + "version": "3.0.0-alpha13", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -2806,20 +2806,12 @@ "dev": true }, "is-reference": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/is-reference/-/is-reference-1.1.0.tgz", - "integrity": "sha512-h37O/IX4efe56o9k41II1ECMqKwtqHa7/12dLDEzJIFux2x15an4WCDb0/eKdmUgRpLJ3bR0DrzDc7vOrVgRDw==", + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/is-reference/-/is-reference-1.1.1.tgz", + "integrity": "sha512-URlByVARcyP2E2GC7d3Ur702g3vqW391VKCHuF5Goo/M8IT97k4RU/+56OYImwDdX1J/V/VRxECE/wJqB0I2tg==", "dev": true, "requires": { - "@types/estree": "0.0.38" - }, - "dependencies": { - "@types/estree": { - "version": "0.0.38", - "resolved": "https://registry.npmjs.org/@types/estree/-/estree-0.0.38.tgz", - "integrity": "sha512-F/v7t1LwS4vnXuPooJQGBRKRGIoxWUTmA4VHfqjOccFsNDThD5bfUNpITive6s352O7o384wcpEaDV8rHCehDA==", - "dev": true - } + "@types/estree": "0.0.39" } }, "is-typedarray": { diff --git a/package.json b/package.json index a519b784c9..ba5512f589 100644 --- a/package.json +++ b/package.json @@ -63,7 +63,7 @@ "eslint-plugin-html": "^4.0.3", "eslint-plugin-import": "^2.11.0", "estree-walker": "^0.6.0", - "is-reference": "^1.1.0", + "is-reference": "^1.1.1", "jsdom": "^11.8.0", "kleur": "^3.0.0", "locate-character": "^2.0.5", diff --git a/src/compile/Component.ts b/src/compile/Component.ts index 3118d8fa08..541814a1ff 100644 --- a/src/compile/Component.ts +++ b/src/compile/Component.ts @@ -539,9 +539,43 @@ export default class Component { this.extract_imports_and_exports(script.content, this.imports, this.props); this.hoist_instance_declarations(); this.extract_reactive_declarations(); + this.extract_reactive_store_references(); this.javascript = this.extract_javascript(script); } + extract_reactive_store_references() { + // TODO this pattern happens a lot... can we abstract it + // (or better still, do fewer AST walks)? + const component = this; + let { instance_scope: scope, instance_scope_map: map } = this; + + walk(this.instance_script.content, { + enter(node, parent) { + if (map.has(node)) { + scope = map.get(node); + } + + if (isReference(node, parent)) { + const object = getObject(node); + const { name } = object; + + if (name[0] === '$' && !scope.has(name)) { + component.warn_if_undefined(object, null); + + // cheeky hack + component.template_references.add(name); + } + } + }, + + leave(node) { + if (map.has(node)) { + scope = scope.parent; + } + } + }); + } + rewrite_props() { const component = this; const { code, instance_scope, instance_scope_map: map, meta } = this; @@ -735,7 +769,11 @@ export default class Component { const { name } = flattenReference(node); const owner = scope.findOwner(name); - if (owner === instance_scope) { + if (name[0] === '$' && !owner) { + hoistable = false; + } + + else if (owner === instance_scope) { if (name === fn_declaration.id.name) return; if (hoistable_names.has(name)) return; if (imported_declarations.has(name)) return; @@ -818,13 +856,7 @@ export default class Component { const object = getObject(node); const { name } = object; - if (component.declarations.indexOf(name) !== -1) { - dependencies.add(name); - } else if (name[0] === '$') { - component.warn_if_undefined(object, null); - - // cheeky hack - component.template_references.add(name); + if (name[0] === '$' || component.declarations.indexOf(name) !== -1) { dependencies.add(name); } diff --git a/test/runtime/samples/store-auto-subscribe-in-script/_config.js b/test/runtime/samples/store-auto-subscribe-in-script/_config.js new file mode 100644 index 0000000000..a7a8ef8d27 --- /dev/null +++ b/test/runtime/samples/store-auto-subscribe-in-script/_config.js @@ -0,0 +1,26 @@ +import { writable } from '../../../../store.js'; + +const count = writable(0); + +export default { + props: { + count + }, + + html: ` + + `, + + async test({ assert, component, target, window }) { + assert.equal(component.get_count(), 0); + + const button = target.querySelector('button'); + const click = new window.MouseEvent('click'); + + await button.dispatchEvent(click); + assert.equal(component.get_count(), 1); + + await count.set(42); + assert.equal(component.get_count(), 42); + } +}; \ No newline at end of file diff --git a/test/runtime/samples/store-auto-subscribe-in-script/main.html b/test/runtime/samples/store-auto-subscribe-in-script/main.html new file mode 100644 index 0000000000..b47bddf9c1 --- /dev/null +++ b/test/runtime/samples/store-auto-subscribe-in-script/main.html @@ -0,0 +1,9 @@ + + + \ No newline at end of file