fix: don't fail on flushSync while flushing effects (#16674)

* fix: don't fail on `flushSync` while flushing effects

WIP
fixes #16640

* cleanup, unrelated lint

* revert

* fix

Author: dummdidumm

.changeset/honest-coins-work.md

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

packages/svelte/elements.d.ts

@@ -1268,7 +1268,7 @@ export interface HTMLMetaAttributes extends HTMLAttributes<HTMLMetaElement> {
 	charset?: string | undefined | null;
 	content?: string | undefined | null;
 	'http-equiv'?:
-	  | 'accept-ch'
+		| 'accept-ch'
 		| 'content-security-policy'
 		| 'content-type'
 		| 'default-style'

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

@@ -187,7 +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
-		if (batches.size > 1) {
+		if (async_mode_flag && batches.size > 1) {
 			current_values = new Map();
 			batch_deriveds = new Map();
 
@@ -484,6 +484,7 @@ export class Batch {
  */
 export function flushSync(fn) {
 	if (async_mode_flag && active_effect !== null) {
+		// We disallow this because it creates super-hard to reason about stack trace and because it's generally a bad idea
 		e.flush_sync_in_effect();
 	}
 
@@ -622,7 +623,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/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,12 @@
+import { async_mode } from '../../../helpers';
+import { test } from '../../test';
+
+export default test({
+	// In legacy mode this succeeds and logs 'hello'
+	// In async mode this throws an error because flushSync is called inside an effect
+	async test({ assert, target, logs }) {
+		assert.htmlEqual(target.innerHTML, `<button>show</button> <div>hello</div>`);
+		assert.deepEqual(logs, ['hello']);
+	},
+	runtime_error: async_mode ? 'flush_sync_in_effect' : undefined
+});

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>