fix: revert buggy reactive vars optimization (#8382)

Reverts #7942
Fixes #8374

src/compiler/compile/Component.ts

@@ -38,7 +38,6 @@ import compiler_warnings from './compiler_warnings';
 import compiler_errors from './compiler_errors';
 import { extract_ignores_above_position, extract_svelte_ignore_from_comments } from '../utils/extract_svelte_ignore';
 import check_enable_sourcemap from './utils/check_enable_sourcemap';
-import is_dynamic from './render_dom/wrappers/shared/is_dynamic';
 
 interface ComponentOptions {
 	namespace?: string;
@@ -1392,11 +1391,12 @@ export default class Component {
 										module_dependencies.add(name);
 									}
 								}
-
+								const is_writable_or_mutated =
+									variable && (variable.writable || variable.mutated);
 								if (
 									should_add_as_dependency &&
 									(!owner || owner === component.instance_scope) &&
-									(name[0] === '$' || variable)
+									(name[0] === '$' || is_writable_or_mutated)
 								) {
 									dependencies.add(name);
 								}
@@ -1420,19 +1420,6 @@ export default class Component {
 				const { expression } = node.body as ExpressionStatement;
 				const declaration = expression && (expression as AssignmentExpression).left;
 
-				const is_dependency_static = Array.from(dependencies).every(
-					dependency => dependency !== '$$props' && dependency !== '$$restProps' && !is_dynamic(this.var_lookup.get(dependency))
-				);
-
-				if (is_dependency_static) {
-					assignees.forEach(assignee => {
-						const variable = component.var_lookup.get(assignee);
-						if (variable) {
-							variable.is_reactive_static = true;
-						}
-					});
-				}
-
 				unsorted_reactive_declarations.push({
 					assignees,
 					dependencies,

src/compiler/compile/render_dom/index.ts

@@ -390,13 +390,13 @@ export default function dom(
 	const resubscribable_reactive_store_unsubscribers = reactive_stores
 		.filter(store => {
 			const variable = component.var_lookup.get(store.name.slice(1));
-			return variable && (variable.reassigned || variable.export_name) && !variable.is_reactive_static;
+			return variable && (variable.reassigned || variable.export_name);
 		})
 		.map(({ name }) => b`$$self.$$.on_destroy.push(() => ${`$$unsubscribe_${name.slice(1)}`}());`);
 
 	if (has_definition) {
-		const reactive_declarations: Node[] = [];
+		const reactive_declarations: (Node | Node[]) = [];
-		const fixed_reactive_declarations: Array<Node | Node[]> = []; // not really 'reactive' but whatever
+		const fixed_reactive_declarations: Node[] = []; // not really 'reactive' but whatever
 
 		component.reactive_declarations.forEach(d => {
 			const dependencies = Array.from(d.dependencies);
@@ -417,15 +417,6 @@ export default function dom(
 				reactive_declarations.push(statement);
 			} else {
 				fixed_reactive_declarations.push(statement);
-				for (const assignee of d.assignees) {
-					const variable = component.var_lookup.get(assignee);
-					if (variable && variable.subscribable) {
-						fixed_reactive_declarations.push(b`
-							${component.compile_options.dev && b`@validate_store(${assignee}, '${assignee}');`}
-							@component_subscribe($$self, ${assignee}, $$value => $$invalidate(${renderer.context_lookup.get('$' + assignee).index}, ${'$' + assignee} = $$value));
-						`);
-					}
-				}
 			}
 		});
 
@@ -439,7 +430,7 @@ export default function dom(
 			const name = $name.slice(1);
 
 			const store = component.var_lookup.get(name);
-			if (store && (store.reassigned || store.export_name) && !store.is_reactive_static) {
+			if (store && (store.reassigned || store.export_name)) {
 				const unsubscribe = `$$unsubscribe_${name}`;
 				const subscribe = `$$subscribe_${name}`;
 				const i = renderer.context_lookup.get($name).index;

src/compiler/compile/render_dom/invalidate.ts

@@ -19,7 +19,6 @@ export function invalidate(renderer: Renderer, scope: Scope, node: Node, names:
 				!variable.hoistable &&
 				!variable.global &&
 				!variable.module &&
-				!variable.is_reactive_static &&
 				(
 					variable.referenced ||
 					variable.subscribable ||

src/compiler/interfaces.ts

@@ -223,7 +223,6 @@ export interface Var {
 	subscribable?: boolean;
 	is_reactive_dependency?: boolean;
 	imported?: boolean;
-	is_reactive_static?: boolean;
 }
 
 export interface CssResult {

test/js/samples/reactive-class-optimized/expected.js

@@ -9,6 +9,7 @@ import {
 	noop,
 	safe_not_equal,
 	space,
+	subscribe,
 	toggle_class
 } from "svelte/internal";
 
@@ -132,8 +133,13 @@ let reactiveModuleVar = Math.random();
 function instance($$self, $$props, $$invalidate) {
 	let reactiveDeclaration;
 	let $reactiveStoreVal;
-	let $reactiveDeclaration;
+
+	let $reactiveDeclaration,
+		$$unsubscribe_reactiveDeclaration = noop,
+		$$subscribe_reactiveDeclaration = () => ($$unsubscribe_reactiveDeclaration(), $$unsubscribe_reactiveDeclaration = subscribe(reactiveDeclaration, $$value => $$invalidate(3, $reactiveDeclaration = $$value)), reactiveDeclaration);
+
 	component_subscribe($$self, reactiveStoreVal, $$value => $$invalidate(2, $reactiveStoreVal = $$value));
+	$$self.$$.on_destroy.push(() => $$unsubscribe_reactiveDeclaration());
 	nonReactiveGlobal = Math.random();
 	const reactiveConst = { x: Math.random() };
 	reactiveModuleVar += 1;
@@ -142,8 +148,7 @@ function instance($$self, $$props, $$invalidate) {
 		reactiveConst.x += 1;
 	}
 
-	$: reactiveDeclaration = reactiveModuleVar * 2;
+	$: $$subscribe_reactiveDeclaration($$invalidate(1, reactiveDeclaration = reactiveModuleVar * 2));
-	component_subscribe($$self, reactiveDeclaration, $$value => $$invalidate(3, $reactiveDeclaration = $$value));
 	return [reactiveConst, reactiveDeclaration, $reactiveStoreVal, $reactiveDeclaration];
 }
 

test/js/samples/reactive-values/expected.js

@@ -1,60 +0,0 @@
-/* generated by Svelte vX.Y.Z */
-import {
-	SvelteComponent,
-	detach,
-	element,
-	init,
-	insert,
-	noop,
-	safe_not_equal,
-	set_data,
-	space,
-	text
-} from "svelte/internal";
-
-function create_fragment(ctx) {
-	let h1;
-	let t3;
-	let t4;
-
-	return {
-		c() {
-			h1 = element("h1");
-			h1.textContent = `Hello ${name}!`;
-			t3 = space();
-			t4 = text(/*foo*/ ctx[0]);
-		},
-		m(target, anchor) {
-			insert(target, h1, anchor);
-			insert(target, t3, anchor);
-			insert(target, t4, anchor);
-		},
-		p(ctx, [dirty]) {
-			if (dirty & /*foo*/ 1) set_data(t4, /*foo*/ ctx[0]);
-		},
-		i: noop,
-		o: noop,
-		d(detaching) {
-			if (detaching) detach(h1);
-			if (detaching) detach(t3);
-			if (detaching) detach(t4);
-		}
-	};
-}
-
-let name = 'world';
-
-function instance($$self) {
-	let foo;
-	$: foo = name + name;
-	return [foo];
-}
-
-class Component extends SvelteComponent {
-	constructor(options) {
-		super();
-		init(this, options, instance, create_fragment, safe_not_equal, {});
-	}
-}
-
-export default Component;

test/js/samples/reactive-values/input.svelte

@@ -1,7 +0,0 @@
-<script>
-	let name = 'world';
-	$: foo = name + name;
-</script>
-
-<h1>Hello {name}!</h1>
-{foo}

test/runtime/samples/reactive-statement-indirect/_config.js

@@ -0,0 +1,14 @@
+export default {
+	html: `
+		<h1>2</h1>
+		<button>Increment</button>
+	`,
+	async test({ assert, target }) {
+		await target.querySelector('button').dispatchEvent(new window.MouseEvent('click'));
+
+		assert.htmlEqual(target.innerHTML, `
+			<h1>4</h1>
+			<button>Increment</button>
+		`);
+	}
+};

test/runtime/samples/reactive-statement-indirect/main.svelte

@@ -0,0 +1,11 @@
+<script>
+	let count = 1;
+	// Could be a let or simplified, but this tests that it still works like this
+	$: indirect_double = 2;
+	$: if (count > 0) {
+		indirect_double = count * 2;
+	}
+</script>
+
+<h1>{indirect_double}</h1>
+<button on:click={() => count++}>Increment</button>