fix: discard batches made obsolete by commit (#17934)

Batches that are made stale (because of a `STALE_REACTION`) can end up
sticking around indefinitely, forcing every subsequent batch into
time-traveling mode and causing incorrect `previous` values to be
rendered.

This partially fixes it, by discarding any older batches that are
subsets of a batch currently being committed. It's not a complete fix,
though — if an earlier batch is stale but is _not_ a subset of the
committed batch, it becomes a zombie, and its changes will never be
applied. Haven't quite figured out how to think about that yet.

### Before submitting the PR, please make sure you do the following

- [x] It's really useful if your PR references an issue where it is
discussed ahead of time. In many cases, features are absent for a
reason. For large changes, please create an RFC:
https://github.com/sveltejs/rfcs
- [x] Prefix your PR title with `feat:`, `fix:`, `chore:`, or `docs:`.
- [x] This message body should clearly illustrate what problems it
solves.
- [x] Ideally, include a test that fails without this PR but passes with
it.
- [x] If this PR changes code within `packages/svelte/src`, add a
changeset (`npx changeset`).

### Tests and linting

- [x] Run the tests with `pnpm test` and lint the project with `pnpm
lint`
pull/17600/merge
Rich Harris 6 days ago committed by GitHub
parent 8b86bdd82d
commit 98e8b635fa
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: discard batches made obsolete by commit

@ -438,6 +438,8 @@ export class Batch {
discard() {
for (const fn of this.#discard_callbacks) fn(this);
this.#discard_callbacks.clear();
batches.delete(this);
}
#commit() {
@ -466,13 +468,15 @@ export class Batch {
sources.push(source);
}
if (sources.length === 0) {
continue;
}
// Re-run async/block effects that depend on distinct values changed in both batches
var others = [...batch.current.keys()].filter((s) => !this.current.has(s));
if (others.length > 0) {
if (others.length === 0) {
if (is_earlier) {
// this batch is now obsolete and can be discarded
batch.discard();
}
} else if (sources.length > 0) {
if (DEV) {
invariant(batch.#roots.length === 0, 'Batch has scheduled roots');
}
@ -1095,7 +1099,6 @@ export function fork(fn) {
}
if (!committed && batches.has(batch)) {
batches.delete(batch);
batch.discard();
}
}

@ -0,0 +1,89 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
async test({ assert, target }) {
await tick();
const [increment, shift, pop] = target.querySelectorAll('button');
assert.htmlEqual(
target.innerHTML,
`
<button>1</button>
<button>shift</button>
<button>pop</button>
<p>1 = 1</p>
`
);
increment.click();
await tick();
increment.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>3</button>
<button>shift</button>
<button>pop</button>
<p>1 = 1</p>
`
);
shift.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>3</button>
<button>shift</button>
<button>pop</button>
<p>1 = 1</p>
`
);
shift.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>3</button>
<button>shift</button>
<button>pop</button>
<p>3 = 3</p>
`
);
increment.click();
await tick();
increment.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>5</button>
<button>shift</button>
<button>pop</button>
<p>3 = 3</p>
`
);
pop.click();
await tick();
assert.htmlEqual(
target.innerHTML,
`
<button>5</button>
<button>shift</button>
<button>pop</button>
<p>5 = 5</p>
`
);
}
});

@ -0,0 +1,36 @@
<script>
import { getAbortSignal } from 'svelte';
const queue = [];
function push(value) {
if (value === 1) return 1;
const d = Promise.withResolvers();
queue.push(() => d.resolve(value));
const signal = getAbortSignal();
signal.onabort = () => d.reject(signal.reason);
return d.promise;
}
function shift() {
queue.shift()?.();
}
function pop() {
queue.pop()?.();
}
let n = $state(1);
</script>
<button onclick={() => n++}>
{$state.eager(n)}
</button>
<button onclick={shift}>shift</button>
<button onclick={pop}>pop</button>
<p>{n} = {await push(n)}</p>
Loading…
Cancel
Save