fix: prevent flushed effects from running again (#17787)

We never cleared the list of (maybe)dirty_effects on the assumption that
once a batch has run them it's complete. But that's not the case when a
boundary has a pending snippet, in which case the pending snippet shows
up, so `blocking_pending` is already 0 and effects are flushed. That can
lead to effects being run unnecessarily, even leading to infinite loops.

So we clear them. This is safe because any additional effects would
either be scheduled by the boundary (which keeps track of the offscreen
effects created while the pending snippet is shown, and schedules them
once the pending snippet goes away) or by unskipping skipped branches
(which reschedules the effects inside it)

Fixes #17717

After creating the test I noticed it fails when run together with other
tests, but not alone, which lead me to discover that we're missing an
`unset_context`. I also added clearing of `#skipped_branches` just to be
safe.
pull/17786/head
Simon H 5 months ago committed by GitHub
parent f6e8b1d11e
commit 1b3e660519
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: prevent flushed effects from running again

@ -248,6 +248,7 @@ export function run(thunks) {
promise.finally(() => {
blocker.settled = true;
unset_context();
});
for (const fn of thunks.slice(1)) {

@ -1,6 +1,5 @@
/** @import { Fork } from 'svelte' */
/** @import { Derived, Effect, Reaction, Source, Value } from '#client' */
/** @import { Boundary } from '../dom/blocks/boundary' */
import {
BLOCK_EFFECT,
BRANCH_EFFECT,
@ -14,7 +13,6 @@ import {
ROOT_EFFECT,
MAYBE_DIRTY,
DERIVED,
BOUNDARY_EFFECT,
EAGER_EFFECT,
HEAD_EFFECT,
ERROR_VALUE,
@ -225,6 +223,10 @@ export class Batch {
flush_queued_effects(render_effects);
flush_queued_effects(effects);
// Clear effects. Those that are still needed will be rescheduled through unskipping the skipped branches.
this.#dirty_effects.clear();
this.#maybe_dirty_effects.clear();
previous_batch = null;
this.#deferred?.resolve();
@ -423,6 +425,7 @@ export class Batch {
batch_values = previous_batch_values;
}
this.#skipped_branches.clear();
batches.delete(this);
}

@ -0,0 +1,82 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
html: `
<button>a</button>
<button>b</button>
<button>resolve a</button>
<button>resolve b</button>
<p>pending a</p>
`,
async test({ assert, target }) {
const [a, b, resolve_a, resolve_b] = target.querySelectorAll('button');
resolve_a.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>a</button>
<button>b</button>
<button>resolve a</button>
<button>resolve b</button>
<p>page a</p>
`
);
b.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>a</button>
<button>b</button>
<button>resolve a</button>
<button>resolve b</button>
<p>pending b</p>
`
);
a.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>a</button>
<button>b</button>
<button>resolve a</button>
<button>resolve b</button>
<p>pending a</p>
`
);
resolve_b.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>a</button>
<button>b</button>
<button>resolve a</button>
<button>resolve b</button>
<p>pending a</p>
`
);
resolve_a.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>a</button>
<button>b</button>
<button>resolve a</button>
<button>resolve b</button>
<p>page a</p>
`
);
await new Promise((r) => setTimeout(r, 100));
}
});

@ -0,0 +1,52 @@
<script>
let page = $state('a');
/** @type {Array<() => void>} */
const a = [];
/** @type {Array<() => void>} */
const b = [];
function gate(next) {
const deferred = Promise.withResolvers();
if (next === 'a') a.push(deferred.resolve);
else b.push(deferred.resolve);
return deferred.promise;
}
function nav(next) {
page = next;
}
const to_render = $derived(page === 'a' ? snippet_a : snippet_b);
</script>
<button onclick={() => nav('a')}>a</button>
<button onclick={() => nav('b')}>b</button>
<button onclick={() => a.shift()?.()}>resolve a</button>
<button onclick={() => b.shift()?.()}>resolve b</button>
{#snippet snippet_a()}
<svelte:boundary>
{@const _a = await gate('a')}
<p>page a</p>
{#snippet pending()}
<p>pending a</p>
{/snippet}
</svelte:boundary>
{/snippet}
{#snippet snippet_b()}
<svelte:boundary>
{@const _b = await gate('b')}
<p>page b</p>
{#snippet pending()}
<p>pending b</p>
{/snippet}
</svelte:boundary>
{/snippet}
{@render to_render()}
Loading…
Cancel
Save