fix: don't fail on flushSync while flushing effects

WIP
fixes #16640

Author: dummdidumm

.changeset/honest-coins-work.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: don't fail on `flushSync` while flushing effects

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

@@ -187,6 +187,7 @@ export class Batch {
 		// if there are multiple batches, we are 'time travelling' —
 		// we need to undo the changes belonging to any batch
 		// other than the current one
+		// TODO this is wrong when flushSync is called while another batch is active, can be two when there's actually no time traveling
 		if (batches.size > 1) {
 			current_values = new Map();
 			batch_deriveds = new Map();
@@ -484,7 +485,8 @@ export class Batch {
  */
 export function flushSync(fn) {
 	if (async_mode_flag && active_effect !== null) {
-		e.flush_sync_in_effect();
+		// TODO why do we disallow this?
+		// e.flush_sync_in_effect();
 	}
 
 	var was_flushing_sync = is_flushing_sync;
@@ -622,7 +624,9 @@ function flush_queued_effects(effects) {
 				}
 			}
 
-			if (eager_block_effects.length > 0) {
+			// If update_effect() has a flushSync() in it, we may have flushed another flush_queued_effects(),
+			// which already handled this logic and did set eager_block_effects to null.
+			if (eager_block_effects?.length > 0) {
 				// TODO this feels incorrect! it gets the tests passing
 				old_values.clear();
 

packages/svelte/src/legacy/legacy-client.js

@@ -11,7 +11,6 @@ import * as w from '../internal/client/warnings.js';
 import { DEV } from 'esm-env';
 import { FILENAME } from '../constants.js';
 import { component_context, dev_current_component_function } from '../internal/client/context.js';
-import { async_mode_flag } from '../internal/flags/index.js';
 
 /**
  * Takes the same options as a Svelte 4 component and the component function and returns a Svelte 4 compatible component.
@@ -121,9 +120,8 @@ class Svelte4Component {
 			recover: options.recover
 		});
 
-		// We don't flushSync for custom element wrappers or if the user doesn't want it,
-		// or if we're in async mode since `flushSync()` will fail
-		if (!async_mode_flag && (!options?.props?.$$host || options.sync === false)) {
+		// We don't flushSync for custom element wrappers or if the user doesn't want it
+		if (!options?.props?.$$host || options.sync === false) {
 			flushSync();
 		}
 

packages/svelte/tests/runtime-runes/samples/flush-sync-inside-attachment/Child.svelte

@@ -0,0 +1,7 @@
+<script>
+    let { text } = $props();
+
+    $effect(() => console.log(text));
+</script>
+
+{text}

packages/svelte/tests/runtime-runes/samples/flush-sync-inside-attachment/_config.js

@@ -0,0 +1,8 @@
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target, logs }) {
+		assert.htmlEqual(target.innerHTML, `<button>show</button> <div>hello</div>`);
+		assert.deepEqual(logs, ['hello']);
+	}
+});

packages/svelte/tests/runtime-runes/samples/flush-sync-inside-attachment/main.svelte

@@ -0,0 +1,13 @@
+<script>
+	import { flushSync, mount } from 'svelte'
+	import Child from './Child.svelte';
+
+	let show = $state(false);
+</script>
+
+<button onclick={() => show = true}>show</button>
+
+<div {@attach (target) => {
+	mount(Child, { target, props: { text: 'hello' }  });
+	flushSync();
+}}></div>