fix: don't cancel transition of already outroing elements

#16977 forgot one detail: While an element is outroing, the block of e.g. an if block can be triggered again, resolving to the same condition. In that case we have an in-between state where the element is still onscreen but already outroing. We have to detect this to not eagerly destroy the corresponding effect when we arrive at the outro/destroy logic.

Fixes #16982

.changeset/shiny-otters-learn.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: don't cancel transition of already outroing elements

packages/svelte/src/internal/client/dom/blocks/branches.js

@@ -24,12 +24,35 @@ export class BranchManager {
 	/** @type {Map<Batch, Key>} */
 	#batches = new Map();
 
-	/** @type {Map<Key, Effect>} */
+	/**
+	 * Map of keys to effects that are currently rendered in the DOM.
+	 * These effects are visible and actively part of the document tree.
+	 * Example:
+	 * ```
+	 * {#if condition}
+	 * 	foo
+	 * {:else}
+	 * 	bar
+	 * {/if}
+	 * ```
+	 * Can result in the entries `true->Effect` and `false->Effect`
+	 * @type {Map<Key, Effect>}
+	 */
 	#onscreen = new Map();
 
-	/** @type {Map<Key, Branch>} */
+	/**
+	 * Similar to #onscreen with respect to the keys, but contains branches that are not yet
+	 * in the DOM, because their insertion is deferred.
+	 * @type {Map<Key, Branch>}
+	 */
 	#offscreen = new Map();
 
+	/**
+	 * Keys of effects that are currently outroing
+	 * @type {Set<Key>}
+	 */
+	#outroing = new Set();
+
 	/**
 	 * Whether to pause (i.e. outro) on change, or destroy immediately.
 	 * This is necessary for `<svelte:element>`
@@ -96,7 +119,8 @@ export class BranchManager {
 		// outro/destroy all onscreen effects...
 		for (const [k, effect] of this.#onscreen) {
 			// ...except the one that was just committed
-			if (k === key) continue;
+			//    or those that are already outroing (else the transition is aborted and the effect destroyed right away)
+			if (k === key || this.#outroing.has(k)) continue;
 
 			const on_destroy = () => {
 				const keys = Array.from(this.#batches.values());
@@ -113,10 +137,12 @@ export class BranchManager {
 					destroy_effect(effect);
 				}
 
+				this.#outroing.delete(k);
 				this.#onscreen.delete(k);
 			};
 
 			if (this.#transition || !onscreen) {
+				this.#outroing.add(k);
 				pause_effect(effect, on_destroy, false);
 			} else {
 				on_destroy();

packages/svelte/tests/runtime-runes/samples/transition-if/_config.js

@@ -0,0 +1,28 @@
+import { flushSync } from '../../../../src/index-client.js';
+import { test } from '../../test';
+
+export default test({
+	test({ assert, raf, target }) {
+		const [x, y] = target.querySelectorAll('button');
+
+		// Set second part of condition to false first...
+		y.click();
+		flushSync();
+		raf.tick(50);
+		assert.htmlEqual(
+			target.innerHTML,
+			'<button>toggle x</button> <button>toggle y</button> <p foo="0.5">hello</p>'
+		);
+
+		// ...so that when both are toggled the block condition runs again
+		x.click();
+		flushSync();
+		assert.htmlEqual(
+			target.innerHTML,
+			'<button>toggle x</button> <button>toggle y</button> <p foo="0.5">hello</p>'
+		);
+
+		raf.tick(100);
+		assert.htmlEqual(target.innerHTML, '<button>toggle x</button> <button>toggle y</button>');
+	}
+});

packages/svelte/tests/runtime-runes/samples/transition-if/main.svelte

@@ -0,0 +1,24 @@
+<script>
+	function foo(node) {
+		return {
+			duration: 100,
+			tick: (t, u) => {
+				node.setAttribute('foo', t)
+			}
+		};
+	}
+
+	let x = $state(true);
+	let y = $state(true);
+</script>
+
+<button onclick={() => {
+	x = !x;
+}}>toggle x</button>
+<button onclick={() => {
+	y = !y;
+}}>toggle y</button>
+
+{#if x && y}
+	<p transition:foo>hello</p>
+{/if}