fix: measure nested transitions before applying their starting styles (#18647)

Fixes #18421 (regression from #16035, which re-broke the #4784
scenario).

Since #16035 the dummy delay animation runs with `fill: 'forwards'`, so
an intro's `t = 0` styles (for `slide`, `height: 0`) apply the moment
the transition is created and stay applied until the real animation
starts a frame later. Transition functions of other elements in the same
batch run after that and measure a collapsed DOM. With nested intros
(the TreeView from the issue), intro effects run deepest-first, so every
ancestor's `slide` captures a height where its children are pinned at 0.
On the issue's REPL every `<ul>` animates `0px -> 36px` (the height of a
single row) and then jumps to its real size when the animation finishes,
which is the "children pop out" behavior from #4784.

The fix waits one microtask before applying the initial styles and
creating the dummy, the same way deferred transitions (`crossfade`)
already work in `animate()`. Everything in the batch measures the DOM
first; the microtask still runs before the next paint, so elements never
render unstyled and the #14732 behavior is preserved. Verified in a real
browser that a delayed intro (`delay: 300`) stays held at `height: 0`
for the whole delay, including the first painted frame.

Most of the diff is the existing dummy/onfinish block moving into
`queue_micro_task`; hiding whitespace shows the actual change.
pull/18721/head
Khaled Waleed 4 days ago committed by GitHub
parent c7d8233a5c
commit 955b701df2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: measure nested transitions before applying their starting styles

@ -337,6 +337,7 @@ export function transition(flags, element, get_fn, get_params) {
*/ */
function animate(element, options, counterpart, t2, on_begin, on_finish) { function animate(element, options, counterpart, t2, on_begin, on_finish) {
var is_intro = t2 === 1; var is_intro = t2 === 1;
var aborted = false;
if (is_function(options)) { if (is_function(options)) {
// In the case of a deferred transition (such as `crossfade`), `option` will be // In the case of a deferred transition (such as `crossfade`), `option` will be
@ -344,7 +345,6 @@ function animate(element, options, counterpart, t2, on_begin, on_finish) {
// once the DOM has been updated... // once the DOM has been updated...
/** @type {Animation} */ /** @type {Animation} */
var a; var a;
var aborted = false;
queue_micro_task(() => { queue_micro_task(() => {
if (aborted) return; if (aborted) return;
@ -381,99 +381,112 @@ function animate(element, options, counterpart, t2, on_begin, on_finish) {
const { delay = 0, css, tick, easing = linear } = options; const { delay = 0, css, tick, easing = linear } = options;
var keyframes = []; /** @type {globalThis.Animation} */
var animation;
if (is_intro && counterpart === undefined) { var get_t = () => 1 - t2;
if (tick) {
tick(0, 1); // TODO put in nested effect, to avoid interleaved reads/writes?
}
if (css) { // wait a microtask before applying the initial styles and creating the dummy animation,
var styles = css_to_keyframe(css(0, 1)); // so that transitions created in the same batch (e.g. on nested elements) all measure
keyframes.push(styles, styles); // the DOM first (#18421). this still happens before the next paint, so the element
} // won't be rendered without styles applied (#14732)
} queue_micro_task(() => {
if (aborted) return;
var get_t = () => 1 - t2; var keyframes = [];
// create a dummy animation that lasts as long as the delay (but with whatever devtools if (is_intro && counterpart === undefined) {
// multiplier is in effect). in the common case that it is `0`, we keep it anyway so that if (tick) {
// the CSS keyframes aren't created until the DOM is updated tick(0, 1); // TODO put in nested effect, to avoid interleaved reads/writes?
// }
// fill forwards to prevent the element from rendering without styles applied
// see https://github.com/sveltejs/svelte/issues/14732
var animation = element.animate(keyframes, { duration: delay, fill: 'forwards' });
animation.onfinish = () => { if (css) {
// remove dummy animation from the stack to prevent conflict with main animation var styles = css_to_keyframe(css(0, 1));
animation.cancel(); keyframes.push(styles, styles);
}
}
on_begin(); // create a dummy animation that lasts as long as the delay (but with whatever devtools
// multiplier is in effect). in the common case that it is `0`, we keep it anyway so that
// the CSS keyframes aren't created until the DOM is updated
//
// fill forwards to prevent the element from rendering without styles applied
// see https://github.com/sveltejs/svelte/issues/14732
animation = element.animate(keyframes, { duration: delay, fill: 'forwards' });
// for bidirectional transitions, we start from the current position, animation.onfinish = () => {
// rather than doing a full intro/outro // remove dummy animation from the stack to prevent conflict with main animation
var t1 = counterpart?.t() ?? 1 - t2; animation.cancel();
counterpart?.abort();
var delta = t2 - t1; on_begin();
var duration = /** @type {number} */ (options.duration) * Math.abs(delta);
var keyframes = [];
if (duration > 0) { // for bidirectional transitions, we start from the current position,
/** // rather than doing a full intro/outro
* Whether or not the CSS includes `overflow: hidden`, in which case we need to var t1 = counterpart?.t() ?? 1 - t2;
* add it as an inline style to work around a Safari <18 bug counterpart?.abort();
* TODO 6.0 remove this, if possible
*/
var needs_overflow_hidden = false;
if (css) { var delta = t2 - t1;
var n = Math.ceil(duration / (1000 / 60)); // `n` must be an integer, or we risk missing the `t2` value var duration = /** @type {number} */ (options.duration) * Math.abs(delta);
var keyframes = [];
if (duration > 0) {
/**
* Whether or not the CSS includes `overflow: hidden`, in which case we need to
* add it as an inline style to work around a Safari <18 bug
* TODO 6.0 remove this, if possible
*/
var needs_overflow_hidden = false;
for (var i = 0; i <= n; i += 1) { if (css) {
var t = t1 + delta * easing(i / n); var n = Math.ceil(duration / (1000 / 60)); // `n` must be an integer, or we risk missing the `t2` value
var styles = css_to_keyframe(css(t, 1 - t));
keyframes.push(styles);
needs_overflow_hidden ||= styles.overflow === 'hidden'; for (var i = 0; i <= n; i += 1) {
var t = t1 + delta * easing(i / n);
var styles = css_to_keyframe(css(t, 1 - t));
keyframes.push(styles);
needs_overflow_hidden ||= styles.overflow === 'hidden';
}
} }
}
if (needs_overflow_hidden) { if (needs_overflow_hidden) {
/** @type {HTMLElement} */ (element).style.overflow = 'hidden'; /** @type {HTMLElement} */ (element).style.overflow = 'hidden';
} }
get_t = () => { get_t = () => {
var time = /** @type {number} */ ( var time = /** @type {number} */ (
/** @type {globalThis.Animation} */ (animation).currentTime /** @type {globalThis.Animation} */ (animation).currentTime
); );
return t1 + delta * easing(time / duration); return t1 + delta * easing(time / duration);
}; };
if (tick) { if (tick) {
loop(() => { loop(() => {
if (animation.playState !== 'running') return false; if (animation.playState !== 'running') return false;
var t = get_t(); var t = get_t();
tick(t, 1 - t); tick(t, 1 - t);
return true; return true;
}); });
}
} }
}
animation = element.animate(keyframes, { duration, fill: 'forwards' }); animation = element.animate(keyframes, { duration, fill: 'forwards' });
animation.onfinish = () => { animation.onfinish = () => {
get_t = () => t2; get_t = () => t2;
tick?.(t2, 1 - t2); tick?.(t2, 1 - t2);
on_finish(); on_finish();
};
}; };
}; });
return { return {
abort: () => { abort: () => {
aborted = true;
if (animation) { if (animation) {
animation.cancel(); animation.cancel();
// This prevents memory leaks in Chromium // This prevents memory leaks in Chromium

@ -0,0 +1,14 @@
<script>
import Nested from './Nested.svelte';
import { slide } from 'svelte/transition';
let { depth } = $props();
</script>
<div class="level-{depth}" in:slide|global={{ duration: 100 }}>
{#if depth > 0}
<Nested depth={depth - 1} />
{:else}
<div style="height: 100px">leaf</div>
{/if}
</div>

@ -0,0 +1,36 @@
import { test } from '../../assert';
export default test({
async test({ assert, target }) {
const button = target.querySelector('button');
button?.click();
// wait for the transition's keyframes to be created
const animation = await new Promise((resolve, reject) => {
const start = performance.now();
function check() {
const outer = target.querySelector('.level-2');
const animation = outer
?.getAnimations()
.find((a) => a.effect?.getTiming().duration === 100);
if (animation) {
resolve(animation);
} else if (performance.now() - start > 2000) {
reject(new Error('timed out waiting for the transition to start'));
} else {
requestAnimationFrame(check);
}
}
check();
});
// the outermost `slide` must have measured the element with its
// descendants at their natural size, not collapsed to zero by their
// own starting styles (#18421)
const keyframes = animation.effect?.getKeyframes() ?? [];
assert.equal(keyframes[keyframes.length - 1].height, '100px');
}
});

@ -0,0 +1,11 @@
<script>
import Nested from './Nested.svelte';
let visible = $state(false);
</script>
<button onclick={() => (visible = !visible)}>toggle</button>
{#if visible}
<Nested depth={2} />
{/if}
Loading…
Cancel
Save