Merge pull request #2111 from sveltejs/gh-2108

fix initialisation of imported stores

src/compile/Component.ts

@@ -433,7 +433,7 @@ export default class Component {
 		});
 	}
 
-	extract_imports(content, is_module: boolean) {
+	extract_imports(content) {
 		const { code } = this;
 
 		content.body.forEach(node => {
@@ -441,26 +441,11 @@ export default class Component {
 				// imports need to be hoisted out of the IIFE
 				removeNode(code, content.start, content.end, content.body, node);
 				this.imports.push(node);
-
-				node.specifiers.forEach((specifier: Node) => {
-					if (specifier.local.name[0] === '$') {
-						this.error(specifier.local, {
-							code: 'illegal-declaration',
-							message: `The $ prefix is reserved, and cannot be used for variable and import names`
-						});
-					}
-
-					this.add_var({
-						name: specifier.local.name,
-						module: is_module,
-						hoistable: true
-					});
-				});
 			}
 		});
 	}
 
-	extract_exports(content, is_module: boolean) {
+	extract_exports(content) {
 		const { code } = this;
 
 		content.body.forEach(node => {
@@ -558,30 +543,20 @@ export default class Component {
 				});
 			}
 
-			if (!/Import/.test(node.type)) {
-				const kind = node.type === 'VariableDeclaration'
-					? node.kind
-					: node.type === 'ClassDeclaration'
-						? 'class'
-						: node.type === 'FunctionDeclaration'
-							? 'function'
-							: null;
-
-				// sanity check
-				if (!kind) throw new Error(`Unknown declaration type ${node.type}`);
-
-				this.add_var({
-					name,
-					module: true,
-					hoistable: true,
-					writable: kind === 'var' || kind === 'let'
-				});
-			}
+			this.add_var({
+				name,
+				module: true,
+				hoistable: true,
+				writable: node.kind === 'var' || node.kind === 'let'
+			});
 		});
 
-		globals.forEach(name => {
+		globals.forEach((node, name) => {
 			if (name[0] === '$') {
-				// TODO should this be possible?
+				this.error(node, {
+					code: 'illegal-subscription',
+					message: `Cannot reference store value inside <script context="module">`
+				})
 			} else {
 				this.add_var({
 					name,
@@ -590,8 +565,8 @@ export default class Component {
 			}
 		});
 
-		this.extract_imports(script.content, true);
-		this.extract_exports(script.content, true);
+		this.extract_imports(script.content);
+		this.extract_exports(script.content);
 		remove_indentation(this.code, script.content);
 		this.module_javascript = this.extract_javascript(script);
 	}
@@ -627,29 +602,17 @@ export default class Component {
 				});
 			}
 
-			if (!/Import/.test(node.type)) {
-				const kind = node.type === 'VariableDeclaration'
-					? node.kind
-					: node.type === 'ClassDeclaration'
-						? 'class'
-						: node.type === 'FunctionDeclaration'
-							? 'function'
-							: null;
-
-				// sanity check
-				if (!kind) throw new Error(`Unknown declaration type ${node.type}`);
-
-				this.add_var({
-					name,
-					initialised: instance_scope.initialised_declarations.has(name),
-					writable: kind === 'var' || kind === 'let'
-				});
-			}
+			this.add_var({
+				name,
+				initialised: instance_scope.initialised_declarations.has(name),
+				hoistable: /^Import/.test(node.type),
+				writable: node.kind === 'var' || node.kind === 'let'
+			});
 
 			this.node_for_declaration.set(name, node);
 		});
 
-		globals.forEach(name => {
+		globals.forEach((node, name) => {
 			if (this.var_lookup.has(name)) return;
 
 			if (this.injected_reactive_declaration_vars.has(name)) {
@@ -680,8 +643,8 @@ export default class Component {
 			}
 		});
 
-		this.extract_imports(script.content, false);
-		this.extract_exports(script.content, false);
+		this.extract_imports(script.content);
+		this.extract_exports(script.content);
 		this.track_mutations();
 	}
 

src/compile/render-ssr/index.ts

@@ -27,10 +27,13 @@ export default function ssr(
 	const reactive_stores = component.vars.filter(variable => variable.name[0] === '$');
 	const reactive_store_values = reactive_stores
 		.map(({ name }) => {
-			const assignment = `${name} = @get_store_value(${name.slice(1)});`;
+			const store = component.var_lookup.get(name.slice(1));
+			if (store.hoistable) return;
+
+			const assignment = `${name} = @get_store_value(${store.name});`;
 
 			return component.compileOptions.dev
-				? `@validate_store(${name.slice(1)}, '${name.slice(1)}'); ${assignment}`
+				? `@validate_store(${store.name}, '${store.name}'); ${assignment}`
 				: assignment;
 		});
 
@@ -109,7 +112,16 @@ export default function ssr(
 			return \`${renderer.code}\`;`;
 
 	const blocks = [
-		reactive_stores.length > 0 && `let ${reactive_stores.map(store => store.name).join(', ')};`,
+		reactive_stores.length > 0 && `let ${reactive_stores
+			.map(({ name }) => {
+				const store = component.var_lookup.get(name.slice(1));
+				if (store.hoistable) {
+					const get_store_value = component.helper('get_store_value');
+					return `${name} = ${get_store_value}(${store.name})`;
+				}
+				return name;
+			})
+			.join(', ')};`,
 		user_code,
 		parent_bindings.join('\n'),
 		css.code && `$$result.css.add(#css);`,

src/utils/annotateWithScopes.ts

@@ -5,7 +5,7 @@ import { Node } from '../interfaces';
 export function createScopes(expression: Node) {
 	const map = new WeakMap();
 
-	const globals = new Set();
+	const globals: Map<string, Node> = new Map();
 	let scope = new Scope(null, false);
 
 	walk(expression, {
@@ -39,8 +39,8 @@ export function createScopes(expression: Node) {
 			} else if (/(Class|Variable)Declaration/.test(node.type)) {
 				scope.addDeclaration(node);
 			} else if (node.type === 'Identifier' && isReference(node, parent)) {
-				if (!scope.has(node.name)) {
-					globals.add(node.name);
+				if (!scope.has(node.name) && !globals.has(node.name)) {
+					globals.set(node.name, node);
 				}
 			}
 		},

test/runtime/samples/store-imported-module-b/_config.js

@@ -0,0 +1,3 @@
+export default {
+	error: `Cannot reference store value inside <script context="module">`
+};

test/runtime/samples/store-imported-module-b/foo.js

@@ -0,0 +1,3 @@
+import { writable } from '../../../../store.js';
+
+export default writable(42);

test/runtime/samples/store-imported-module-b/main.svelte

@@ -0,0 +1,6 @@
+<script context="module">
+	import foo from './foo.js';
+	const answer = $foo;
+</script>
+
+<p>{answer}</p>

test/runtime/samples/store-imported-module/_config.js

@@ -0,0 +1,5 @@
+export default {
+	html: `
+		<p>42</p>
+	`
+};

test/runtime/samples/store-imported-module/foo.js

@@ -0,0 +1,3 @@
+import { writable } from '../../../../store.js';
+
+export default writable(42);

test/runtime/samples/store-imported-module/main.svelte

@@ -0,0 +1,9 @@
+<script context="module">
+	import foo from './foo.js';
+</script>
+
+<script>
+	const answer = $foo;
+</script>
+
+<p>{answer}</p>

test/runtime/samples/store-imported/_config.js

@@ -0,0 +1,5 @@
+export default {
+	html: `
+		<p>42</p>
+	`
+};

test/runtime/samples/store-imported/foo.js

@@ -0,0 +1,3 @@
+import { writable } from '../../../../store.js';
+
+export default writable(42);

test/runtime/samples/store-imported/main.svelte

@@ -0,0 +1,6 @@
+<script>
+	import foo from './foo.js';
+	const answer = $foo;
+</script>
+
+<p>{answer}</p>