fix: improve action support for nested $effect

Author: trueadm

.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,5 +1,6 @@
 import { effect } from '../../reactivity/effects.js';
-import { deep_read_state, untrack } from '../../runtime.js';
+import { source } from '../../reactivity/sources.js';
+import { deep_read_state, get, untrack, update } from '../../runtime.js';
 
 /**
  * @template P
@@ -11,33 +12,54 @@ import { deep_read_state, untrack } from '../../runtime.js';
 export function action(dom, action, value_fn) {
 	/** @type {undefined | import('#client').ActionPayload<P>} */
 	var payload = undefined;
-	var needs_deep_read = false;
+	var version = source(0);
+	/**
+	 * @type {P}
+	 */
+	var value;
+	var init = false;
+
+	// The value needs to be calculated independent of the action body, as we only want to evaluate
+	// the payload of the action once – not when the value changes.
+	effect(() => {
+		get(version);
+		if (value_fn) {
+			value = value_fn();
+		}
+		if (payload !== undefined) {
+			var update = payload.update;
+			if (typeof update === 'function') {
+				update(value);
+			}
+		}
+	});
 
 	// 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 value = value_fn();
-			untrack(() => {
+		untrack(() => {
+			if (value_fn) {
 				if (payload === undefined) {
 					payload = action(dom, value) || {};
-					needs_deep_read = !!payload?.update;
-				} else {
-					var update = payload.update;
-					if (typeof update === 'function') {
-						update(value);
+					if (payload?.update) {
+						// 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.
+						effect(() => {
+							deep_read_state(value);
+							if (init) {
+								untrack(() => {
+									update(version, 1);
+								});
+							}
+							init = true;
+						});
 					}
 				}
-			});
-			// 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 {
+				payload = action(dom);
 			}
-		} else {
-			untrack(() => (payload = action(dom)));
-		}
+		});
 	});
 
 	effect(() => {

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>