fix: don't resurrect outroing elements when an ancestor block is paused and resumed (#18431)

Fixes #15604

Given two nested blocks that both transition:

```svelte
{#if fetching}
	<p>loading</p>
{:else}
	<div transition:fade>
		{#if shown}
			<div class="red" transition:fade|global>red square</div>
		{/if}
	</div>
{/if}
```

and this sequence, where each step happens before the previous fade
finished:

```js
shown = false;    // red square starts fading out
fetching = true;  // outer block starts fading out too
fetching = false; // 50ms later
```

the red square should finish fading out and be removed, since `shown` is
still false. Instead it fades back in and stays on screen: resuming the
outer block walks the whole subtree, clears `INERT` on every effect and
plays `in()` on every transition it finds — including the inner block's,
so the square's outro is aborted and the removal callback waiting on it
never runs. The inner block has no reason to re-run on its own, `shown`
never changed again.

The root issue is that `INERT` records no ownership — it can't
distinguish "paused by the ancestor currently being resumed" (revive)
from "paused by its own block for its own reasons" (leave alone). The
pause side already has this restraint: `pause_children` refuses to touch
a subtree that is already `INERT`. The resume side had nothing to check.
This PR marks the one effect `pause_effect` was actually called on — the
root of the paused subtree — with a `PAUSED` flag, and gives resume the
same restraint:

```js
function resume_children(effect, local) {
	if ((effect.f & PAUSED) !== 0) return;
```

so a resume can only ever undo its own pause; the flag is only cleared
by `resume_effect` on that exact effect. If `shown` flips back to true
while the outer block is paused, this still works: the inner block
effect carries no `PAUSED` itself, so it is resumed and rescheduled,
re-evaluates its condition and revives its own branch.
pull/17306/merge
Nic Polumeyv 3 days ago committed by GitHub
parent 78979c87c9
commit a6560bbe08
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: don't resurrect outroing elements when an ancestor block is paused and resumed

@ -15,6 +15,13 @@ export const BLOCK_EFFECT = 1 << 4;
export const BRANCH_EFFECT = 1 << 5;
export const ROOT_EFFECT = 1 << 6;
export const BOUNDARY_EFFECT = 1 << 7;
/**
* Set on the effect that `pause_effect` was called on, i.e. the root of a paused subtree,
* as opposed to its descendants which are merely `INERT`. This allows `resume_effect` on
* an ancestor to skip subtrees that were paused for their own reasons (such as a block
* whose condition is still false) rather than resurrecting them
*/
export const PAUSED = 1 << 8;
/**
* Indicates that a reaction is connected to an effect root either it is an effect,
* or it is a derived that is depended on by at least one effect. If a derived has

@ -34,7 +34,8 @@ import {
ASYNC,
CONNECTED,
MANAGED_EFFECT,
DESTROYING
DESTROYING,
PAUSED
} from '#client/constants';
import { invoke_error_boundary } from '../error-handling.js';
import * as e from '../errors.js';
@ -616,6 +617,7 @@ export function pause_effect(effect, callback, destroy = true) {
/** @type {TransitionManager[]} */
var transitions = [];
effect.f |= PAUSED;
pause_children(effect, transitions, true);
var fn = () => {
@ -683,6 +685,7 @@ function pause_children(effect, transitions, local) {
* @param {Effect} effect
*/
export function resume_effect(effect) {
effect.f &= ~PAUSED;
resume_children(effect, true);
}
@ -691,6 +694,10 @@ export function resume_effect(effect) {
* @param {boolean} local
*/
function resume_children(effect, local) {
// this subtree was paused for its own reasons (e.g. a block whose condition
// is still false) — its controller will resume or destroy it
if ((effect.f & PAUSED) !== 0) return;
if ((effect.f & INERT) === 0) return;
effect.f ^= INERT;

@ -0,0 +1,23 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
// #15604 — a removed each item mid-outro must not be resurrected by an ancestor pause/resume
export default test({
test({ assert, target, raf }) {
const [remove, fetch] = target.querySelectorAll('button');
raf.tick(200);
assert.equal(target.querySelectorAll('.item').length, 3);
flushSync(() => remove.click());
flushSync(() => fetch.click());
raf.tick(30);
flushSync(() => fetch.click());
raf.tick(2000);
const items = [...target.querySelectorAll('.item')].map((el) => el.textContent);
assert.deepEqual(items, ['a', 'c']);
}
});

@ -0,0 +1,19 @@
<script>
import { fade } from 'svelte/transition';
let fetching = $state(false);
let items = $state(['a', 'b', 'c']);
</script>
<button onclick={() => (items = items.filter((i) => i !== 'b'))}>remove</button>
<button onclick={() => (fetching = !fetching)}>fetch</button>
{#if fetching}
<p>loading</p>
{:else}
<div transition:fade={{ duration: 100 }}>
{#each items as item (item)}
<div class="item" transition:fade|global={{ duration: 100 }}>{item}</div>
{/each}
</div>
{/if}

@ -0,0 +1,24 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
// #15604 companion — flipping the condition back while the ancestor is paused must still revive the element
export default test({
test({ assert, target, raf }) {
const [toggle, fetch] = target.querySelectorAll('button');
raf.tick(200);
assert.ok(target.querySelector('.red'));
flushSync(() => toggle.click());
flushSync(() => fetch.click());
raf.tick(30);
flushSync(() => toggle.click());
flushSync(() => fetch.click());
raf.tick(2000);
assert.ok(target.querySelector('.red'));
}
});

@ -0,0 +1,19 @@
<script>
import { fade } from 'svelte/transition';
let fetching = $state(false);
let shown = $state(true);
</script>
<button onclick={() => (shown = !shown)}>toggle</button>
<button onclick={() => (fetching = !fetching)}>fetch</button>
{#if fetching}
<p>loading</p>
{:else}
<div transition:fade={{ duration: 100 }}>
{#if shown}
<div class="red" transition:fade|global={{ duration: 100 }}>red</div>
{/if}
</div>
{/if}

@ -0,0 +1,29 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
// #15604 — an element mid-outro must not be resurrected when an ancestor
// block is paused and resumed while the element's own condition is still false.
// The outer `transition:fade` matters: it keeps the outer branch alive (pending
// its own outro) so that toggling back takes the resume path
export default test({
test({ assert, target, raf }) {
const [hide, fetch] = target.querySelectorAll('button');
// let the mount intro (dom variant) finish
raf.tick(200);
assert.ok(target.querySelector('.red'));
// start the outro of the inner block
flushSync(() => hide.click());
// pause the outer block while the outro is in flight...
flushSync(() => fetch.click());
raf.tick(250);
// ...then resume it before either outro completes
flushSync(() => fetch.click());
raf.tick(2000);
assert.equal(target.querySelector('.red'), null);
}
});

@ -0,0 +1,19 @@
<script>
import { fade } from 'svelte/transition';
let fetching = $state(false);
let shown = $state(true);
</script>
<button onclick={() => (shown = false)}>hide</button>
<button onclick={() => (fetching = !fetching)}>fetch</button>
{#if fetching}
<p>loading</p>
{:else}
<div transition:fade={{ duration: 100 }}>
{#if shown}
<div class="red" transition:fade|global={{ duration: 100 }}>red</div>
{/if}
</div>
{/if}
Loading…
Cancel
Save