mirror of https://github.com/sveltejs/svelte
fix: treat batches as a linked list (#18205)
This is another attempt to address some of the tricky edge cases that
arise when async batches resolve out of order. The idea is this:
Essentially, when a batch resolves:
1. we find the latest batch it shares changes with
2. if none exists (either because this is the earliest batch, or because
it is independent of any earlier batches):
- we commit it: effects are flushed, `oncommit` callbacks are run
- we restart any async work in later batches that depends on both the
committed values and the later batch's changes
3. otherwise, we don't commit the batch. instead:
- we merge the changes from the later batch onto the earlier batch. in
some cases this may mean restarting async work on the earlier batch, but
we avoid doing so unnecessarily.
- we then `#process()` the earlier batch. if it resolves, goto 1
This feels like it ought to work. There are still two failing tests,
which I'm currently looking into.
Notable changes:
- Instead of having a `batches` set, we have a linked list. When a batch
resolves, this makes it easy to find the batch that it should be merged
into
- The `#is_deferred` logic now takes account of skipped effects —
there's no need to wait for a promise inside a falsy `if` block (this
accounts for the change in the `async-inner-after-outer` test)
- We no longer care about `#blockers`
I would like to believe that this approach will allow us to simplify and
delete some code, for example the `rebase` logic, though that remains to
be seen. It also feels like some version of #18035 would be helpful.
- closes #18189
- closes #18162
- fixes https://github.com/sveltejs/kit/issues/15431
### 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`
---------
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
pull/18217/head
Rich Harris
2 months ago
committed by
GitHub
parent
1688c84be9
commit
5122936edb
@ -0,0 +1,5 @@
|
||||
---
|
||||
"svelte": patch
|
||||
---
|
||||
|
||||
fix: replacing async 'blocking' strategy with 'merging'
|
||||
@ -0,0 +1,20 @@
|
||||
import { tick } from 'svelte';
|
||||
import { test } from '../../test';
|
||||
|
||||
export default test({
|
||||
async test({ assert, target }) {
|
||||
const [increment, pop] = target.querySelectorAll('button');
|
||||
|
||||
increment.click();
|
||||
await tick();
|
||||
increment.click();
|
||||
await tick();
|
||||
pop.click();
|
||||
await tick();
|
||||
assert.htmlEqual(target.innerHTML, `<button>increment</button> <button>pop</button> 2 2 1`);
|
||||
|
||||
pop.click();
|
||||
await tick();
|
||||
assert.htmlEqual(target.innerHTML, `<button>increment</button> <button>pop</button> 2 2 1`);
|
||||
}
|
||||
});
|
||||
@ -0,0 +1,26 @@
|
||||
<script>
|
||||
let count = $state(0);
|
||||
let other = $state(0);
|
||||
|
||||
const queue = [];
|
||||
function push(v) {
|
||||
return new Promise((r,e) => queue.push(() => v === 1 ? e(v) : r(v)));
|
||||
}
|
||||
</script>
|
||||
|
||||
<button onclick={() => {
|
||||
if (count === 0) {
|
||||
other++;
|
||||
count++;
|
||||
} else {
|
||||
count++
|
||||
}
|
||||
}}>increment</button>
|
||||
<button onclick={() => queue.pop()?.()}>pop</button>
|
||||
|
||||
{#if count > 0}
|
||||
<svelte:boundary>
|
||||
{await push(count)} {count} {other}
|
||||
{#snippet failed()}boom{/snippet}
|
||||
</svelte:boundary>
|
||||
{/if}
|
||||
@ -0,0 +1,20 @@
|
||||
import { tick } from 'svelte';
|
||||
import { test } from '../../test';
|
||||
|
||||
export default test({
|
||||
async test({ assert, target }) {
|
||||
const [increment, pop] = target.querySelectorAll('button');
|
||||
|
||||
increment.click();
|
||||
await tick();
|
||||
increment.click();
|
||||
await tick();
|
||||
pop.click();
|
||||
await tick();
|
||||
assert.htmlEqual(target.innerHTML, '<button>increment</button><button>pop</button> 2 2 1'); // showing nothing here yet would also be ok
|
||||
|
||||
pop.click();
|
||||
await tick();
|
||||
assert.htmlEqual(target.innerHTML, '<button>increment</button><button>pop</button> 2 2 1');
|
||||
}
|
||||
});
|
||||
@ -0,0 +1,23 @@
|
||||
|
||||
<script>
|
||||
let count = $state(0);
|
||||
let other = $state(0);
|
||||
|
||||
const queue = [];
|
||||
|
||||
function push(v) {
|
||||
if (v === 0) return v;
|
||||
|
||||
return new Promise((resolve) => queue.push(() => resolve(v)));
|
||||
}
|
||||
</script>
|
||||
|
||||
<button onclick={() => {
|
||||
if (count === 0) other++;
|
||||
count++;
|
||||
}}>increment</button>
|
||||
<button onclick={() => queue.pop()?.()}>pop</button>
|
||||
|
||||
{#if count > 0}
|
||||
{await push(count)} {count} {other}
|
||||
{/if}
|
||||
Loading…
Reference in new issue