fix: do not add module declared variables as dependencies (sveltejs#9122

) closes sveltejs#5943
kelvinsjk · Oct 19, 2023 · ec5a340 · ec5a340
1 parent 558c8d5
commit ec5a340
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 22 deletions.
diff --git a/.changeset/famous-news-perform.md b/.changeset/famous-news-perform.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: do not add module declared variables as dependencies
diff --git a/packages/svelte/src/compiler/compile/render_dom/wrappers/shared/is_dynamic.js b/packages/svelte/src/compiler/compile/render_dom/wrappers/shared/is_dynamic.js
@@ -3,8 +3,11 @@ import { is_reserved_keyword } from '../../../utils/reserved_keywords.js';
 /** @param {import('../../../../interfaces.js').Var} variable */
 export default function is_dynamic(variable) {
 	if (variable) {
-		if (variable.mutated || variable.reassigned) return true; // dynamic internal state
-		if (!variable.module && variable.writable && variable.export_name) return true; // writable props
+		// Only variables declared in the instance script tags should be considered dynamic
+		const is_declared_in_reactive_context = !variable.module && !variable.global;
+
+		if (is_declared_in_reactive_context && (variable.mutated || variable.reassigned)) return true; // dynamic internal state
+		if (is_declared_in_reactive_context && variable.writable && variable.export_name) return true; // writable props
 		if (is_reserved_keyword(variable.name)) return true;
 	}
 	return false;

diff --git a/packages/svelte/test/js/samples/reactive-class-optimized/expected.js b/packages/svelte/test/js/samples/reactive-class-optimized/expected.js
@@ -53,11 +53,11 @@ function create_fragment(ctx) {
 			div7 = element("div");
 			t7 = space();
 			div8 = element("div");
-			toggle_class(div0, "update1", reactiveModuleVar);
-			toggle_class(div1, "update2", /*reactiveConst*/ ctx[0].x);
-			toggle_class(div2, "update3", nonReactiveGlobal && /*reactiveConst*/ ctx[0].x);
-			toggle_class(div3, "update4", /*$reactiveStoreVal*/ ctx[2]);
-			toggle_class(div4, "update5", /*$reactiveDeclaration*/ ctx[3]);
+			toggle_class(div0, "update2", /*reactiveConst*/ ctx[0].x);
+			toggle_class(div1, "update3", nonReactiveGlobal && /*reactiveConst*/ ctx[0].x);
+			toggle_class(div2, "update4", /*$reactiveStoreVal*/ ctx[2]);
+			toggle_class(div3, "update5", /*$reactiveDeclaration*/ ctx[3]);
+			toggle_class(div4, "update1", reassignedModuleVar);
 			toggle_class(div5, "static1", nonReactiveModuleVar);
 			toggle_class(div6, "static2", nonReactiveGlobal);
 			toggle_class(div7, "static3", nonReactiveModuleVar && nonReactiveGlobal);
@@ -83,24 +83,20 @@ function create_fragment(ctx) {
 			insert(target, div8, anchor);
 		},
 		p(ctx, [dirty]) {
-			if (dirty & /*reactiveModuleVar*/ 0) {
-				toggle_class(div0, "update1", reactiveModuleVar);
-			}
-
 			if (dirty & /*reactiveConst*/ 1) {
-				toggle_class(div1, "update2", /*reactiveConst*/ ctx[0].x);
+				toggle_class(div0, "update2", /*reactiveConst*/ ctx[0].x);
 			}
 
 			if (dirty & /*nonReactiveGlobal, reactiveConst*/ 1) {
-				toggle_class(div2, "update3", nonReactiveGlobal && /*reactiveConst*/ ctx[0].x);
+				toggle_class(div1, "update3", nonReactiveGlobal && /*reactiveConst*/ ctx[0].x);
 			}
 
 			if (dirty & /*$reactiveStoreVal*/ 4) {
-				toggle_class(div3, "update4", /*$reactiveStoreVal*/ ctx[2]);
+				toggle_class(div2, "update4", /*$reactiveStoreVal*/ ctx[2]);
 			}
 
 			if (dirty & /*$reactiveDeclaration*/ 8) {
-				toggle_class(div4, "update5", /*$reactiveDeclaration*/ ctx[3]);
+				toggle_class(div3, "update5", /*$reactiveDeclaration*/ ctx[3]);
 			}
 		},
 		i: noop,
@@ -130,7 +126,7 @@ function create_fragment(ctx) {
 }
 
 let nonReactiveModuleVar = Math.random();
-let reactiveModuleVar = Math.random();
+let reassignedModuleVar = Math.random();
 
 function instance($$self, $$props, $$invalidate) {
 	let reactiveDeclaration;
@@ -144,13 +140,13 @@ function instance($$self, $$props, $$invalidate) {
 	$$self.$$.on_destroy.push(() => $$unsubscribe_reactiveDeclaration());
 	nonReactiveGlobal = Math.random();
 	const reactiveConst = { x: Math.random() };
-	reactiveModuleVar += 1;
+	reassignedModuleVar += 1;
 
 	if (Math.random()) {
 		reactiveConst.x += 1;
 	}
 
-	$: $$subscribe_reactiveDeclaration($$invalidate(1, reactiveDeclaration = reactiveModuleVar * 2));
+	$: $$subscribe_reactiveDeclaration($$invalidate(1, reactiveDeclaration = reassignedModuleVar * 2));
 	return [reactiveConst, reactiveDeclaration, $reactiveStoreVal, $reactiveDeclaration];
 }
 

diff --git a/packages/svelte/test/js/samples/reactive-class-optimized/input.svelte b/packages/svelte/test/js/samples/reactive-class-optimized/input.svelte
@@ -1,6 +1,6 @@
 <script context="module">
 	let nonReactiveModuleVar = Math.random();
-	let reactiveModuleVar = Math.random();
+	let reassignedModuleVar = Math.random();
 </script>
 
 <script>
@@ -9,22 +9,22 @@
 	nonReactiveGlobal = Math.random();
 	const reactiveConst = {x: Math.random()};
 
-	$: reactiveDeclaration = reactiveModuleVar * 2;
+	$: reactiveDeclaration = reassignedModuleVar * 2;
 
-	reactiveModuleVar += 1;
+	reassignedModuleVar += 1;
 	if (Math.random()) {
 		reactiveConst.x += 1;
 	}
 </script>
 
 <!--These should all get updaters because they have at least one reactive dependency-->
-<div class:update1={reactiveModuleVar}></div>
 <div class:update2={reactiveConst.x}></div>
 <div class:update3={nonReactiveGlobal && reactiveConst.x}></div>
 <div class:update4={$reactiveStoreVal}></div>
 <div class:update5={$reactiveDeclaration}></div>
 
 <!--These shouldn't get updates because they're purely non-reactive-->
+<div class:update1={reassignedModuleVar}></div>
 <div class:static1={nonReactiveModuleVar}></div>
 <div class:static2={nonReactiveGlobal}></div>
 <div class:static3={nonReactiveModuleVar && nonReactiveGlobal}></div>