Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
66 changes: 50 additions & 16 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 @@ -335,31 +335,43 @@ 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;
const revert = Batch.apply(batch);

for (const root of queued_root_effects) {
batch.#traverse_effect_tree(root);
// Only reschedule an async effect if it depends on something else than
// the sources that were updated in this batch and that something else changed.
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 = [];
revert();
if (queued_root_effects.length > 0) {
current_batch = batch;
const revert = Batch.apply(batch);

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

queued_root_effects = [];
revert();
}
}
}

Expand Down Expand Up @@ -640,24 +652,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 one of the 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