fix: reschedule new effects in prior batches (#18021)

If a batch creates a new branch (e.g. through an if block becoming true)
the previous batches so far do not know about the new effects created
through that. This can lead to stale values being shown. We therefore
schedule those new effects on prior batches if they are touched by a
`current` value of that batch

Fixes #17099

extracted from #17971
pull/18025/head
Simon H 3 months ago committed by GitHub
parent 669f6b45a3
commit a9d8439ad1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: reschedule new effects in prior batches

@ -145,6 +145,12 @@ export class Batch {
*/
#roots = [];
/**
* Effects created while this batch was active.
* @type {Effect[]}
*/
#new_effects = [];
/**
* Deferred effects (which run after async work has completed) that are DIRTY
* @type {Set<Effect>}
@ -472,6 +478,13 @@ export class Batch {
batches.delete(this);
}
/**
* @param {Effect} effect
*/
register_created_effect(effect) {
this.#new_effects.push(effect);
}
#commit() {
// If there are other pending batches, they now need to be 'rebased' —
// in other words, we re-run block/async effects with the newly
@ -525,6 +538,25 @@ export class Batch {
mark_effects(source, others, marked, checked);
}
checked = new Map();
var current_unequal = [...batch.current.keys()].filter((c) =>
this.current.has(c) ? /** @type {[any, boolean]} */ (this.current.get(c))[0] !== c : true
);
for (const effect of this.#new_effects) {
if (
(effect.f & (DESTROYED | INERT | EAGER_EFFECT)) === 0 &&
depends_on(effect, current_unequal, checked)
) {
if ((effect.f & (ASYNC | BLOCK_EFFECT)) !== 0) {
set_signal_status(effect, DIRTY);
batch.schedule(effect);
} else {
batch.#dirty_effects.add(effect);
}
}
}
// Only apply and traverse when we know we triggered async work with marking the effects
if (batch.#roots.length > 0) {
batch.apply();

@ -42,7 +42,7 @@ import { DEV } from 'esm-env';
import { define_property } from '../../shared/utils.js';
import { get_next_sibling } from '../dom/operations.js';
import { component_context, dev_current_component_function, dev_stack } from '../context.js';
import { Batch, collected_effects } from './batch.js';
import { Batch, collected_effects, current_batch } from './batch.js';
import { flatten, increment_pending } from './async.js';
import { without_reactive_context } from '../dom/elements/bindings/shared.js';
import { set_signal_status } from './status.js';
@ -120,6 +120,8 @@ function create_effect(type, fn) {
effect.component_function = dev_current_component_function;
}
current_batch?.register_created_effect(effect);
/** @type {Effect | null} */
var e = effect;

@ -2,7 +2,6 @@ import { tick } from 'svelte';
import { test } from '../../test';
export default test({
skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971
async test({ assert, target, logs }) {
const [x, y, resolve] = target.querySelectorAll('button');
@ -17,12 +16,20 @@ export default test({
<button>x</button>
<button>y++</button>
<button>resolve</button>
` // if this shows world world - that would also be ok
world
` // if this does not show world - that would also be ok
);
resolve.click();
await tick();
assert.deepEqual(logs, ['universe', 'universe', '$effect: universe', '$effect: universe']);
assert.deepEqual(logs, [
'universe',
'world',
'$effect: world',
'$effect: universe',
'$effect: universe'
]);
// assert.deepEqual(logs, ['universe', 'universe', '$effect: universe', '$effect: universe']); // this would also be ok
assert.htmlEqual(
target.innerHTML,
`

@ -2,7 +2,6 @@ import { tick } from 'svelte';
import { test } from '../../test';
export default test({
skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971
async test({ assert, target }) {
const [x, y, resolve] = target.querySelectorAll('button');
@ -18,7 +17,13 @@ export default test({
<button>y++</button>
<button>resolve</button>
<hr>
` // if this shows world world "world" world world world "world" - then this would also be ok
world
"world"
world
world
world
"world"
` // if this does not show world "world" world world world "world" - then this would also be ok
);
resolve.click();

@ -2,7 +2,6 @@ import { tick } from 'svelte';
import { test } from '../../test';
export default test({
skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971
async test({ assert, target }) {
const [x, y, resolve] = target.querySelectorAll('button');
@ -30,9 +29,17 @@ export default test({
<button>y++</button>
<button>resolve</button>
<hr>
` // if this shows world world "world" world world world "world" - then this would also be ok
world
"world"
world
world
world
"world"
` // if this does not show world "world" world world world "world" - then this would also be ok
);
resolve.click();
await tick();
resolve.click();
await tick();
assert.htmlEqual(

@ -31,4 +31,3 @@
{#if y > 0}
<Child x={await delay2(x)} />
{/if}

@ -2,7 +2,6 @@ import { tick } from 'svelte';
import { test } from '../../test';
export default test({
skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971
async test({ assert, target }) {
const [x, y, shift, pop, commit] = target.querySelectorAll('button');
@ -43,6 +42,8 @@ export default test({
await tick();
shift.click();
await tick();
shift.click(); // would be ok to not need this one
await tick();
assert.htmlEqual(
target.innerHTML,
`

@ -2,7 +2,6 @@ import { tick } from 'svelte';
import { test } from '../../test';
export default test({
skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971
async test({ assert, target }) {
const [x, y, resolve, commit] = target.querySelectorAll('button');

@ -2,7 +2,6 @@ import { tick } from 'svelte';
import { test } from '../../test';
export default test({
skip: true, // this fails on main, too; skip for now
async test({ assert, target, logs }) {
const [x, y, resolve] = target.querySelectorAll('button');

Loading…
Cancel
Save