Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/wild-mirrors-take.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: don't rerun async effects unnecessarily
67 changes: 52 additions & 15 deletions packages/svelte/src/internal/client/reactivity/batch.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** @import { Derived, Effect, Source, Value } from '#client' */
/** @import { Derived, Effect, Reaction, Source, Value } from '#client' */
import {
BLOCK_EFFECT,
BRANCH_EFFECT,
Expand Down Expand Up @@ -342,31 +342,46 @@ export class Batch {
continue;
}

/** @type {Source[]} */
const sources = [];

for (const [source, value] of this.current) {
if (batch.current.has(source)) {
if (is_earlier) {
if (is_earlier && value !== batch.current.get(source)) {
// bring the value up to date
batch.current.set(source, value);
} else {
// later batch has more recent value,
// same value or later batch has more recent value,
// no need to re-run these effects
continue;
}
}

mark_effects(source);
sources.push(source);
}

if (queued_root_effects.length > 0) {
current_batch = batch;
batch.apply();
if (sources.length === 0) {
continue;
}

for (const root of queued_root_effects) {
batch.#traverse_effect_tree(root);
// Re-run async/block effects that depend on distinct values changed in both batches
const others = [...batch.current.keys()].filter((s) => !this.current.has(s));
if (others.length > 0) {
for (const source of sources) {
mark_effects(source, others);
}

queued_root_effects = [];
batch.deactivate();
if (queued_root_effects.length > 0) {
current_batch = batch;
batch.apply();

for (const root of queued_root_effects) {
batch.#traverse_effect_tree(root);
}

queued_root_effects = [];
batch.deactivate();
}
}
}

Expand Down Expand Up @@ -621,24 +636,46 @@ function flush_queued_effects(effects) {

/**
* This is similar to `mark_reactions`, but it only marks async/block effects
* so that these can re-run after another batch has been committed
* depending on `value` and at least one of the other `sources`, so that
* these effects can re-run after another batch has been committed
* @param {Value} value
* @param {Source[]} sources
*/
function mark_effects(value) {
function mark_effects(value, sources) {
if (value.reactions !== null) {
for (const reaction of value.reactions) {
const flags = reaction.f;

if ((flags & DERIVED) !== 0) {
mark_effects(/** @type {Derived} */ (reaction));
} else if ((flags & (ASYNC | BLOCK_EFFECT)) !== 0) {
mark_effects(/** @type {Derived} */ (reaction), sources);
} else if ((flags & (ASYNC | BLOCK_EFFECT)) !== 0 && depends_on(reaction, sources)) {
set_signal_status(reaction, DIRTY);
schedule_effect(/** @type {Effect} */ (reaction));
}
}
}
}

/**
* @param {Reaction} reaction
* @param {Source[]} sources
*/
function depends_on(reaction, sources) {
if (reaction.deps !== null) {
for (const dep of reaction.deps) {
if (sources.includes(dep)) {
return true;
}

if ((dep.f & DERIVED) !== 0 && depends_on(/** @type {Derived} */ (dep), sources)) {
return true;
}
}
}

return false;
}

/**
* @param {Effect} signal
* @returns {void}
Expand Down
7 changes: 7 additions & 0 deletions packages/svelte/src/internal/client/reactivity/deriveds.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,13 @@ export function async_derived(fn, location) {

internal_set(signal, value);

// All prior async derived runs are now stale
for (const [b, d] of deferreds) {
deferreds.delete(b);
if (b === batch) break;
d.reject(STALE_REACTION);
}

if (DEV && location !== undefined) {
recent_async_deriveds.add(signal);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export default test({
input.value = '12';
input.dispatchEvent(new Event('input', { bubbles: true }));
await macrotask(6);
// TODO this is wrong (separate bug), this should be 3 | 12
assert.htmlEqual(target.innerHTML, '<input> 5 | 12');
assert.htmlEqual(target.innerHTML, '<input> 3 | 12');
}
});
Loading