fix: reschedule effects inside unskipped branches

When a branch is speculatively marked for destruction (condition temporarily falsy), its child effects are reset to `CLEAN` to prevent them running in a doomed branch (as of #17581). However, if the branch survives (condition becomes truthy again), those effects remain `CLEAN` and never run - the source was already marked dirty before the reset, so no new dirty marking occurs.

The fix is to change `skipped_effects` from a `Set` to a `Map` that tracks which child effects were dirty/maybe_dirty before being reset. When a branch is unskipped (survives), restore their status and reschedule them.
pull/17605/head
Simon Holthausen 2 days ago
parent 61fb78c71c
commit 4ca62c19a7

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: reschedule effects inside unskipped branches

@ -200,17 +200,17 @@ export class BranchManager {
if (defer) {
for (const [k, effect] of this.#onscreen) {
if (k === key) {
batch.skipped_effects.delete(effect);
batch.unskip_effect(effect);
} else {
batch.skipped_effects.add(effect);
batch.skip_effect(effect);
}
}
for (const [k, branch] of this.#offscreen) {
if (k === key) {
batch.skipped_effects.delete(branch.effect);
batch.unskip_effect(branch.effect);
} else {
batch.skipped_effects.add(branch.effect);
batch.skip_effect(branch.effect);
}
}

@ -257,7 +257,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f
if (item.i) internal_set(item.i, index);
if (defer) {
batch.skipped_effects.delete(item.e);
batch.unskip_effect(item.e);
}
} else {
item = create_item(
@ -299,7 +299,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f
if (defer) {
for (const [key, item] of items) {
if (!keys.has(key)) {
batch.skipped_effects.add(item.e);
batch.skip_effect(item.e);
}
}

@ -130,11 +130,13 @@ export class Batch {
#maybe_dirty_effects = new Set();
/**
* A set of branches that still exist, but will be destroyed when this batch
* is committed we skip over these during `process`
* @type {Set<Effect>}
* A map of branches that still exist, but will be destroyed when this batch
* is committed we skip over these during `process`.
* The value contains child effects that were dirty/maybe_dirty before being reset,
* so they can be rescheduled if the branch survives.
* @type {Map<Effect, { d: Effect[], m: Effect[] }>}
*/
skipped_effects = new Set();
skipped_effects = new Map();
is_fork = false;
@ -144,6 +146,38 @@ export class Batch {
return this.is_fork || this.#blocking_pending > 0;
}
/**
* Add an effect to the skipped_effects map and reset its children
* @param {Effect} effect
*/
skip_effect(effect) {
if (!this.skipped_effects.has(effect)) {
this.skipped_effects.set(effect, { d: [], m: [] });
}
}
/**
* Remove an effect from the skipped_effects map and reschedule
* any tracked dirty/maybe_dirty child effects
* @param {Effect} effect
*/
unskip_effect(effect) {
var tracked = this.skipped_effects.get(effect);
if (tracked) {
this.skipped_effects.delete(effect);
for (var e of tracked.d) {
set_signal_status(e, DIRTY);
schedule_effect(e);
}
for (var e of tracked.m) {
set_signal_status(e, MAYBE_DIRTY);
schedule_effect(e);
}
}
}
/**
*
* @param {Effect[]} root_effects
@ -172,8 +206,8 @@ export class Batch {
this.#defer_effects(render_effects);
this.#defer_effects(effects);
for (const e of this.skipped_effects) {
reset_branch(e);
for (const [e, t] of this.skipped_effects) {
reset_branch(e, t);
}
} else {
// append/remove branches
@ -887,20 +921,28 @@ export function eager(fn) {
/**
* Mark all the effects inside a skipped branch CLEAN, so that
* they can be correctly rescheduled later
* they can be correctly rescheduled later. Tracks dirty and maybe_dirty
* effects so they can be rescheduled if the branch survives.
* @param {Effect} effect
* @param {{ d: Effect[], m: Effect[] }} tracked
*/
function reset_branch(effect) {
function reset_branch(effect, tracked) {
// clean branch = nothing dirty inside, no need to traverse further
if ((effect.f & BRANCH_EFFECT) !== 0 && (effect.f & CLEAN) !== 0) {
return;
}
if ((effect.f & DIRTY) !== 0) {
tracked.d.push(effect);
} else if ((effect.f & MAYBE_DIRTY) !== 0) {
tracked.m.push(effect);
}
set_signal_status(effect, CLEAN);
var e = effect.first;
while (e !== null) {
reset_branch(e);
reset_branch(e, tracked);
e = e.next;
}
}

@ -47,9 +47,9 @@ export interface RuntimeTest<Props extends Record<string, any> = Record<string,
mode?: Array<'server' | 'async-server' | 'client' | 'hydrate'>;
/** Temporarily skip specific modes, without skipping the entire test */
skip_mode?: Array<'server' | 'async-server' | 'client' | 'hydrate'>;
/** Skip if running with process.env.NO_ASYNC */
/** Skip if running with process.env.SVELTE_NO_ASYNC */
skip_no_async?: boolean;
/** Skip if running without process.env.NO_ASYNC */
/** Skip if running without process.env.SVELTE_NO_ASYNC */
skip_async?: boolean;
html?: string;
ssrHtml?: string;

Loading…
Cancel
Save