fix: don't execute attachments and attribute effects eagerly

.changeset/curvy-clouds-cut.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: don't execute attachments and attribute effects eagerly

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

@@ -2,6 +2,15 @@
 export const DERIVED = 1 << 1;
 export const EFFECT = 1 << 2;
 export const RENDER_EFFECT = 1 << 3;
+/**
+ * An effect that does not destroy its child effects when it reruns.
+ * Runs as part of render effects, i.e. not eagerly as part of tree traversal or effect flushing.
+ */
+export const MANAGED_EFFECT = 1 << 24;
+/**
+ * An effect that does not destroy its child effects when it reruns (like MANAGED_EFFECT).
+ * Runs eagerly as part of tree traversal or effect flushing.
+ */
 export const BLOCK_EFFECT = 1 << 4;
 export const BRANCH_EFFECT = 1 << 5;
 export const ROOT_EFFECT = 1 << 6;

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

@@ -1,5 +1,5 @@
 /** @import { Effect } from '#client' */
-import { block, branch, effect, destroy_effect } from '../../reactivity/effects.js';
+import { branch, effect, destroy_effect, managed } from '../../reactivity/effects.js';
 
 // TODO in 6.0 or 7.0, when we remove legacy mode, we can simplify this by
 // getting rid of the block/branch stuff and just letting the effect rip.
@@ -16,7 +16,7 @@ export function attach(node, get_fn) {
 	/** @type {Effect | null} */
 	var e;
 
-	block(() => {
+	managed(() => {
 		if (fn !== (fn = get_fn())) {
 			if (e) {
 				destroy_effect(e);

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

@@ -20,7 +20,7 @@ import { clsx } from '../../../shared/attributes.js';
 import { set_class } from './class.js';
 import { set_style } from './style.js';
 import { ATTACHMENT_KEY, NAMESPACE_HTML, UNINITIALIZED } from '../../../../constants.js';
-import { block, branch, destroy_effect, effect } from '../../reactivity/effects.js';
+import { branch, destroy_effect, effect, managed } from '../../reactivity/effects.js';
 import { init_select, select_option } from './bindings/select.js';
 import { flatten } from '../../reactivity/async.js';
 
@@ -508,7 +508,7 @@ export function attribute_effect(
 		var is_select = element.nodeName === 'SELECT';
 		var inited = false;
 
-		block(() => {
+		managed(() => {
 			var next = fn(...values.map(get));
 			/** @type {Record<string | symbol, any>} */
 			var current = set_attributes(

packages/svelte/src/internal/client/reactivity/batch.js

@@ -17,7 +17,8 @@ import {
 	EAGER_EFFECT,
 	HEAD_EFFECT,
 	ERROR_VALUE,
-	WAS_MARKED
+	WAS_MARKED,
+	MANAGED_EFFECT
 } from '#client/constants';
 import { async_mode_flag } from '../../flags/index.js';
 import { deferred, define_property } from '../../shared/utils.js';
@@ -234,7 +235,7 @@ export class Batch {
 					effect.f ^= CLEAN;
 				} else if ((flags & EFFECT) !== 0) {
 					target.effects.push(effect);
-				} else if (async_mode_flag && (flags & RENDER_EFFECT) !== 0) {
+				} else if (async_mode_flag && (flags & (RENDER_EFFECT | MANAGED_EFFECT)) !== 0) {
 					target.render_effects.push(effect);
 				} else if (is_dirty(effect)) {
 					if ((effect.f & BLOCK_EFFECT) !== 0) target.block_effects.push(effect);
@@ -779,7 +780,7 @@ function mark_effects(value, sources, marked, checked) {
 				mark_effects(/** @type {Derived} */ (reaction), sources, marked, checked);
 			} else if (
 				(flags & (ASYNC | BLOCK_EFFECT)) !== 0 &&
-				(flags & DIRTY) === 0 && // we may have scheduled this one already
+				(flags & DIRTY) === 0 &&
 				depends_on(reaction, sources, checked)
 			) {
 				set_signal_status(reaction, DIRTY);

packages/svelte/src/internal/client/reactivity/effects.js

@@ -33,7 +33,8 @@ import {
 	STALE_REACTION,
 	USER_EFFECT,
 	ASYNC,
-	CONNECTED
+	CONNECTED,
+	MANAGED_EFFECT
 } from '#client/constants';
 import * as e from '../errors.js';
 import { DEV } from 'esm-env';
@@ -401,6 +402,18 @@ export function block(fn, flags = 0) {
 	return effect;
 }
 
+/**
+ * @param {(() => void)} fn
+ * @param {number} flags
+ */
+export function managed(fn, flags = 0) {
+	var effect = create_effect(MANAGED_EFFECT | flags, fn, true);
+	if (DEV) {
+		effect.dev_stack = dev_stack;
+	}
+	return effect;
+}
+
 /**
  * @param {(() => void)} fn
  */

packages/svelte/src/internal/client/reactivity/sources.js

@@ -363,10 +363,8 @@ function mark_reactions(signal, status) {
 				mark_reactions(derived, MAYBE_DIRTY);
 			}
 		} else if (not_dirty) {
-			if ((flags & BLOCK_EFFECT) !== 0) {
-				if (eager_block_effects !== null) {
-					eager_block_effects.add(/** @type {Effect} */ (reaction));
-				}
+			if ((flags & BLOCK_EFFECT) !== 0 && eager_block_effects !== null) {
+				eager_block_effects.add(/** @type {Effect} */ (reaction));
 			}
 
 			schedule_effect(/** @type {Effect} */ (reaction));

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

@@ -21,7 +21,8 @@ import {
 	REACTION_IS_UPDATING,
 	STALE_REACTION,
 	ERROR_VALUE,
-	WAS_MARKED
+	WAS_MARKED,
+	MANAGED_EFFECT
 } from './constants.js';
 import { old_values } from './reactivity/sources.js';
 import {
@@ -421,7 +422,7 @@ export function update_effect(effect) {
 	}
 
 	try {
-		if ((flags & BLOCK_EFFECT) !== 0) {
+		if ((flags & (BLOCK_EFFECT | MANAGED_EFFECT)) !== 0) {
 			destroy_block_effect_children(effect);
 		} else {
 			destroy_effect_children(effect);

packages/svelte/tests/runtime-runes/samples/async-fork-attributes/_config.js

@@ -0,0 +1,60 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target }) {
+		const [fork, commit] = target.querySelectorAll('button');
+
+		fork.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>fork</button>
+			<button>commit</button>
+			<p style="">foo</p>
+			<p style="">foo</p>
+			<p>foo</p>
+		`
+		);
+
+		commit.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>fork</button>
+			<button>commit</button>
+			<p style="color: red;">foo</p>
+			<p style="color: red;" data-attached=true>foo</p>
+			<p data-attached=true>foo</p>
+		`
+		);
+
+		fork.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>fork</button>
+			<button>commit</button>
+			<p style="color: red;">foo</p>
+			<p style="color: red;" data-attached=true>foo</p>
+			<p data-attached=true>foo</p>
+		`
+		);
+
+		commit.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>fork</button>
+			<button>commit</button>
+			<p style="">foo</p>
+			<p style="">foo</p>
+			<p>foo</p>
+		`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-fork-attributes/main.svelte

@@ -0,0 +1,28 @@
+<script>
+	import { fork } from "svelte";
+	import { createAttachmentKey } from "svelte/attachments";
+
+	let style = $state('');
+	let attach = $state(undefined);
+
+	let forked;
+</script>
+
+<button onclick={()=>{
+	forked = fork(()=>{
+		style = style ? '' : 'color: red';
+		attach = attach ? undefined : (node) => {
+			node.setAttribute('data-attached', 'true');
+			return () => node.removeAttribute('data-attached');
+		};
+	})
+}}>fork</button>
+
+<button onclick={()=>{
+	forked.commit();
+}}>commit</button>
+
+<!-- force $.attribute_effect, which uses a block effect -->
+<p {...{style}}>foo</p>
+<p {...{style, [createAttachmentKey()]: attach}}>foo</p>
+<p {@attach attach}>foo</p>