fix: ensure previous transitions are properly aborted (#12460)

* fix: ensure previous transitions are properly aborted

- nullify options when the transition is aborted. This ensures a change in options is reflected the next time, else it would stick around indefinetly
- abort previous intro (if exists) when new intro plays (same for outro)

fixes #11372

* add a test

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>

.changeset/wicked-carrots-explain.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: ensure previous transitions are properly aborted

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

@@ -96,10 +96,17 @@ export function animation(element, get_fn, get_params) {
 			) {
 				const options = get_fn()(this.element, { from, to }, get_params?.());
 
-				animation = animate(this.element, options, undefined, 1, () => {
+				animation = animate(
-					animation?.abort();
+					this.element,
-					animation = undefined;
+					options,
-				});
+					undefined,
+					1,
+					() => {
+						animation?.abort();
+						animation = undefined;
+					},
+					undefined
+				);
 			}
 		},
 		fix() {
@@ -157,10 +164,11 @@ export function animation(element, get_fn, get_params) {
 export function transition(flags, element, get_fn, get_params) {
 	var is_intro = (flags & TRANSITION_IN) !== 0;
 	var is_outro = (flags & TRANSITION_OUT) !== 0;
+	var is_both = is_intro && is_outro;
 	var is_global = (flags & TRANSITION_GLOBAL) !== 0;
 
 	/** @type {'in' | 'out' | 'both'} */
-	var direction = is_intro && is_outro ? 'both' : is_intro ? 'in' : 'out';
+	var direction = is_both ? 'both' : is_intro ? 'in' : 'out';
 
 	/** @type {import('#client').AnimationConfig | ((opts: { direction: 'in' | 'out' }) => import('#client').AnimationConfig) | undefined} */
 	var current_options;
@@ -191,27 +199,54 @@ export function transition(flags, element, get_fn, get_params) {
 
 			// abort the outro to prevent overlap with the intro
 			outro?.abort();
+			// abort previous intro (can happen if an element is intro'd, then outro'd, then intro'd again)
+			intro?.abort();
 
 			if (is_intro) {
 				dispatch_event(element, 'introstart');
-				intro = animate(element, get_options(), outro, 1, () => {
+				intro = animate(
-					dispatch_event(element, 'introend');
+					element,
-					intro = current_options = undefined;
+					get_options(),
-				});
+					outro,
+					1,
+					() => {
+						dispatch_event(element, 'introend');
+						intro = current_options = undefined;
+					},
+					is_both
+						? undefined
+						: () => {
+								intro = current_options = undefined;
+							}
+				);
 			} else {
 				reset?.();
 			}
 		},
 		out(fn) {
+			// abort previous outro (can happen if an element is outro'd, then intro'd, then outro'd again)
+			outro?.abort();
+
 			if (is_outro) {
 				element.inert = true;
 
 				dispatch_event(element, 'outrostart');
-				outro = animate(element, get_options(), intro, 0, () => {
+				outro = animate(
-					dispatch_event(element, 'outroend');
+					element,
-					outro = current_options = undefined;
+					get_options(),
-					fn?.();
+					intro,
-				});
+					0,
+					() => {
+						dispatch_event(element, 'outroend');
+						outro = current_options = undefined;
+						fn?.();
+					},
+					is_both
+						? undefined
+						: () => {
+								outro = current_options = undefined;
+							}
+				);
 
 				// TODO arguably the outro should never null itself out until _all_ outros for this effect have completed...
 				// in that case we wouldn't need to store `reset` separately
@@ -263,10 +298,11 @@ export function transition(flags, element, get_fn, get_params) {
  * @param {import('#client').AnimationConfig | ((opts: { direction: 'in' | 'out' }) => import('#client').AnimationConfig)} options
  * @param {import('#client').Animation | undefined} counterpart The corresponding intro/outro to this outro/intro
  * @param {number} t2 The target `t` value — `1` for intro, `0` for outro
- * @param {(() => void) | undefined} callback
+ * @param {(() => void) | undefined} on_finish Called after successfully completing the animation
+ * @param {(() => void) | undefined} on_abort Called if the animation is aborted
  * @returns {import('#client').Animation}
  */
-function animate(element, options, counterpart, t2, callback) {
+function animate(element, options, counterpart, t2, on_finish, on_abort) {
 	var is_intro = t2 === 1;
 
 	if (is_function(options)) {
@@ -278,7 +314,7 @@ function animate(element, options, counterpart, t2, callback) {
 
 		queue_micro_task(() => {
 			var o = options({ direction: is_intro ? 'in' : 'out' });
-			a = animate(element, o, counterpart, t2, callback);
+			a = animate(element, o, counterpart, t2, on_finish, on_abort);
 		});
 
 		// ...but we want to do so without using `async`/`await` everywhere, so
@@ -294,7 +330,7 @@ function animate(element, options, counterpart, t2, callback) {
 	counterpart?.deactivate();
 
 	if (!options?.duration) {
-		callback?.();
+		on_finish?.();
 		return {
 			abort: noop,
 			deactivate: noop,
@@ -319,6 +355,7 @@ function animate(element, options, counterpart, t2, callback) {
 	var task;
 
 	if (css) {
+		// run after a micro task so that all transitions that are lining up and are about to run can correctly measure the DOM
 		queue_micro_task(() => {
 			// WAAPI
 			var keyframes = [];
@@ -349,7 +386,7 @@ function animate(element, options, counterpart, t2, callback) {
 
 			animation.finished
 				.then(() => {
-					callback?.();
+					on_finish?.();
 
 					if (t2 === 1) {
 						animation.cancel();
@@ -376,7 +413,7 @@ function animate(element, options, counterpart, t2, callback) {
 		task = loop((now) => {
 			if (now >= end) {
 				tick?.(t2, 1 - t2);
-				callback?.();
+				on_finish?.();
 				return false;
 			}
 
@@ -393,9 +430,11 @@ function animate(element, options, counterpart, t2, callback) {
 		abort: () => {
 			animation?.cancel();
 			task?.abort();
+			on_abort?.();
 		},
 		deactivate: () => {
-			callback = undefined;
+			on_finish = undefined;
+			on_abort = undefined;
 		},
 		reset: () => {
 			if (t2 === 0) {

packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-bidi/_config.js

@@ -0,0 +1,27 @@
+import { test } from '../../test';
+
+export default test({
+	get props() {
+		return { visible: false };
+	},
+
+	test({ assert, component, target, raf, logs }) {
+		component.visible = true;
+		const span = /** @type {HTMLSpanElement & { foo: number }} */ (target.querySelector('span'));
+
+		raf.tick(50);
+		assert.equal(span.foo, 0.5);
+
+		component.visible = false;
+		assert.equal(span.foo, 0.5);
+
+		raf.tick(75);
+		assert.equal(span.foo, 0.25);
+
+		component.visible = true;
+		raf.tick(100);
+		assert.equal(span.foo, 0.5);
+
+		assert.deepEqual(logs, ['transition']); // should only run once
+	}
+});

packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-bidi/main.svelte

@@ -0,0 +1,18 @@
+<script>
+	export let visible;
+
+	function foo(node) {
+		console.log('transition');
+
+		return {
+			duration: 100,
+			tick: (t) => {
+				node.foo = t;
+			}
+		};
+	}
+</script>
+
+{#if visible}
+	<span transition:foo>hello</span>
+{/if}

packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-intro-outro/_config.js

@@ -0,0 +1,31 @@
+import { test } from '../../test';
+
+export default test({
+	get props() {
+		return { visible: false };
+	},
+
+	test({ assert, component, target, raf, logs }) {
+		component.visible = true;
+		const span = /** @type {HTMLSpanElement & { foo: number, bar: number }} */ (
+			target.querySelector('span')
+		);
+
+		raf.tick(50);
+		assert.equal(span.foo, 0.5);
+
+		component.visible = false;
+		assert.equal(span.foo, 0.5);
+
+		raf.tick(75);
+		assert.equal(span.foo, 0.75);
+		assert.equal(span.bar, 0.75);
+
+		component.visible = true;
+		raf.tick(100);
+		assert.equal(span.foo, 0.25);
+		assert.equal(span.bar, 1);
+
+		assert.deepEqual(logs, ['in', 'out', 'in']);
+	}
+});

packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-intro-outro/main.svelte

@@ -0,0 +1,29 @@
+<script>
+	export let visible;
+
+	function foo(node) {
+		console.log('in');
+
+		return {
+			duration: 100,
+			tick: (t) => {
+				node.foo = t;
+			}
+		};
+	}
+
+	function bar(node) {
+		console.log('out');
+
+		return {
+			duration: 100,
+			tick: (t) => {
+				node.bar = t;
+			}
+		};
+	}
+</script>
+
+{#if visible}
+	<span in:foo out:bar>hello</span>
+{/if}