fix: address runtime effect issues (#9417)

* Fix runtime effect issues

* Prettier

* Add changeset

* Fix operations

* Update .changeset/khaki-mails-draw.md

* more tweaks

* more tweaks

---------

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>

Author: trueadm

.changeset/khaki-mails-draw.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: tighten up signals implementation

packages/svelte/src/compiler/phases/3-transform/client/utils.js

@@ -251,6 +251,8 @@ export const function_visitor = (node, context) => {
 		const in_constructor = parent.type === 'MethodDefinition' && parent.kind === 'constructor';
 
 		state = { ...context.state, in_constructor };
+	} else {
+		state = { ...context.state, in_constructor: false };
 	}
 
 	if (metadata?.hoistable === true) {

packages/svelte/src/internal/client/operations.js

@@ -8,21 +8,19 @@ const has_browser_globals = typeof window !== 'undefined';
 // than megamorphic.
 const node_prototype = /** @type {Node} */ (has_browser_globals ? Node.prototype : {});
 const element_prototype = /** @type {Element} */ (has_browser_globals ? Element.prototype : {});
-const event_target_prototype = /** @type {EventTarget} */ (
-	has_browser_globals ? EventTarget.prototype : {}
-);
+const text_prototype = /** @type {Text} */ (has_browser_globals ? Text.prototype : {});
 const map_prototype = Map.prototype;
 const append_child_method = node_prototype.appendChild;
 const clone_node_method = node_prototype.cloneNode;
 const map_set_method = map_prototype.set;
 const map_get_method = map_prototype.get;
 const map_delete_method = map_prototype.delete;
-// @ts-expect-error improve perf of expando on DOM nodes for events
-event_target_prototype.__click = undefined;
-// @ts-expect-error improve perf of expando on DOM textValue updates
-event_target_prototype.__nodeValue = ' ';
+// @ts-expect-error improve perf of expando on DOM events
+element_prototype.__click = undefined;
+// @ts-expect-error improve perf of expando on DOM text updates
+text_prototype.__nodeValue = ' ';
 // @ts-expect-error improve perf of expando on DOM className updates
-event_target_prototype.__className = '';
+element_prototype.__className = '';
 
 const first_child_get = /** @type {(this: Node) => ChildNode | null} */ (
 	// @ts-ignore
@@ -162,11 +160,10 @@ export function set_class_name(node, class_name) {
 /**
  * @template {Node} N
  * @param {N} node
- * @param {string} text
  * @returns {void}
  */
-export function text_content(node, text) {
-	text_content_set.call(node, text);
+export function clear_text_content(node) {
+	text_content_set.call(node, '');
 }
 
 /** @param {string} name */

packages/svelte/src/internal/client/reconciler.js

@@ -1,4 +1,4 @@
-import { append_child, map_get, map_set, text_content } from './operations.js';
+import { append_child, map_get, map_set, clear_text_content } from './operations.js';
 import {
 	current_hydration_fragment,
 	get_hydration_fragment,
@@ -198,7 +198,7 @@ export function reconcile_indexed_array(
 		b_blocks = [];
 		// Remove old blocks
 		if (is_controlled && a !== 0) {
-			text_content(dom, '');
+			clear_text_content(dom);
 		}
 		while (index < length) {
 			block = a_blocks[index++];
@@ -295,7 +295,7 @@ export function reconcile_tracked_array(
 		b_blocks = [];
 		// Remove old blocks
 		if (is_controlled && a !== 0) {
-			text_content(dom, '');
+			clear_text_content(dom);
 		}
 		while (a > 0) {
 			block = a_blocks[--a];

packages/svelte/src/internal/client/render.js

@@ -1270,15 +1270,15 @@ function handle_event_propagation(root_element, event) {
 	}
 
 	// composedPath contains list of nodes the event has propagated through.
-	// We check __handled_event_at to skip all nodes below it in case this is a
-	// parent of the __handled_event_at node, which indicates that there's nested
+	// We check __root to skip all nodes below it in case this is a
+	// parent of the __root node, which indicates that there's nested
 	// mounted apps. In this case we don't want to trigger events multiple times.
 	// We're deliberately not skipping if the index is the same or higher, because
 	// someone could create an event programmatically and emit it multiple times,
 	// in which case we want to handle the whole propagation chain properly each time.
 	let path_idx = 0;
 	// @ts-expect-error is added below
-	const handled_at = event.__handled_event_at;
+	const handled_at = event.__root;
 	if (handled_at) {
 		const at_idx = path.indexOf(handled_at);
 		if (at_idx < path.indexOf(root_element)) {
@@ -1317,7 +1317,7 @@ function handle_event_propagation(root_element, event) {
 	}
 
 	// @ts-expect-error is used above
-	event.__handled_event_at = root_element;
+	event.__root = root_element;
 }
 
 /**

packages/svelte/src/internal/client/runtime.js

@@ -343,7 +343,13 @@ function destroy_references(signal) {
 	if (references !== null) {
 		let i;
 		for (i = 0; i < references.length; i++) {
-			destroy_signal(references[i]);
+			const reference = references[i];
+			if ((reference.flags & IS_EFFECT) !== 0) {
+				destroy_signal(reference);
+			} else {
+				remove_consumer(reference, 0, true);
+				reference.dependencies = null;
+			}
 		}
 	}
 }
@@ -710,7 +716,7 @@ export function exposable(fn) {
 export function get(signal) {
 	const flags = signal.flags;
 	if ((flags & DESTROYED) !== 0) {
-		return /** @type {V} */ (UNINITIALIZED);
+		return signal.value;
 	}
 
 	if (is_signal_exposed && current_should_capture_signal) {
@@ -1156,6 +1162,11 @@ export function managed_pre_effect(init, sync) {
  * @returns {import('./types.js').EffectSignal}
  */
 export function pre_effect(init) {
+	if (current_effect === null) {
+		throw new Error(
+			'The Svelte $effect.pre rune can only be used during component initialisation.'
+		);
+	}
 	const sync = current_effect !== null && (current_effect.flags & RENDER_EFFECT) !== 0;
 	return internal_create_effect(
 		PRE_EFFECT,

packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private/_config.js

@@ -0,0 +1,13 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	html: `<button>10</button>`,
+	ssrHtml: `<button>0</button>`,
+
+	async test({ assert, target }) {
+		flushSync();
+
+		assert.htmlEqual(target.innerHTML, `<button>10</button>`);
+	}
+});

packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private/main.svelte

@@ -0,0 +1,14 @@
+<script>
+	class Counter {
+		count = $state(0);
+
+		constructor() {
+			$effect(() => {
+				this.count = 10;
+			});
+		}
+	}
+	const counter = new Counter();
+</script>
+
+<button on:click={() => counter.count++}>{counter.count}</button>

packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure/_config.js

@@ -0,0 +1,13 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	html: `<button>10</button>`,
+	ssrHtml: `<button>0</button>`,
+
+	async test({ assert, target }) {
+		flushSync();
+
+		assert.htmlEqual(target.innerHTML, `<button>10</button>`);
+	}
+});

packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure/main.svelte

@@ -0,0 +1,22 @@
+<script>
+	class Counter {
+		#count = $state(0);
+
+		constructor() {
+			$effect(() => {
+				this.#count = 10;
+			});
+		}
+
+		getCount() {
+			return this.#count;
+		}
+
+		increment() {
+			this.#count++;
+		}
+	}
+	const counter = new Counter();
+</script>
+
+<button on:click={() => counter.increment()}>{counter.getCount()}</button>

packages/svelte/tests/runtime-runes/samples/effect-cleanup/_config.js

@@ -0,0 +1,19 @@
+import { test } from '../../test';
+import { flushSync } from 'svelte';
+
+export default test({
+	get props() {
+		return { log: [] };
+	},
+
+	async test({ assert, target, component }) {
+		const [b1] = target.querySelectorAll('button');
+		flushSync(() => {
+			b1.click();
+		});
+		flushSync(() => {
+			b1.click();
+		});
+		assert.deepEqual(component.log, ['init 0', 'cleanup 2', 'init 2', 'cleanup 4', 'init 4']);
+	}
+});

packages/svelte/tests/runtime-runes/samples/effect-cleanup/main.svelte

@@ -0,0 +1,17 @@
+<script>
+	const {log} = $props();
+
+	let count = $state(0);
+
+	$effect(() => {
+		let double = $derived(count * 2)
+
+		log.push('init ' + double);
+
+		return () => {
+			log.push('cleanup ' + double);
+		};
+	})
+</script>
+
+<button on:click={() => count++ }>Click</button>