mirror of https://github.com/sveltejs/svelte
This fixes the failing redirect test in SvelteKit. It consists of two parts, both needed:
- reschedule new effects: when a batch commits, it needs to tell other batches about its new effects. We had this logic already but didn't apply it often enough. To avoid overfiring we add an additional set to track which of the new effects of a batch another batch already saw
- avoid persisting stale async values: when an async derived commits a value of a later batch, and then a value of an earlier batch is committed, we're persistent a stale value. We need to avoid that to not "go back in time"
- these two make the new test in this PR pass but not the redirect test in SvelteKit. For that I had to add the hack in `deriveds.js` to also unblock on the surrounding `flatten` effect
For the first part I also had a different approach, but it was more LOC and felt a bit scary with respects to deadlocks so I discarded it. In `deriveds.js` I had this before `decrement_pending?.()`:
```ts
if (!error) {
let blocked = false;
// All prior async derived runs are now stale
for (const [b, _d] of deferreds) {
if (b.id < batch.id) {
batch.unblocked.add(effect);
if (parent !== null) {
// Terrible hack: this way we unblock ourselves from `flatten`
// which can block us on initial run
batch.unblocked.add(parent);
}
batch.oncommit(() => _d.resolve(value));
if (batch.is_blocked_by(b)) {
// We do not want to commit just yet because it means we could write a newer
// value to the source before an older value, and then the older value is the
// latest one, creating stale UI / UI that "travels backwards".
const resume = () => queue_micro_task(() => handler(value, error));
_d.promise.then(resume, resume);
b.ondiscard(resume);
blocked = true;
}
}
}
if (blocked) return;
}
```
(and the related logic further down got removed)
stale-values-async-fix
Simon Holthausen
4 days ago
parent
fcaa8ce723
commit
41ede2cbfc
@ -0,0 +1,5 @@
|
||||
---
|
||||
'svelte': patch
|
||||
---
|
||||
|
||||
fix: reschedule new effects from other branches
|
||||
@ -0,0 +1,5 @@
|
||||
---
|
||||
'svelte': patch
|
||||
---
|
||||
|
||||
fix: avoid persisting stale async values
|
||||
@ -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