fix: improve action support for nested $effect (#10962)

* fix: improve action support for nested $effect

* tweaks

* simplify

* comment

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>

.changeset/itchy-bulldogs-tan.md

@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+fix: improve action support for nested $effect

packages/svelte/src/internal/client/dom/elements/actions.js

@@ -1,53 +1,37 @@
-import { effect } from '../../reactivity/effects.js';
+import { effect, render_effect } from '../../reactivity/effects.js';
 import { deep_read_state, untrack } from '../../runtime.js';
 
 /**
  * @template P
  * @param {Element} dom
  * @param {(dom: Element, value?: P) => import('#client').ActionPayload<P>} action
- * @param {() => P} [value_fn]
+ * @param {() => P} [get_value]
  * @returns {void}
  */
-export function action(dom, action, value_fn) {
+export function action(dom, action, get_value) {
-	/** @type {undefined | import('#client').ActionPayload<P>} */
-	var payload = undefined;
-	var needs_deep_read = false;
-
-	// Action could come from a prop, therefore could be a signal, therefore untrack
-	// TODO we could take advantage of this and enable https://github.com/sveltejs/svelte/issues/6942
 	effect(() => {
-		if (value_fn) {
+		var payload = untrack(() => action(dom, get_value?.()) || {});
-			var value = value_fn();
+		var update = payload?.update;
-			untrack(() => {
+
-				if (payload === undefined) {
+		if (get_value && update) {
-					payload = action(dom, value) || {};
+			var inited = false;
-					needs_deep_read = !!payload?.update;
+
-				} else {
+			render_effect(() => {
-					var update = payload.update;
+				var value = get_value();
-					if (typeof update === 'function') {
+
-						update(value);
+				// Action's update method is coarse-grained, i.e. when anything in the passed value changes, update.
-					}
+				// This works in legacy mode because of mutable_source being updated as a whole, but when using $state
+				// together with actions and mutation, it wouldn't notice the change without a deep read.
+				deep_read_state(value);
+
+				if (inited) {
+					/** @type {Function} */ (update)(value);
 				}
 			});
-			// Action's update method is coarse-grained, i.e. when anything in the passed value changes, update.
-			// This works in legacy mode because of mutable_source being updated as a whole, but when using $state
-			// together with actions and mutation, it wouldn't notice the change without a deep read.
-			if (needs_deep_read) {
-				deep_read_state(value);
-			}
-		} else {
-			untrack(() => (payload = action(dom)));
-		}
-	});
 
-	effect(() => {
+			inited = true;
-		if (payload !== undefined) {
-			var destroy = payload.destroy;
-			if (typeof destroy === 'function') {
-				return () => {
-					/** @type {Function} */ (destroy)();
-				};
-			}
 		}
+
+		return payload?.destroy;
 	});
 }

packages/svelte/tests/runtime-runes/samples/action-state-arg-deep/Task.svelte

@@ -0,0 +1,29 @@
+<script>
+	import { log } from './log.js';
+
+  const { prop } = $props()
+
+  let trackedState = $state(0)
+  const getTrackedState = () => trackedState
+
+  function dummyAction(el, { getTrackedState, propFromComponent }) {
+    $effect(() => {
+      log.push("action $effect: ", { buttonClicked: getTrackedState() })
+    })
+  }
+</script>
+
+<div
+  class="container"
+  use:dummyAction={{ getTrackedState, propFromComponent: prop }}
+>
+  {JSON.stringify(prop)}
+</div>
+
+<button
+  onclick={() => {
+    trackedState += 1
+  }}
+>
+  update tracked state
+</button>

packages/svelte/tests/runtime-runes/samples/action-state-arg-deep/_config.js

@@ -0,0 +1,40 @@
+import { test } from '../../test';
+import { flushSync, tick } from 'svelte';
+import { log } from './log.js';
+
+export default test({
+	before_test() {
+		log.length = 0;
+	},
+
+	html: `<div class="container">{"text":"initial"}</div><button>update tracked state</button><button>Update prop</button>`,
+
+	async test({ assert, target }) {
+		const [btn1, btn2] = target.querySelectorAll('button');
+
+		flushSync(() => {
+			btn2.click();
+		});
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`<div class="container">{"text":"updated"}</div><button>update tracked state</button><button>Update prop</button>`
+		);
+
+		flushSync(() => {
+			btn1.click();
+		});
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`<div class="container">{"text":"updated"}</div><button>update tracked state</button><button>Update prop</button>`
+		);
+
+		assert.deepEqual(log, [
+			'action $effect: ',
+			{ buttonClicked: 0 },
+			'action $effect: ',
+			{ buttonClicked: 1 }
+		]);
+	}
+});

packages/svelte/tests/runtime-runes/samples/action-state-arg-deep/log.js

@@ -0,0 +1,2 @@
+/** @type {any[]} */
+export const log = [];

packages/svelte/tests/runtime-runes/samples/action-state-arg-deep/main.svelte

@@ -0,0 +1,13 @@
+<script>
+  import Task from "./Task.svelte"
+
+  let task = $state({ text: "initial" })
+</script>
+
+<Task prop={task} />
+
+<button onclick={() => {
+  task = { text: "updated" }
+}}>
+  Update prop
+</button>