diff --git a/CHANGELOG.md b/CHANGELOG.md index d5e7583891..e79b16379e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +* Warn when using `module` variables reactively, and close weird reactivity loophole ([#5847](https://github.com/sveltejs/svelte/pull/5847)) * Throw a parser error for `class:` directives with an empty class name ([#5858](https://github.com/sveltejs/svelte/issues/5858)) * Fix extraneous store subscription in SSR mode ([#5883](https://github.com/sveltejs/svelte/issues/5883)) * Don't emit update code for `class:` directives whose expression is not dynamic ([#5919](https://github.com/sveltejs/svelte/issues/5919)) diff --git a/src/compiler/compile/Component.ts b/src/compiler/compile/Component.ts index d2b22c65e1..8aab2b4898 100644 --- a/src/compiler/compile/Component.ts +++ b/src/compiler/compile/Component.ts @@ -1175,15 +1175,20 @@ export default class Component { extract_reactive_declarations() { const component = this; - const unsorted_reactive_declarations = []; + const unsorted_reactive_declarations: Array<{ + assignees: Set; + dependencies: Set; + node: Node; + declaration: Node; + }> = []; this.ast.instance.content.body.forEach(node => { if (node.type === 'LabeledStatement' && node.label.name === '$') { this.reactive_declaration_nodes.add(node); - const assignees = new Set(); + const assignees = new Set(); const assignee_nodes = new Set(); - const dependencies = new Set(); + const dependencies = new Set(); let scope = this.instance_scope; const map = this.instance_scope_map; @@ -1214,10 +1219,22 @@ export default class Component { const { name } = identifier; const owner = scope.find_owner(name); const variable = component.var_lookup.get(name); - if (variable) variable.is_reactive_dependency = true; + let should_add_as_dependency = true; + + if (variable) { + variable.is_reactive_dependency = true; + if (variable.module) { + should_add_as_dependency = false; + component.warn(node as any, { + code: 'module-script-reactive-declaration', + message: `"${name}" is declared in a module script and will not be reactive` + }); + } + } const is_writable_or_mutated = variable && (variable.writable || variable.mutated); if ( + should_add_as_dependency && (!owner || owner === component.instance_scope) && (name[0] === '$' || is_writable_or_mutated) ) { diff --git a/test/js/samples/reactive-class-optimized/expected.js b/test/js/samples/reactive-class-optimized/expected.js index 293249fd62..b7c2c3a60b 100644 --- a/test/js/samples/reactive-class-optimized/expected.js +++ b/test/js/samples/reactive-class-optimized/expected.js @@ -148,12 +148,7 @@ function instance($$self, $$props, $$invalidate) { reactiveConst.x += 1; } - $$self.$$.update = () => { - if ($$self.$$.dirty & /*reactiveModuleVar*/ 0) { - $: $$subscribe_reactiveDeclaration($$invalidate(1, reactiveDeclaration = reactiveModuleVar * 2)); - } - }; - + $: $$subscribe_reactiveDeclaration($$invalidate(1, reactiveDeclaration = reactiveModuleVar * 2)); return [reactiveConst, reactiveDeclaration, $reactiveStoreVal, $reactiveDeclaration]; } diff --git a/test/runtime/samples/reactive-statement-module-vars/_config.js b/test/runtime/samples/reactive-statement-module-vars/_config.js new file mode 100644 index 0000000000..63b89cd164 --- /dev/null +++ b/test/runtime/samples/reactive-statement-module-vars/_config.js @@ -0,0 +1,18 @@ +export default { + html: ` + a: moduleA + b: moduleB + moduleA: moduleA + moduleB: moduleB + `, + async test({ assert, target, component }) { + await component.updateModuleA(); + + assert.htmlEqual(target.innerHTML, ` + a: moduleA + b: moduleB + moduleA: moduleA + moduleB: moduleB + `); + } +}; diff --git a/test/runtime/samples/reactive-statement-module-vars/main.svelte b/test/runtime/samples/reactive-statement-module-vars/main.svelte new file mode 100644 index 0000000000..70e0404657 --- /dev/null +++ b/test/runtime/samples/reactive-statement-module-vars/main.svelte @@ -0,0 +1,15 @@ + + +a: {a} +b: {b} +moduleA: {moduleA} +moduleB: {moduleB} diff --git a/test/validator/samples/reactive-module-variable/input.svelte b/test/validator/samples/reactive-module-variable/input.svelte new file mode 100644 index 0000000000..5a540de4f0 --- /dev/null +++ b/test/validator/samples/reactive-module-variable/input.svelte @@ -0,0 +1,6 @@ + + diff --git a/test/validator/samples/reactive-module-variable/warnings.json b/test/validator/samples/reactive-module-variable/warnings.json new file mode 100644 index 0000000000..c3c52cb479 --- /dev/null +++ b/test/validator/samples/reactive-module-variable/warnings.json @@ -0,0 +1,17 @@ +[ + { + "code": "module-script-reactive-declaration", + "message": "\"foo\" is declared in a module script and will not be reactive", + "pos": 65, + "start": { + "character": 65, + "column": 10, + "line": 5 + }, + "end": { + "character": 68, + "column": 13, + "line": 5 + } + } +]