From 02b9723e24991f8bfe02125bcbbd28829c6caf50 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Tue, 16 Jul 2024 18:10:26 +0200 Subject: [PATCH 1/2] 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 --- .changeset/wicked-carrots-explain.md | 5 ++ .../client/dom/elements/transitions.js | 81 ++++++++++++++----- 2 files changed, 65 insertions(+), 21 deletions(-) create mode 100644 .changeset/wicked-carrots-explain.md diff --git a/.changeset/wicked-carrots-explain.md b/.changeset/wicked-carrots-explain.md new file mode 100644 index 000000000000..43df35d095a1 --- /dev/null +++ b/.changeset/wicked-carrots-explain.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: ensure previous transitions are properly aborted diff --git a/packages/svelte/src/internal/client/dom/elements/transitions.js b/packages/svelte/src/internal/client/dom/elements/transitions.js index 202db6f3f558..a0aabd92dcf2 100644 --- a/packages/svelte/src/internal/client/dom/elements/transitions.js +++ b/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?.abort(); - animation = undefined; - }); + animation = animate( + this.element, + 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, () => { - dispatch_event(element, 'introend'); - intro = current_options = undefined; - }); + intro = animate( + element, + 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, () => { - dispatch_event(element, 'outroend'); - outro = current_options = undefined; - fn?.(); - }); + outro = animate( + element, + get_options(), + 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) { From dc6d9e21c796c13b9088ef4a94d5d9746b5128a6 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 16 Jul 2024 14:17:13 -0400 Subject: [PATCH 2/2] add a test --- .../transition-js-aborted-bidi/_config.js | 27 ++++++++++++++++ .../transition-js-aborted-bidi/main.svelte | 18 +++++++++++ .../_config.js | 31 +++++++++++++++++++ .../main.svelte | 29 +++++++++++++++++ 4 files changed, 105 insertions(+) create mode 100644 packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-bidi/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-bidi/main.svelte create mode 100644 packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-intro-outro/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-intro-outro/main.svelte diff --git a/packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-bidi/_config.js b/packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-bidi/_config.js new file mode 100644 index 000000000000..a41766b25a5c --- /dev/null +++ b/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 + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-bidi/main.svelte b/packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-bidi/main.svelte new file mode 100644 index 000000000000..7766fdfa7717 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-bidi/main.svelte @@ -0,0 +1,18 @@ + + +{#if visible} + hello +{/if} diff --git a/packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-intro-outro/_config.js b/packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-intro-outro/_config.js new file mode 100644 index 000000000000..efb17426981d --- /dev/null +++ b/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']); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-intro-outro/main.svelte b/packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-intro-outro/main.svelte new file mode 100644 index 000000000000..942eec968eab --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/transition-js-aborted-intro-outro/main.svelte @@ -0,0 +1,29 @@ + + +{#if visible} + hello +{/if}