diff --git a/.changeset/pretty-llamas-explode.md b/.changeset/pretty-llamas-explode.md new file mode 100644 index 0000000000..00109112de --- /dev/null +++ b/.changeset/pretty-llamas-explode.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +chore: simplify `batch.apply()` diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 102d0670b6..0dc149260a 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -44,12 +44,12 @@ export let current_batch = null; export let previous_batch = null; /** - * When time travelling, we re-evaluate deriveds based on the temporary - * values of their dependencies rather than their actual values, and cache - * the results in this map rather than on the deriveds themselves - * @type {Map | null} + * When time travelling (i.e. working in one batch, while other batches + * still have ongoing work), we ignore the real values of affected + * signals in favour of their values within the batch + * @type {Map | null} */ -export let batch_deriveds = null; +export let batch_values = null; /** @type {Set<() => void>} */ export let effect_pending_updates = new Set(); @@ -152,7 +152,7 @@ export class Batch { previous_batch = null; - var revert = Batch.apply(this); + this.apply(); for (const root of root_effects) { this.#traverse_effect_tree(root); @@ -161,6 +161,10 @@ export class Batch { // if we didn't start any new async work, and no async work // is outstanding from a previous flush, commit if (this.#pending === 0) { + // TODO we need this because we commit _then_ flush effects... + // maybe there's a way we can reverse the order? + var previous_batch_sources = batch_values; + this.#commit(); var render_effects = this.#render_effects; @@ -175,6 +179,7 @@ export class Batch { previous_batch = this; current_batch = null; + batch_values = previous_batch_sources; flush_queued_effects(render_effects); flush_queued_effects(effects); @@ -187,7 +192,7 @@ export class Batch { this.#defer_effects(this.#block_effects); } - revert(); + batch_values = null; for (const effect of this.#boundary_async_effects) { update_effect(effect); @@ -274,6 +279,7 @@ export class Batch { } this.current.set(source, source.v); + batch_values?.set(source, source.v); } activate() { @@ -282,6 +288,7 @@ export class Batch { deactivate() { current_batch = null; + batch_values = null; } flush() { @@ -352,14 +359,14 @@ export class Batch { if (queued_root_effects.length > 0) { current_batch = batch; - const revert = Batch.apply(batch); + batch.apply(); for (const root of queued_root_effects) { batch.#traverse_effect_tree(root); } queued_root_effects = []; - revert(); + batch.deactivate(); } } @@ -423,49 +430,23 @@ export class Batch { queue_micro_task(task); } - /** - * @param {Batch} current_batch - */ - static apply(current_batch) { - if (!async_mode_flag || batches.size === 1) { - return noop; - } + apply() { + if (!async_mode_flag || batches.size === 1) return; // if there are multiple batches, we are 'time travelling' — - // we need to undo the changes belonging to any batch - // other than the current one - - /** @type {Map} */ - var current_values = new Map(); - batch_deriveds = new Map(); - - for (const [source, current] of current_batch.current) { - current_values.set(source, { v: source.v, wv: source.wv }); - source.v = current; - } + // we need to override values with the ones in this batch... + batch_values = new Map(this.current); + // ...and undo changes belonging to other batches for (const batch of batches) { - if (batch === current_batch) continue; + if (batch === this) continue; for (const [source, previous] of batch.#previous) { - if (!current_values.has(source)) { - current_values.set(source, { v: source.v, wv: source.wv }); - source.v = previous; + if (!batch_values.has(source)) { + batch_values.set(source, previous); } } } - - return () => { - for (const [source, { v, wv }] of current_values) { - // reset the source to the current value (unless - // it got a newer value as a result of effects running) - if (source.wv <= wv) { - source.v = v; - } - } - - batch_deriveds = null; - }; } } diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 076a919236..bf8733cfe5 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -33,7 +33,7 @@ import { async_mode_flag, tracing_mode_flag } from '../../flags/index.js'; import { Boundary } from '../dom/blocks/boundary.js'; import { component_context } from '../context.js'; import { UNINITIALIZED } from '../../../constants.js'; -import { batch_deriveds, current_batch } from './batch.js'; +import { batch_values, current_batch } from './batch.js'; import { unset_context } from './async.js'; import { deferred } from '../../shared/utils.js'; @@ -336,6 +336,8 @@ export function update_derived(derived) { var value = execute_derived(derived); if (!derived.equals(value)) { + // TODO can we avoid setting `derived.v` when `batch_values !== null`, + // without causing the value to be stale later? derived.v = value; derived.wv = increment_write_version(); } @@ -346,8 +348,8 @@ export function update_derived(derived) { return; } - if (batch_deriveds !== null) { - batch_deriveds.set(derived, derived.v); + if (batch_values !== null) { + batch_values.set(derived, derived.v); } else { var status = (skip_reaction || (derived.f & UNOWNED) !== 0) && derived.deps !== null ? MAYBE_DIRTY : CLEAN; diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index b8f5f5ffc9..a146659bf6 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -42,7 +42,7 @@ import { set_dev_stack } from './context.js'; import * as w from './warnings.js'; -import { Batch, batch_deriveds, flushSync, schedule_effect } from './reactivity/batch.js'; +import { Batch, batch_values, flushSync, schedule_effect } from './reactivity/batch.js'; import { handle_error } from './error-handling.js'; import { UNINITIALIZED } from '../../constants.js'; import { captured_signals } from './legacy.js'; @@ -671,8 +671,8 @@ export function get(signal) { } else if (is_derived) { derived = /** @type {Derived} */ (signal); - if (batch_deriveds?.has(derived)) { - return batch_deriveds.get(derived); + if (batch_values?.has(derived)) { + return batch_values.get(derived); } if (is_dirty(derived)) { @@ -680,6 +680,10 @@ export function get(signal) { } } + if (batch_values?.has(signal)) { + return batch_values.get(signal); + } + if ((signal.f & ERROR_VALUE) !== 0) { throw signal.v; }