diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index 043b50b4b2..d96cb5204c 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -23,14 +23,13 @@ export const BOUNDARY_EFFECT = 1 << 7; */ export const CONNECTED = 1 << 9; export const CLEAN = 1 << 10; -export const DIRTY = 1 << 11; -export const MAYBE_DIRTY = 1 << 12; export const INERT = 1 << 13; export const DESTROYED = 1 << 14; /** Set once a reaction has run for the first time */ export const REACTION_RAN = 1 << 15; /** Effect is in the process of getting destroyed. Can be observed in child teardown functions */ export const DESTROYING = 1 << 25; +export const EFFECT_LEGACY = 1 << 26; // Flags exclusive to effects /** @@ -43,6 +42,7 @@ export const HEAD_EFFECT = 1 << 18; export const EFFECT_PRESERVED = 1 << 19; export const USER_EFFECT = 1 << 20; export const EFFECT_OFFSCREEN = 1 << 25; +export const STATE_EAGER_EFFECT = 1 << 27; // Flags exclusive to deriveds /** diff --git a/packages/svelte/src/internal/client/context.js b/packages/svelte/src/internal/client/context.js index 0baef5c63e..50ff2584bd 100644 --- a/packages/svelte/src/internal/client/context.js +++ b/packages/svelte/src/internal/client/context.js @@ -227,7 +227,15 @@ export function pop(component) { /** @returns {boolean} */ export function is_runes() { - return !legacy_mode_flag || (component_context !== null && component_context.l === null); + if (!legacy_mode_flag) { + return true; + } + + // TODO feels like we could probably simplify this a bit. no tests fail without + // the first part, though would like to better understand usage before deleting + const context = component_context ?? active_reaction?.ctx ?? active_effect?.ctx; + + return context?.l === null; } /** diff --git a/packages/svelte/src/internal/client/dev/debug.js b/packages/svelte/src/internal/client/dev/debug.js index 83cc510ae2..bda682c393 100644 --- a/packages/svelte/src/internal/client/dev/debug.js +++ b/packages/svelte/src/internal/client/dev/debug.js @@ -7,19 +7,18 @@ import { CLEAN, CONNECTED, DERIVED, - DIRTY, EFFECT, ASYNC, DESTROYED, INERT, - MAYBE_DIRTY, RENDER_EFFECT, ROOT_EFFECT, WAS_MARKED, MANAGED_EFFECT } from '#client/constants'; import { snapshot } from '../../shared/clone.js'; -import { untrack } from '../runtime.js'; +import { get_cv, get_wv } from '../reactivity/batch.js'; +import { is_dirty, untrack } from '../runtime.js'; /** * @@ -77,8 +76,15 @@ export function log_effect_tree(effect, highlighted = [], depth = 0, is_reachabl const flags = effect.f; let label = effect_label(effect); - let status = - (flags & CLEAN) !== 0 ? 'clean' : (flags & MAYBE_DIRTY) !== 0 ? 'maybe dirty' : 'dirty'; + let status = 'clean'; + + if ((flags & (BRANCH_EFFECT | ROOT_EFFECT)) !== 0) { + if ((flags & CLEAN) === 0) status = 'dirty'; + } else { + if (is_dirty(effect)) { + status = 'dirty'; + } + } let styles = [`font-weight: ${status === 'clean' ? 'normal' : 'bold'}`]; @@ -96,7 +102,7 @@ export function log_effect_tree(effect, highlighted = [], depth = 0, is_reachabl } // eslint-disable-next-line no-console - console.group(`%c${label} (${status})`, styles.join('; ')); + console.group(`%c${label} (${status}) cv=${get_cv(effect)}`, styles.join('; ')); if (depth === 0) { const callsite = new Error().stack @@ -158,7 +164,7 @@ function log_dep(dep) { // eslint-disable-next-line no-console console.groupCollapsed( - `%c$derived %c${dep.label ?? ''}`, + `%c$derived %c${dep.label ?? ''} wv=${get_wv(derived)} cv=${get_cv(derived)}`, 'font-weight: bold; color: CornflowerBlue', 'font-weight: normal', untrack(() => snapshot(derived.v)) @@ -175,7 +181,7 @@ function log_dep(dep) { } else { // eslint-disable-next-line no-console console.log( - `%c$state %c${dep.label ?? ''}`, + `%c$state %c${dep.label ?? ''} wv=${get_wv(dep)}`, 'font-weight: bold; color: CornflowerBlue', 'font-weight: normal', untrack(() => snapshot(dep.v)) @@ -201,8 +207,6 @@ export function log_reactions(signal) { const names = []; if ((flags & CLEAN) !== 0) names.push('CLEAN'); - if ((flags & DIRTY) !== 0) names.push('DIRTY'); - if ((flags & MAYBE_DIRTY) !== 0) names.push('MAYBE_DIRTY'); if ((flags & CONNECTED) !== 0) names.push('CONNECTED'); if ((flags & WAS_MARKED) !== 0) names.push('WAS_MARKED'); if ((flags & INERT) !== 0) names.push('INERT'); @@ -259,7 +263,7 @@ export function log_reactions(signal) { } else { // It's an effect const label = effect_label(/** @type {Effect} */ (reaction), true); - const status = (flags & MAYBE_DIRTY) !== 0 ? 'maybe dirty' : 'dirty'; + const status = is_dirty(reaction) ? 'dirty' : 'clean'; // Collect parent statuses /** @type {string[]} */ @@ -380,8 +384,7 @@ export function log_inconsistent_branches(effect) { const is_branch = (flags & BRANCH_EFFECT) !== 0; if (is_branch) { - const status = - (flags & CLEAN) !== 0 ? 'clean' : (flags & MAYBE_DIRTY) !== 0 ? 'maybe dirty' : 'dirty'; + const status = (flags & CLEAN) !== 0 ? 'clean' : 'dirty'; /** @type {BranchInfo[]} */ const child_branches = []; diff --git a/packages/svelte/src/internal/client/dev/tracing.js b/packages/svelte/src/internal/client/dev/tracing.js index c6edfde933..b0bd7a4a8e 100644 --- a/packages/svelte/src/internal/client/dev/tracing.js +++ b/packages/svelte/src/internal/client/dev/tracing.js @@ -4,6 +4,7 @@ import { snapshot } from '../../shared/clone.js'; import { DERIVED, ASYNC, PROXY_PATH_SYMBOL, STATE_SYMBOL } from '#client/constants'; import { effect_tracking } from '../reactivity/effects.js'; import { active_reaction, untrack } from '../runtime.js'; +import { get_cv, get_wv } from '../reactivity/batch.js'; /** * @typedef {{ @@ -27,7 +28,7 @@ function log_entry(signal, entry) { const type = get_type(signal); const current_reaction = /** @type {Reaction} */ (active_reaction); - const dirty = signal.wv > current_reaction.wv || current_reaction.wv === 0; + const dirty = get_wv(signal) > get_cv(current_reaction); const style = dirty ? 'color: CornflowerBlue; font-weight: bold' : 'color: grey; font-weight: normal'; diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 0c0903ff52..a1918792a8 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -1,11 +1,5 @@ /** @import { Effect, Source, TemplateNode, } from '#client' */ -import { - BOUNDARY_EFFECT, - DIRTY, - EFFECT_PRESERVED, - EFFECT_TRANSPARENT, - MAYBE_DIRTY -} from '#client/constants'; +import { BOUNDARY_EFFECT, EFFECT_PRESERVED, EFFECT_TRANSPARENT } from '#client/constants'; import { HYDRATION_START_ELSE, HYDRATION_START_FAILED } from '../../../../constants.js'; import { component_context, set_component_context } from '../../context.js'; import { handle_error, invoke_error_boundary } from '../../error-handling.js'; @@ -110,9 +104,6 @@ export class Boundary { /** @type {Set} */ #dirty_effects = new Set(); - /** @type {Set} */ - #maybe_dirty_effects = new Set(); - /** * A source containing the number of pending async deriveds/expressions. * Only created if `$effect.pending()` is used inside the boundary, @@ -272,7 +263,7 @@ export class Boundary { // any effects that were previously deferred should be transferred // to the batch, which will flush in the next microtask - batch.transfer_effects(this.#dirty_effects, this.#maybe_dirty_effects); + batch.transfer_effects(this.#dirty_effects); } /** @@ -280,7 +271,7 @@ export class Boundary { * @param {Effect} effect */ defer_effect(effect) { - defer_effect(effect, this.#dirty_effects, this.#maybe_dirty_effects); + defer_effect(effect, this.#dirty_effects); } /** diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 2df1d6ffa1..60acaf8bc9 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -228,6 +228,8 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f return; } + var array = get(each_array); + state.pending.delete(batch); state.fallback = fallback; @@ -260,7 +262,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f } var effect = block(() => { - array = /** @type {V[]} */ (get(each_array)); + array = get(each_array); var length = array.length; /** `true` if there was a hydration mismatch. Needs to be a `let` or else it isn't treeshaken out */ @@ -383,14 +385,6 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f // continue in hydration mode set_hydrating(true); } - - // When we mount the each block for the first time, the collection won't be - // connected to this effect as the effect hasn't finished running yet and its deps - // won't be assigned. However, it's possible that when reconciling the each block - // that a mutation occurred and it's made the collection MAYBE_DIRTY, so reading the - // collection again can provide consistency to the reactive graph again as the deriveds - // will now be `CLEAN`. - get(each_array); }); /** @type {EachState} */ diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 3b82059788..d906d59498 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -1,22 +1,21 @@ /** @import { Fork } from 'svelte' */ -/** @import { Derived, Effect, Reaction, Source, Value } from '#client' */ +/** @import { Derived, Effect, Reaction, Source, Value, ValueSnapshot } from '#client' */ import { BLOCK_EFFECT, BRANCH_EFFECT, CLEAN, DESTROYED, - DIRTY, EFFECT, ASYNC, INERT, RENDER_EFFECT, ROOT_EFFECT, - MAYBE_DIRTY, DERIVED, EAGER_EFFECT, ERROR_VALUE, MANAGED_EFFECT, REACTION_RAN, + STATE_EAGER_EFFECT, DESTROYING } from '#client/constants'; import { async_mode_flag } from '../../flags/index.js'; @@ -27,20 +26,26 @@ import { get, increment_write_version, is_dirty, - update_effect + update_effect, + write_version } from '../runtime.js'; import * as e from '../errors.js'; import { flush_tasks, queue_micro_task } from '../dom/task.js'; import { DEV } from 'esm-env'; import { invoke_error_boundary } from '../error-handling.js'; -import { flush_eager_effects, old_values, set_eager_effects, source, update } from './sources.js'; +import { + flush_eager_effects, + mark_reactions, + old_values, + set_eager_effects, + source, + update +} from './sources.js'; import { eager_effect, teardown, unlink_effect } from './effects.js'; import { defer_effect } from './utils.js'; import { UNINITIALIZED } from '../../../constants.js'; -import { set_signal_status } from './status.js'; import { legacy_is_updating_store } from './store.js'; import { invariant } from '../../shared/dev.js'; -import { log_effect_tree } from '../dev/debug.js'; /** @type {Batch | null} */ let first_batch = null; @@ -52,18 +57,19 @@ let last_batch = null; export let current_batch = null; /** - * This is needed to avoid overwriting inputs + * The batch that is currently applied. May not be the same as `current_batch`, since we + * null that out when flushing effects in case they set state, resulting in a new + * batch being created. Effects always run inside an active_batch. + * TODO most occurrences of `current_batch` should be this * @type {Batch | null} - */ -export let previous_batch = null; + **/ +export let active_batch = 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} + * This is needed to avoid overwriting inputs + * @type {Batch | null} */ -export let batch_values = null; +export let previous_batch = null; /** @type {Effect | null} */ let last_scheduled_effect = null; @@ -116,17 +122,26 @@ export class Batch { * The current values of any signals that are updated in this batch. * Tuple format: [value, is_derived] (note: is_derived is false for deriveds, too, if they were overridden via assignment) * They keys of this map are identical to `this.#previous` - * @type {Map} + * @type {Map>} */ current = new Map(); /** * The values of any signals (sources and deriveds) that are updated in this batch _before_ those updates took place. * They keys of this map are identical to `this.#current` - * @type {Map} + * @type {Map>} */ previous = new Map(); + /** + * The combination of this batch's `current` and other batches' `previous` values, + * 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} + */ + values = null; + /** * When the batch is committed (and the DOM is updated), we need to remove old branches * and append new ones by calling the functions added inside (if/each/key/etc) blocks @@ -171,16 +186,13 @@ export class Batch { #new_effects = []; /** - * Deferred effects (which run after async work has completed) that are DIRTY + * Deferred effects (which run after async work has completed) that are dirty * @type {Set} */ #dirty_effects = new Set(); - /** - * Deferred effects that are MAYBE_DIRTY - * @type {Set} - */ - #maybe_dirty_effects = new Set(); + /** @type {Map} */ + cvs = new Map(); /** * A map of branches that still exist, but will be destroyed when this batch @@ -260,12 +272,10 @@ export class Batch { this.#skipped_branches.delete(effect); for (var e of tracked.d) { - set_signal_status(e, DIRTY); callback(e); } for (e of tracked.m) { - set_signal_status(e, MAYBE_DIRTY); callback(e); } } @@ -273,6 +283,9 @@ export class Batch { } #process() { + if (this.#committed) return; + + current_batch = this; this.#started = true; if (flush_count++ > 1000) { @@ -280,6 +293,11 @@ export class Batch { infinite_loop_guard(); } + // TODO we only need to do this for re-runs + for (const [source, snapshot] of this.current) { + mark_reactions(this, source, snapshot.wv, null); + } + if (DEV) { // track all the values that were updated during this flush, // so that they can be reset afterwards @@ -288,18 +306,7 @@ export class Batch { } } - // We always reschedule previously-deferred effects, not just when - // #is_deferred() is true, because traversing the tree could make - // an if block that contains the last blocking pending effect falsy, - // causing the block to no longer be deferred. for (const e of this.#dirty_effects) { - this.#maybe_dirty_effects.delete(e); - set_signal_status(e, DIRTY); - this.schedule(e); - } - - for (const e of this.#maybe_dirty_effects) { - set_signal_status(e, MAYBE_DIRTY); this.schedule(e); } @@ -377,7 +384,6 @@ export class Batch { // clear effects. Those that are still needed will be rescheduled through unskipping the skipped branches. this.#dirty_effects.clear(); - this.#maybe_dirty_effects.clear(); // append/remove branches for (const fn of this.#commit_callbacks) fn(this); @@ -418,6 +424,8 @@ export class Batch { } } + active_batch = null; + if (next_batch !== null) { next_batch.#process(); } @@ -450,7 +458,6 @@ export class Batch { } else if (async_mode_flag && (flags & (RENDER_EFFECT | MANAGED_EFFECT)) !== 0) { render_effects.push(effect); } else if (is_dirty(effect)) { - if ((flags & BLOCK_EFFECT) !== 0) this.#maybe_dirty_effects.add(effect); update_effect(effect); } @@ -481,8 +488,8 @@ export class Batch { while (batch !== null) { if (!batch.is_fork) { // if the batches are connected, break - for (const [value, [, is_derived]] of this.current) { - if (batch.current.has(value) && !is_derived) { + for (const value of this.current.keys()) { + if (batch.current.has(value)) { return batch; } } @@ -498,21 +505,41 @@ export class Batch { * @param {Batch} batch */ #merge(batch) { - for (const [source, value] of batch.current) { - if (!this.previous.has(source) && batch.previous.has(source)) { - this.previous.set(source, batch.previous.get(source)); + for (const [source, snapshot] of batch.current) { + var previous = batch.previous.get(source); + + if (previous && !this.previous.has(source)) { + this.previous.set(source, previous); } - this.current.set(source, value); + this.current.set(source, snapshot); } for (const [effect, deferred] of batch.async_deriveds) { const d = this.async_deriveds.get(effect); if (d) deferred.promise.then(d.resolve).catch(d.reject); + + var cv = batch.cvs.get(effect); + if (cv !== undefined) this.cvs.set(effect, cv); } - // Mark is not guaranteed not touch these, so we transfer them - this.transfer_effects(batch.#dirty_effects, batch.#maybe_dirty_effects); + for (const fn of batch.#commit_callbacks) { + this.#commit_callbacks.add(() => fn(batch)); + } + + for (const fn of batch.#discard_callbacks) { + this.#discard_callbacks.add(() => fn(batch)); + } + + // TODO this doesn't feel quite right, but it gets the tests to pass + this.oncommit(() => { + for (const fn of batch.#discard_callbacks) fn(batch); + }); + + this.settled().then(() => batch.#deferred?.resolve()); + + // Mark is not guaranteed to not touch these, so we transfer them + this.transfer_effects(batch.#dirty_effects); /** * mark all effects that depend on `batch.current`, except the @@ -535,8 +562,10 @@ export class Batch { var effect = /** @type {Effect} */ (reaction); if (flags & (ASYNC | BLOCK_EFFECT) && !this.async_deriveds.has(effect)) { - this.#maybe_dirty_effects.delete(effect); - set_signal_status(effect, DIRTY); + if ((effect.f & CLEAN) !== 0) { + effect.f ^= CLEAN; + } + this.schedule(effect); } } @@ -547,7 +576,6 @@ export class Batch { mark(source); } - this.oncommit(() => batch.discard()); batch.#unlink(); current_batch = this; @@ -559,30 +587,35 @@ export class Batch { */ #defer_effects(effects) { for (var i = 0; i < effects.length; i += 1) { - defer_effect(effects[i], this.#dirty_effects, this.#maybe_dirty_effects); + defer_effect(effects[i], this.#dirty_effects); } } /** * Associate a change to a given source with the current * batch, noting its previous and current values - * @param {Value} source - * @param {any} value - * @param {boolean} [is_derived] + * @param {Value} value + * @param {any} v + * @param {number} wv */ - capture(source, value, is_derived = false) { - if (source.v !== UNINITIALIZED && !this.previous.has(source)) { - this.previous.set(source, source.v); + capture(value, v, wv) { + if (value.v !== UNINITIALIZED && !this.previous.has(value)) { + this.previous.set(value, { v: value.v, wv: value.wv }); } - // Don't save errors in `batch_values`, or they won't be thrown in `runtime.js#get` - if ((source.f & ERROR_VALUE) === 0) { - this.current.set(source, [value, is_derived]); - batch_values?.set(source, value); + // Don't save errors or they won't be thrown in `runtime.js#get` + if ((value.f & ERROR_VALUE) === 0) { + var snapshot = { v, wv }; + + this.current.delete(value); // order must be preserved + + this.current.set(value, snapshot); + active_batch?.values?.set(value, snapshot); } if (!this.is_fork) { - source.v = value; + value.v = v; + value.wv = wv; } } @@ -592,7 +625,7 @@ export class Batch { deactivate() { current_batch = null; - batch_values = null; + active_batch = null; } flush() { @@ -602,8 +635,6 @@ export class Batch { } is_processing = true; - current_batch = this; - this.#process(); } finally { flush_count = 0; @@ -613,7 +644,7 @@ export class Batch { is_processing = false; current_batch = null; - batch_values = null; + active_batch = null; old_values.clear(); @@ -640,7 +671,13 @@ export class Batch { this.#new_effects.push(effect); } + #committed = false; + #commit() { + // TODO seems like a bug that we can end up here more than once + if (this.#committed) return; + this.#committed = true; + // If there are other pending batches, they now need to be 'rebased' — // in other words, we re-run block/async effects with the newly // committed state, unless the batch in question has a more @@ -651,13 +688,13 @@ export class Batch { /** @type {Source[]} */ var sources = []; - for (const [source, [value, is_derived]] of this.current) { - if (batch.current.has(source)) { - var batch_value = /** @type {[any, boolean]} */ (batch.current.get(source))[0]; // faster than destructuring + for (const [source, snapshot] of this.current) { + var batch_snapshot = batch.current.get(source); - if (is_earlier && value !== batch_value) { + if (batch_snapshot) { + if (is_earlier && snapshot.v !== batch_snapshot.v) { // bring the value up to date - batch.current.set(source, [value, is_derived]); + batch.current.set(source, snapshot); } else { // same value or later batch has more recent value, // no need to re-run these effects @@ -680,9 +717,10 @@ export class Batch { if (!batch.#started) continue; // Re-run async/block effects that depend on distinct values changed in both batches (ignoring deriveds) - var others = [...batch.current.keys()].filter( - (s) => !(/** @type {[any, boolean]} */ (batch.current.get(s))[1]) && !this.current.has(s) - ); + var others = Array.from(batch.current.keys()).filter((value) => { + if ((value.f & DERIVED) !== 0) return false; + return !this.current.has(value); + }); if (others.length === 0) { if (is_earlier) { @@ -718,32 +756,24 @@ export class Batch { var checked = new Map(); for (var source of sources) { - mark_effects(source, others, marked, checked); + mark_effects(batch, source, others, marked, checked); } checked = new Map(); var current_unequal = [...batch.current] .filter(([c, v1]) => { const v2 = this.current.get(c); - if (!v2) return true; - // Either their values are different or one is a derived but not the other - return v2[0] !== v1[0] || v2[1] !== v1[1]; + return !v2 || v2.v !== v1.v; }) .map(([c]) => c); - if (current_unequal.length > 0) { - 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); - } - } + for (const effect of this.#new_effects) { + if ( + (effect.f & (DESTROYED | INERT | EAGER_EFFECT)) === 0 && + depends_on(effect, current_unequal, checked) + ) { + batch.schedule(effect); + batch.cvs.set(effect, -1); } } @@ -808,19 +838,13 @@ export class Batch { /** * @param {Set} dirty_effects - * @param {Set} maybe_dirty_effects */ - transfer_effects(dirty_effects, maybe_dirty_effects) { + transfer_effects(dirty_effects) { for (const e of dirty_effects) { this.#dirty_effects.add(e); } - for (const e of maybe_dirty_effects) { - this.#maybe_dirty_effects.add(e); - } - dirty_effects.clear(); - maybe_dirty_effects.clear(); } /** @param {(batch: Batch) => void} fn */ @@ -854,17 +878,17 @@ export class Batch { } apply() { - if (!async_mode_flag || (!this.is_fork && this.#prev === null && this.#next === null)) { - batch_values = null; + if (!async_mode_flag) { + // TODO previously we bailed here if there was only one (non-fork) batch... maybe we can reinstate that return; } + if (active_batch === this) return; + active_batch = this; + // if there are multiple batches, we are 'time travelling' — // we need to override values with the ones in this batch... - batch_values = new Map(); - for (const [source, [value]] of this.current) { - batch_values.set(source, value); - } + this.values = new Map(this.current); // ...and undo changes belonging to other batches unless they intersect for (let batch = first_batch; batch !== null; batch = batch.#next) { @@ -875,11 +899,7 @@ export class Batch { var intersects = false; if (batch.id < this.id) { - for (const [source, [, is_derived]] of batch.current) { - // Derived values don't partake in the intersection mechanism, because a derived could - // be triggered in one batch already but not the other one yet, causing a false-positive - if (is_derived) continue; - + for (const source of batch.current.keys()) { if (this.current.has(source)) { intersects = true; break; @@ -887,15 +907,10 @@ export class Batch { } } - // Since the latter batch merges into the earlier (if it resolves before the earlier one), - // we treat the earlier values as "already applied". This way we don't need to rerun async - // effects of the earlier batch in case they are merged. - // As a result you can think of batch_values as having the latest values of all intersecting - // batches up until this batch. if (!intersects) { - for (const [source, previous] of batch.previous) { - if (!batch_values.has(source)) { - batch_values.set(source, previous); + for (const [value, snapshot] of batch.previous) { + if (!this.values.has(value)) { + this.values.set(value, snapshot); } } } @@ -909,6 +924,10 @@ export class Batch { schedule(effect) { last_scheduled_effect = effect; + if (!this.cvs.has(effect)) { + this.cvs.set(effect, effect.cv); + } + // defer render effects inside a pending boundary // TODO the `REACTION_RAN` check is only necessary because of legacy `$:` effects AFAICT — we can remove later if ( @@ -1137,12 +1156,13 @@ function flush_queued_effects(effects) { * This is similar to `mark_reactions`, but it only marks async/block effects * 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 {Batch} batch * @param {Value} value * @param {Source[]} sources * @param {Set} marked * @param {Map} checked */ -function mark_effects(value, sources, marked, checked) { +function mark_effects(batch, value, sources, marked, checked) { if (marked.has(value)) return; marked.add(value); @@ -1151,14 +1171,13 @@ function mark_effects(value, sources, marked, checked) { const flags = reaction.f; if ((flags & DERIVED) !== 0) { - mark_effects(/** @type {Derived} */ (reaction), sources, marked, checked); - } else if ( - (flags & (ASYNC | BLOCK_EFFECT)) !== 0 && - (flags & DIRTY) === 0 && - depends_on(reaction, sources, checked) - ) { - set_signal_status(reaction, DIRTY); - schedule_effect(/** @type {Effect} */ (reaction)); + batch.current.delete(/** @type {Derived} */ (reaction)); + batch.cvs.set(/** @type {Derived} */ (reaction), -1); + + mark_effects(batch, /** @type {Derived} */ (reaction), sources, marked, checked); + } else if ((flags & (ASYNC | BLOCK_EFFECT)) !== 0 && depends_on(reaction, sources, checked)) { + batch.schedule(/** @type {Effect} */ (reaction)); + batch.cvs.set(reaction, -1); } } } @@ -1180,7 +1199,6 @@ function mark_eager_effects(value, effects) { if ((flags & DERIVED) !== 0) { mark_eager_effects(/** @type {Derived} */ (reaction), effects); } else if ((flags & EAGER_EFFECT) !== 0) { - set_signal_status(reaction, DIRTY); effects.add(/** @type {Effect} */ (reaction)); } } @@ -1213,17 +1231,11 @@ function depends_on(reaction, sources, checked) { return false; } -/** - * @param {Effect} effect - * @returns {void} - */ -export function schedule_effect(effect) { - /** @type {Batch} */ (current_batch).schedule(effect); -} - /** @type {Source[]} */ let eager_versions = []; +let running_eager_effect = false; + function eager_flush() { flushSync(() => { const eager = eager_versions; @@ -1262,32 +1274,47 @@ export function eager(fn) { get(version); - eager_effect(() => { + if (DEV) { + version.label = ''; + } + + var effect = eager_effect(() => { if (initial) { // the first time this runs, we create an eager effect // that will run eagerly whenever the expression changes - var previous_batch_values = batch_values; + var previous_batch = active_batch; + var previous_running_eager_effect = running_eager_effect; try { - batch_values = null; + running_eager_effect = true; + active_batch = null; value = fn(); } finally { - batch_values = previous_batch_values; + active_batch = previous_batch; + running_eager_effect = previous_running_eager_effect; } return; } - // the second time this effect runs, it's to schedule a - // `version` update. since this will recreate the effect, - // we don't need to evaluate the expression here - if (eager_versions.length === 0) { - queue_micro_task(eager_flush); - } + if (!current_batch?.is_fork) { + // the second time this effect runs, it's to schedule a + // `version` update. since this will recreate the effect, + // we don't need to evaluate the expression here + if (eager_versions.length === 0) { + queue_micro_task(eager_flush); + } - eager_versions.push(version); + eager_versions.push(version); + } else { + fn(); + } }); + // TODO ideally this wouldn't be necessary. I haven't figured out a way for these + // effects to correctly be marked dirty when `$state.eager(...)` arguments change + effect.f |= STATE_EAGER_EFFECT; + initial = false; return value; @@ -1302,18 +1329,14 @@ export function eager(fn) { */ 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); + if ((effect.f & BRANCH_EFFECT) !== 0) { + if ((effect.f & CLEAN) === 0) { + effect.f ^= CLEAN; + } else { + return; + } } - set_signal_status(effect, CLEAN); - var e = effect.first; while (e !== null) { reset_branch(e, tracked); @@ -1326,8 +1349,6 @@ function reset_branch(effect, tracked) { * @param {Effect} effect */ function reset_all(effect) { - set_signal_status(effect, CLEAN); - var e = effect.first; while (e !== null) { reset_all(e); @@ -1365,11 +1386,11 @@ export function fork(fn) { var batch = Batch.ensure(); batch.is_fork = true; - batch_values = new Map(); var committed = false; var settled = batch.settled(); + batch.apply(); flushSync(fn); return { @@ -1387,10 +1408,15 @@ export function fork(fn) { batch.is_fork = false; - // apply changes and update write versions so deriveds see the change - for (var [source, [value]] of batch.current) { - source.v = value; - source.wv = increment_write_version(); + for (var [reaction, cv] of batch.cvs) { + if (cv > reaction.cv) { + reaction.cv = cv; + } + } + + for (var [value, snapshot] of batch.current) { + value.v = snapshot.v; + value.wv = snapshot.wv; } // trigger any `$state.eager(...)` expressions with the new state. @@ -1398,6 +1424,8 @@ export function fork(fn) { // can't just encounter them during traversal, we need to // proactively flush them // TODO maybe there's a better implementation? + // e.g. maybe we can just schedule them so that they run + // with everything else during batch.flush? flushSync(() => { /** @type {Set} */ var eager_effects = new Set(); @@ -1414,9 +1442,8 @@ export function fork(fn) { await settled; }, discard: () => { - // cause any MAYBE_DIRTY deriveds to update - // if they depend on things thath changed - // inside the discarded fork + // cause any deriveds to update if they depend on + // things that changed inside the discarded fork for (var source of batch.current.keys()) { source.wv = increment_write_version(); } @@ -1428,6 +1455,34 @@ export function fork(fn) { }; } +/** + * @param {Value} value + */ +export function get_wv(value) { + var snapshot = active_batch?.values?.get(value); + return snapshot ? snapshot.wv : value.wv; +} + +/** + * @param {Reaction} reaction + */ +export function get_cv(reaction) { + return active_batch?.cvs.get(reaction) ?? reaction.cv; +} + +/** + * @param {Reaction} reaction + */ +export function set_cv(reaction, cv = write_version) { + // TODO seems weird to have both of these + current_batch?.cvs.set(reaction, cv); + active_batch?.cvs.set(reaction, cv); + + if (!current_batch?.is_fork && !running_eager_effect) { + reaction.cv = cv; + } +} + /** * Forcibly remove all current batches, to prevent cross-talk between tests */ diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index fd64f3b45d..6352c66b17 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -5,48 +5,42 @@ import { DEV } from 'esm-env'; import { ERROR_VALUE, DERIVED, - DIRTY, EFFECT_PRESERVED, STALE_REACTION, ASYNC, WAS_MARKED, DESTROYED, - CLEAN, REACTION_RAN, + CONNECTED, + CLEAN, INERT } from '#client/constants'; import { active_reaction, active_effect, update_reaction, - increment_write_version, set_active_effect, push_reaction_value, - is_destroying_effect, update_effect, remove_reactions, + write_version, skipped_deps, - new_deps + new_deps, + is_destroying_effect, + current_sources } from '../runtime.js'; import { equals, safe_equals } from './equality.js'; import * as e from '../errors.js'; import * as w from '../warnings.js'; -import { - async_effect, - destroy_effect, - destroy_effect_children, - effect_tracking, - teardown -} from './effects.js'; -import { eager_effects, internal_set, set_eager_effects, source } from './sources.js'; +import { async_effect, destroy_effect, destroy_effect_children, teardown } from './effects.js'; +import { eager_effects, internal_set, set_eager_effects, source, state } from './sources.js'; import { get_error } from '../../shared/dev.js'; import { async_mode_flag, tracing_mode_flag } from '../../flags/index.js'; import { component_context } from '../context.js'; import { UNINITIALIZED } from '../../../constants.js'; -import { batch_values, current_batch, previous_batch } from './batch.js'; +import { current_batch, get_wv, active_batch, set_cv, previous_batch } from './batch.js'; import { increment_pending, unset_context } from './async.js'; -import { deferred, includes, noop } from '../../shared/utils.js'; -import { set_signal_status, update_derived_status } from './status.js'; +import { deferred, noop } from '../../shared/utils.js'; /** * This allows us to track 'reactivity loss' that occurs when signals @@ -69,8 +63,6 @@ export const recent_async_deriveds = new Set(); */ /*#__NO_SIDE_EFFECTS__*/ export function derived(fn) { - var flags = DERIVED | DIRTY; - if (active_effect !== null) { // Since deriveds are evaluated lazily, any effects created inside them are // created too late to ensure that the parent effect is added to the tree @@ -83,12 +75,13 @@ export function derived(fn) { deps: null, effects: null, equals, - f: flags, + f: DERIVED, fn, reactions: null, + cv: -1, rv: 0, - v: /** @type {V} */ (UNINITIALIZED), wv: 0, + v: /** @type {V} */ (UNINITIALIZED), parent: active_effect, ac: null }; @@ -118,7 +111,7 @@ export function async_derived(fn, label, location) { } var promise = /** @type {Promise} */ (/** @type {unknown} */ (undefined)); - var signal = source(/** @type {V} */ (UNINITIALIZED)); + var signal = state(/** @type {V} */ (UNINITIALIZED)); if (DEV) signal.label = label ?? fn.toString(); @@ -327,9 +320,9 @@ export function destroy_derived_effects(derived) { /** * The currently updating deriveds, used to detect infinite recursion * in dev mode and provide a nicer error than 'too much recursion' - * @type {Derived[]} + * @type {Derived[] | null} */ -let stack = []; +export let derived_stack = null; /** * @template T @@ -354,31 +347,32 @@ export function execute_derived(derived) { set_active_effect(parent); + derived_stack ??= []; + if (DEV) { + // TODO don't we need eager effects in prod too? let prev_eager_effects = eager_effects; set_eager_effects(new Set()); - try { - if (includes.call(stack, derived)) { - e.derived_references_self(); - } - - stack.push(derived); + try { + derived_stack.push(derived); derived.f &= ~WAS_MARKED; destroy_derived_effects(derived); value = update_reaction(derived); } finally { set_active_effect(prev_active_effect); set_eager_effects(prev_eager_effects); - stack.pop(); + derived_stack.pop(); } } else { try { + derived_stack.push(derived); derived.f &= ~WAS_MARKED; destroy_derived_effects(derived); value = update_reaction(derived); } finally { set_active_effect(prev_active_effect); + derived_stack.pop(); } } @@ -392,51 +386,39 @@ export function execute_derived(derived) { export function update_derived(derived) { var value = execute_derived(derived); - if (!derived.equals(value)) { - derived.wv = increment_write_version(); - - // in a fork, we don't update the underlying value, just `batch_values`. - // the underlying value will be updated when the fork is committed. - // otherwise, the next time we get here after a 'real world' state - // change, `derived.equals` may incorrectly return `true` - if (!current_batch?.is_fork || derived.deps === null) { - if (current_batch !== null) { - // We also write to previous_batch because if it exists, it is a sign that we're - // currently in the process of flushing effects. These updates to deriveds may belong - // to the previous batch, not the new one (which can already exist if an earlier - // effect wrote to a source). This can cause bugs when running batch.#commit() later, - // but not adding it to current_batch can, too, so we add it to both. - // See https://github.com/sveltejs/svelte/pull/18117 for more details. - current_batch.capture(derived, value, true); - previous_batch?.capture(derived, value, true); - } else { - derived.v = value; - } + var deps = derived.deps; + var cv = Infinity; - // deriveds without dependencies should never be recomputed - if (derived.deps === null) { - set_signal_status(derived, CLEAN); - return; - } + if (deps !== null) { + cv = -Infinity; + + for (var i = 0; i < deps.length; i++) { + var dep_wv = get_wv(deps[i]); + if (dep_wv > cv) cv = dep_wv; } } - // don't mark derived clean if we're reading it inside a - // cleanup function, or it will cache a stale value - if (is_destroying_effect) { - return; - } + set_cv(derived, cv); - // During time traveling we don't want to reset the status so that - // traversal of the graph in the other batches still happens - if (batch_values !== null) { - // only cache the value if we're in a tracking context, otherwise we won't - // clear the cache in `mark_reactions` when dependencies are updated - if (effect_tracking() || current_batch?.is_fork) { - batch_values.set(derived, value); + if (!derived.equals(value)) { + if (active_batch !== null) { + (current_batch ?? active_batch).capture(derived, value, write_version); + + // We also write to previous_batch because if it exists, it is a sign that we're + // currently in the process of flushing effects. These updates to deriveds may belong + // to the previous batch, not the new one (which can already exist if an earlier + // effect wrote to a source). This can cause bugs when running batch.#commit() later, + // but not adding it to current_batch can, too, so we add it to both. + // See https://github.com/sveltejs/svelte/pull/18117 for more details. + previous_batch?.capture(derived, value, write_version); + } else { + derived.v = value; + derived.wv = write_version; } - } else { - update_derived_status(derived); + } + + if (active_batch === null && (derived.f & CONNECTED) !== 0) { + derived.f |= CLEAN; } } diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index c5d195dfae..a5ea5fc78d 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -14,7 +14,6 @@ import { set_active_effect } from '../runtime.js'; import { - DIRTY, BRANCH_EFFECT, RENDER_EFFECT, EFFECT, @@ -27,7 +26,6 @@ import { CLEAN, EAGER_EFFECT, HEAD_EFFECT, - MAYBE_DIRTY, EFFECT_PRESERVED, STALE_REACTION, USER_EFFECT, @@ -44,7 +42,6 @@ import { component_context, dev_current_component_function, dev_stack } from '.. import { Batch, collected_effects, current_batch } from './batch.js'; import { flatten } from './async.js'; import { without_reactive_context } from '../dom/elements/bindings/shared.js'; -import { set_signal_status } from './status.js'; /** * @param {'$effect' | '$effect.pre' | '$inspect'} rune @@ -102,7 +99,7 @@ function create_effect(type, fn) { ctx: component_context, deps: null, nodes: null, - f: type | DIRTY | CONNECTED, + f: type | CLEAN | CONNECTED, first: null, fn, last: null, @@ -111,7 +108,7 @@ function create_effect(type, fn) { b: parent && parent.b, prev: null, teardown: null, - wv: 0, + cv: -1, ac: null }; @@ -191,7 +188,6 @@ export function effect_tracking() { */ export function teardown(fn) { const effect = create_effect(RENDER_EFFECT, null); - set_signal_status(effect, CLEAN); effect.teardown = fn; return effect; } @@ -348,12 +344,6 @@ export function legacy_pre_effect_reset() { var effect = token.effect; - // If the effect is CLEAN, then make it MAYBE_DIRTY. This ensures we traverse through - // the effects dependencies and correctly ensure each dependency is up-to-date. - if ((effect.f & CLEAN) !== 0 && effect.deps !== null) { - set_signal_status(effect, MAYBE_DIRTY); - } - if (is_dirty(effect)) { update_effect(effect); } @@ -693,7 +683,6 @@ function resume_children(effect, local) { // here because we don't want to eagerly recompute a derived like // `{#if foo}{foo.bar()}{/if}` if `foo` is now `undefined if ((effect.f & CLEAN) === 0) { - set_signal_status(effect, DIRTY); Batch.ensure().schedule(effect); // Assumption: This happens during the commit phase of the batch, causing another flush, but it's safe } diff --git a/packages/svelte/src/internal/client/reactivity/equality.js b/packages/svelte/src/internal/client/reactivity/equality.js index 1041238573..9f13bdc8c6 100644 --- a/packages/svelte/src/internal/client/reactivity/equality.js +++ b/packages/svelte/src/internal/client/reactivity/equality.js @@ -1,8 +1,10 @@ /** @import { Equals } from '#client' */ +import { active_batch } from './batch.js'; /** @type {Equals} */ export function equals(value) { - return value === this.v; + var snapshot = active_batch?.values?.get(this); + return value === (snapshot ? snapshot.v : this.v); } /** diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 218941ab67..2514163fdd 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -7,28 +7,28 @@ import { get, set_untracked_writes, untrack, - increment_write_version, update_effect, current_sources, is_dirty, untracking, is_destroying_effect, - push_reaction_value + push_reaction_value, + write_version, + increment_write_version } from '../runtime.js'; import { equals, safe_equals } from './equality.js'; import { CLEAN, DERIVED, - DIRTY, BRANCH_EFFECT, EAGER_EFFECT, - MAYBE_DIRTY, BLOCK_EFFECT, ROOT_EFFECT, ASYNC, WAS_MARKED, CONNECTED, - REACTION_IS_UPDATING + STATE_EAGER_EFFECT, + REACTION_RAN } from '#client/constants'; import * as e from '../errors.js'; import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js'; @@ -37,14 +37,15 @@ import { get_error } from '../../shared/dev.js'; import { component_context, is_runes } from '../context.js'; import { Batch, - batch_values, eager_block_effects, - schedule_effect, - legacy_updates + legacy_updates, + set_cv, + get_cv, + active_batch } from './batch.js'; import { proxy } from '../proxy.js'; import { execute_derived } from './deriveds.js'; -import { set_signal_status, update_derived_status } from './status.js'; +import { UNINITIALIZED } from '../../../constants.js'; /** @type {Set} */ export let eager_effects = new Set(); @@ -86,7 +87,6 @@ export function source(v, stack) { if (DEV && tracing_mode_flag) { signal.created = stack ?? get_error('created at'); signal.updated = null; - signal.set_during_effect = false; signal.trace = null; } @@ -183,7 +183,19 @@ export function internal_set(source, value, updated_during_traversal = null) { old_values.set(source, is_destroying_effect ? value : source.v); var batch = Batch.ensure(); - batch.capture(source, value); + + if ((source.f & DERIVED) !== 0) { + const derived = /** @type {Derived} */ (source); + + if (derived.v === UNINITIALIZED) { + // assigning before first read — execute to track dependencies + execute_derived(derived); + } + + set_cv(derived); + } + + batch.capture(source, value, increment_write_version()); if (DEV) { if (tracing_mode_flag || active_effect !== null) { @@ -209,32 +221,11 @@ export function internal_set(source, value, updated_during_traversal = null) { } } } - - if (active_effect !== null) { - source.set_during_effect = true; - } } - if ((source.f & DERIVED) !== 0) { - const derived = /** @type {Derived} */ (source); - - // if we are assigning to a dirty derived we set it to clean/maybe dirty but we also eagerly execute it to track the dependencies - if ((source.f & DIRTY) !== 0) { - execute_derived(derived); - } - - // During time traveling we don't want to reset the status so that - // traversal of the graph in the other batches still happens - if (batch_values === null) { - update_derived_status(derived); - } - } - - source.wv = increment_write_version(); - // For debugging, in case you want to know which reactions are being scheduled: // log_reactions(source); - mark_reactions(source, DIRTY, updated_during_traversal); + mark_reactions(batch, source, write_version, updated_during_traversal); // It's possible that the current reaction might not have up-to-date dependencies // whilst it's actively running. So in the case of ensuring it registers the reaction @@ -265,16 +256,10 @@ export function flush_eager_effects() { eager_effects_deferred = false; for (const effect of eager_effects) { - // Mark clean inspect-effects as maybe dirty and then check their dirtiness - // instead of just updating the effects - this way we avoid overfiring. - if ((effect.f & CLEAN) !== 0) { - set_signal_status(effect, MAYBE_DIRTY); - } - let dirty; try { - dirty = is_dirty(effect); + dirty = (effect.f & STATE_EAGER_EFFECT) !== 0 || is_dirty(effect); } catch { // Dirty-checking can evaluate derived dependencies and throw in cases where // parent effects are about to destroy this eager effect. Run the effect so @@ -329,12 +314,14 @@ export function increment(source) { } /** + * TODO this should probably be a method on `batch` + * @param {Batch} batch * @param {Value} signal - * @param {number} status should be DIRTY or MAYBE_DIRTY + * @param {number} wv * @param {Effect[] | null} updated_during_traversal * @returns {void} */ -function mark_reactions(signal, status, updated_during_traversal) { +export function mark_reactions(batch, signal, wv, updated_during_traversal) { var reactions = signal.reactions; if (reactions === null) return; @@ -348,12 +335,9 @@ function mark_reactions(signal, status, updated_during_traversal) { // In legacy mode, skip the current effect to prevent infinite loops if (!runes && reaction === active_effect) continue; - var not_dirty = (flags & DIRTY) === 0; - - // don't set a DIRTY reaction to MAYBE_DIRTY - if (not_dirty) { - set_signal_status(reaction, status); - } + // TODO ideally this would work, but I think we need to `apply()` before `mark_reactions`. + // Or pass `batch` in as an argument? + // if (wv <= get_cv(reaction)) continue; if ((flags & EAGER_EFFECT) !== 0) { // Eager effects need to run immediately: @@ -363,20 +347,29 @@ function mark_reactions(signal, status, updated_during_traversal) { } else if ((flags & DERIVED) !== 0) { var derived = /** @type {Derived} */ (reaction); - batch_values?.delete(derived); + if (wv > get_cv(derived)) { + // If setting state inside an effect, `batch !== active_batch` — + // we need to invalidate the current overlay so that subsequent + // effects read the correct value + active_batch?.values?.delete(derived); + + batch.current.delete(derived); + derived.f &= ~CLEAN; + + if ((flags & WAS_MARKED) === 0) { + // Only connected deriveds can be reliably unmarked right away + if ( + flags & CONNECTED && + active_reaction !== null && + (active_reaction.f & REACTION_RAN) !== 0 + ) { + reaction.f |= WAS_MARKED; + } - if ((flags & WAS_MARKED) === 0) { - // Only connected deriveds being executed outside the update cycle can be reliably unmarked right away - if ( - flags & CONNECTED && - (active_effect === null || (active_effect.f & REACTION_IS_UPDATING) === 0) - ) { - reaction.f |= WAS_MARKED; + mark_reactions(batch, derived, wv, updated_during_traversal); } - - mark_reactions(derived, MAYBE_DIRTY, updated_during_traversal); } - } else if (not_dirty) { + } else { var effect = /** @type {Effect} */ (reaction); if ((flags & BLOCK_EFFECT) !== 0 && eager_block_effects !== null) { @@ -386,7 +379,7 @@ function mark_reactions(signal, status, updated_during_traversal) { if (updated_during_traversal !== null) { updated_during_traversal.push(effect); } else { - schedule_effect(effect); + batch.schedule(effect); } } } diff --git a/packages/svelte/src/internal/client/reactivity/status.js b/packages/svelte/src/internal/client/reactivity/status.js deleted file mode 100644 index 024285e73a..0000000000 --- a/packages/svelte/src/internal/client/reactivity/status.js +++ /dev/null @@ -1,25 +0,0 @@ -/** @import { Derived, Signal } from '#client' */ -import { CLEAN, CONNECTED, DIRTY, MAYBE_DIRTY } from '#client/constants'; - -const STATUS_MASK = ~(DIRTY | MAYBE_DIRTY | CLEAN); - -/** - * @param {Signal} signal - * @param {number} status - */ -export function set_signal_status(signal, status) { - signal.f = (signal.f & STATUS_MASK) | status; -} - -/** - * Set a derived's status to CLEAN or MAYBE_DIRTY based on its connection state. - * @param {Derived} derived - */ -export function update_derived_status(derived) { - // Only mark as MAYBE_DIRTY if disconnected and has dependencies. - if ((derived.f & CONNECTED) !== 0 || derived.deps === null) { - set_signal_status(derived, CLEAN); - } else { - set_signal_status(derived, MAYBE_DIRTY); - } -} diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index 0ee8570c3d..72065a5ce1 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -11,8 +11,6 @@ import type { Boundary } from '../dom/blocks/boundary'; export interface Signal { /** Flags bitmask */ f: number; - /** Write version */ - wv: number; } export interface Value extends Signal { @@ -20,11 +18,8 @@ export interface Value extends Signal { equals: Equals; /** Signals that read from this signal */ reactions: null | Reaction[]; - /** Read version */ - rv: number; /** The latest value for this signal */ v: V; - // dev-only /** A label (e.g. the `foo` in `let foo = $state(...)`) used for `$inspect.trace()` */ label?: string; @@ -32,13 +27,12 @@ export interface Value extends Signal { created?: Error | null; /** An map of errors with stack traces showing when the source was updated, keyed by the stack trace */ updated?: Map | null; - /** - * Whether or not the source was set while running an effect — if so, we need to - * increment the write version so that it shows up as dirty when the effect re-runs - */ - set_during_effect?: boolean; /** A function that retrieves the underlying source, used for each block item signals */ trace?: null | (() => void); + /** Write version */ + wv: number; + /** Read version */ + rv: number; } export interface Reaction extends Signal { @@ -50,6 +44,8 @@ export interface Reaction extends Signal { deps: null | Value[]; /** An AbortController that aborts when the signal is destroyed */ ac: null | AbortController; + /** Check version */ + cv: number; } export interface Derived extends Value, Reaction { @@ -108,3 +104,8 @@ export interface Blocker { promise: Promise; settled: boolean; } + +export interface ValueSnapshot { + v: T; + wv: number; +} diff --git a/packages/svelte/src/internal/client/reactivity/utils.js b/packages/svelte/src/internal/client/reactivity/utils.js index 0d27cb8b84..daf29204ea 100644 --- a/packages/svelte/src/internal/client/reactivity/utils.js +++ b/packages/svelte/src/internal/client/reactivity/utils.js @@ -1,6 +1,5 @@ /** @import { Derived, Effect, Value } from '#client' */ -import { CLEAN, DERIVED, DIRTY, MAYBE_DIRTY, WAS_MARKED } from '#client/constants'; -import { set_signal_status } from './status.js'; +import { DERIVED, WAS_MARKED } from '#client/constants'; /** * @param {Value[] | null} deps @@ -22,19 +21,11 @@ function clear_marked(deps) { /** * @param {Effect} effect * @param {Set} dirty_effects - * @param {Set} maybe_dirty_effects */ -export function defer_effect(effect, dirty_effects, maybe_dirty_effects) { - if ((effect.f & DIRTY) !== 0) { - dirty_effects.add(effect); - } else if ((effect.f & MAYBE_DIRTY) !== 0) { - maybe_dirty_effects.add(effect); - } +export function defer_effect(effect, dirty_effects) { + dirty_effects.add(effect); // Since we're not executing these effects now, we need to clear any WAS_MARKED flags // so that other batches can correctly reach these effects during their own traversal clear_marked(effect.deps); - - // mark as clean so they get scheduled if they depend on pending async state - set_signal_status(effect, CLEAN); } diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index 50832fb3ff..7e9f80b530 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -172,13 +172,14 @@ function _mount( var unmount = component_root(() => { var anchor_node = anchor ?? target.appendChild(create_text()); + push({}); + boundary( /** @type {TemplateNode} */ (anchor_node), { pending: () => {} }, (anchor_node) => { - push({}); var ctx = /** @type {ComponentContext} */ (component_context); if (context) ctx.c = context; @@ -208,12 +209,12 @@ function _mount( throw HYDRATION_ERROR; } } - - pop(); }, transformError ); + pop(); + // Setup event delegation _after_ component is mounted - if an error would happen during mount, it would otherwise not be cleaned up /** @type {Set} */ var registered_events = new Set(); diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 8d70eee43c..b4dcc04b30 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -8,9 +8,6 @@ import { execute_effect_teardown } from './reactivity/effects.js'; import { - DIRTY, - MAYBE_DIRTY, - CLEAN, DERIVED, DESTROYED, BRANCH_EFFECT, @@ -23,7 +20,9 @@ import { ERROR_VALUE, WAS_MARKED, MANAGED_EFFECT, - REACTION_RAN + REACTION_RAN, + EFFECT_LEGACY, + CLEAN } from './constants.js'; import { old_values } from './reactivity/sources.js'; import { @@ -32,7 +31,8 @@ import { freeze_derived_effects, recent_async_deriveds, unfreeze_derived_effects, - update_derived + update_derived, + derived_stack } from './reactivity/deriveds.js'; import { async_mode_flag, tracing_mode_flag } from '../flags/index.js'; import { tracing_expressions } from './dev/tracing.js'; @@ -48,17 +48,19 @@ import { } from './context.js'; import { Batch, - batch_values, current_batch, flushSync, - schedule_effect + get_cv, + active_batch, + set_cv, + get_wv } from './reactivity/batch.js'; import { handle_error } from './error-handling.js'; import { UNINITIALIZED } from '../../constants.js'; import { captured_signals } from './legacy.js'; import { without_reactive_context } from './dom/elements/bindings/shared.js'; -import { set_signal_status, update_derived_status } from './reactivity/status.js'; import * as w from './warnings.js'; +import * as e from './errors.js'; let is_updating_effect = false; @@ -144,45 +146,58 @@ export function increment_write_version() { } /** - * Determines whether a derived or effect is dirty. - * If it is MAYBE_DIRTY, will set the status to CLEAN + * Determines whether a reaction is dirty * @param {Reaction} reaction * @returns {boolean} */ export function is_dirty(reaction) { var flags = reaction.f; - if ((flags & DIRTY) !== 0) { + if ((flags & REACTION_IS_UPDATING) !== 0) { + return false; + } + + if ((flags & REACTION_RAN) === 0) { return true; } if (flags & DERIVED) { reaction.f &= ~WAS_MARKED; + + if ((flags & CONNECTED) !== 0) { + if (active_batch !== null) { + if (active_batch.values?.has(/** @type {Derived} */ (reaction))) { + return false; + } + } else { + if ((reaction.f & CLEAN) !== 0) { + return false; + } + } + } } - if ((flags & MAYBE_DIRTY) !== 0) { - var dependencies = /** @type {Value[]} */ (reaction.deps); - var length = dependencies.length; + var dependencies = /** @type {Value[]} */ (reaction.deps); - for (var i = 0; i < length; i++) { - var dependency = dependencies[i]; + if (dependencies === null) { + return false; + } + + var cv = get_cv(reaction); + + var length = dependencies.length; + + for (var i = 0; i < length; i++) { + var dependency = dependencies[i]; + if ((dependency.f & DERIVED) !== 0) { if (is_dirty(/** @type {Derived} */ (dependency))) { update_derived(/** @type {Derived} */ (dependency)); } - - if (dependency.wv > reaction.wv) { - return true; - } } - if ( - (flags & CONNECTED) !== 0 && - // During time traveling we don't want to reset the status so that - // traversal of the graph in the other batches still happens - batch_values === null - ) { - set_signal_status(reaction, CLEAN); + if (get_wv(dependency) > cv) { + return true; } } @@ -190,15 +205,15 @@ export function is_dirty(reaction) { } /** + * @param {Batch} batch * @param {Value} signal * @param {Effect} effect - * @param {boolean} [root] */ -function schedule_possible_effect_self_invalidation(signal, effect, root = true) { +function schedule_possible_effect_self_invalidation(batch, signal, effect) { var reactions = signal.reactions; if (reactions === null) return; - if (!async_mode_flag && current_sources !== null && current_sources.has(signal)) { + if (current_sources !== null && current_sources.has(signal)) { return; } @@ -206,14 +221,9 @@ function schedule_possible_effect_self_invalidation(signal, effect, root = true) var reaction = reactions[i]; if ((reaction.f & DERIVED) !== 0) { - schedule_possible_effect_self_invalidation(/** @type {Derived} */ (reaction), effect, false); + schedule_possible_effect_self_invalidation(batch, /** @type {Derived} */ (reaction), effect); } else if (effect === reaction) { - if (root) { - set_signal_status(reaction, DIRTY); - } else if ((reaction.f & CLEAN) !== 0) { - set_signal_status(reaction, MAYBE_DIRTY); - } - schedule_effect(/** @type {Effect} */ (reaction)); + batch.schedule(/** @type {Effect} */ (reaction)); } } } @@ -290,14 +300,17 @@ export function update_reaction(reaction) { // ensure that if any of those untracked writes result in re-invalidation // of the current effect, then that happens accordingly if ( + !async_mode_flag && is_runes() && untracked_writes !== null && !untracking && deps !== null && - (reaction.f & (DERIVED | MAYBE_DIRTY | DIRTY)) === 0 + (reaction.f & DERIVED) === 0 && + current_batch !== null ) { for (i = 0; i < /** @type {Source[]} */ (untracked_writes).length; i++) { schedule_possible_effect_self_invalidation( + current_batch, untracked_writes[i], /** @type {Effect} */ (reaction) ); @@ -395,15 +408,6 @@ function remove_reaction(signal, dependency) { derived.f &= ~WAS_MARKED; } - // In a fork it's possible that a derived is executed and gets reactions, then commits, but is - // never re-executed. This is possible when the derived is only executed once in the context - // of a new branch which happens before fork.commit() runs. In this case, the derived still has - // UNINITIALIZED as its value, and then when it's loosing its reactions we need to ensure it stays - // DIRTY so it is reexecuted once someone wants its value again. - if (derived.v !== UNINITIALIZED) { - update_derived_status(derived); - } - // freeze any effects inside this derived freeze_derived_effects(derived); @@ -437,8 +441,6 @@ export function update_effect(effect) { return; } - set_signal_status(effect, CLEAN); - var previous_effect = active_effect; var was_updating_effect = is_updating_effect; @@ -453,6 +455,10 @@ export function update_effect(effect) { set_dev_stack(effect.dev_stack ?? dev_stack); } + // get this now, so that any writes during execution cause a re-run, + // but don't set it yet so that `$inspect.trace` works + const cv = write_version; + try { if ((flags & (BLOCK_EFFECT | MANAGED_EFFECT)) !== 0) { destroy_block_effect_children(effect); @@ -461,21 +467,19 @@ export function update_effect(effect) { } execute_effect_teardown(effect); + var teardown = update_reaction(effect); effect.teardown = typeof teardown === 'function' ? teardown : null; - effect.wv = write_version; - - // In DEV, increment versions of any sources that were written to during the effect, - // so that they are correctly marked as dirty when the effect re-runs - if (DEV && tracing_mode_flag && (effect.f & DIRTY) !== 0 && effect.deps !== null) { - for (var dep of effect.deps) { - if (dep.set_during_effect) { - dep.wv = increment_write_version(); - dep.set_during_effect = false; - } + } finally { + if (effect.deps !== null) { + if (is_runes() && (effect.f & EFFECT_LEGACY) === 0) { + set_cv(effect, cv); + } else { + // in legacy mode, prevent the effect re-running immediately + set_cv(effect); } } - } finally { + is_updating_effect = was_updating_effect; active_effect = previous_effect; @@ -636,15 +640,16 @@ export function get(signal) { if (is_derived) { var derived = /** @type {Derived} */ (signal); + if (derived_stack !== null && includes.call(derived_stack, derived)) { + e.derived_references_self(); + } + if (is_destroying_effect) { var value = derived.v; // if the derived is dirty and has reactions, or depends on the values that just changed, re-execute - // (a derived can be maybe_dirty due to the effect destroy removing its last reaction) - if ( - ((derived.f & CLEAN) === 0 && derived.reactions !== null) || - depends_on_old_values(derived) - ) { + // (a derived can be dirty due to the effect destroy removing its last reaction) + if ((is_dirty(derived) && derived.reactions !== null) || depends_on_old_values(derived)) { value = execute_derived(derived); } @@ -679,9 +684,8 @@ export function get(signal) { } } - if (batch_values?.has(signal)) { - return batch_values.get(signal); - } + var snapshot = active_batch?.values?.get(signal); + if (snapshot) return /** @type {V} */ (snapshot.v); if ((signal.f & ERROR_VALUE) !== 0) { throw signal.v; diff --git a/packages/svelte/src/legacy/legacy-client.js b/packages/svelte/src/legacy/legacy-client.js index 801c126b2c..adf11eb988 100644 --- a/packages/svelte/src/legacy/legacy-client.js +++ b/packages/svelte/src/legacy/legacy-client.js @@ -1,9 +1,16 @@ /** @import { ComponentConstructorOptions, ComponentType, SvelteComponent, Component } from 'svelte' */ -import { DIRTY, LEGACY_PROPS, MAYBE_DIRTY } from '../internal/client/constants.js'; +/** @import { Derived, Value } from '#client' */ +import { DERIVED, EFFECT_LEGACY, LEGACY_PROPS } from '../internal/client/constants.js'; import { user_pre_effect } from '../internal/client/reactivity/effects.js'; import { mutable_source, set } from '../internal/client/reactivity/sources.js'; import { hydrate, mount, unmount } from '../internal/client/render.js'; -import { active_effect, get } from '../internal/client/runtime.js'; +import { + active_effect, + get, + new_deps, + skipped_deps, + untracked_writes +} from '../internal/client/runtime.js'; import { flushSync } from '../internal/client/reactivity/batch.js'; import { define_property, is_array } from '../internal/shared/utils.js'; import * as e from '../internal/client/errors.js'; @@ -12,7 +19,6 @@ import { DEV } from 'esm-env'; import { FILENAME } from '../constants.js'; import { component_context, dev_current_component_function } from '../internal/client/context.js'; import { async_mode_flag } from '../internal/flags/index.js'; -import { set_signal_status } from '../internal/client/reactivity/status.js'; /** * Takes the same options as a Svelte 4 component and the component function and returns a Svelte 4 compatible component. @@ -189,17 +195,37 @@ class Svelte4Component { */ export function run(fn) { user_pre_effect(() => { - fn(); var effect = /** @type {import('#client').Effect} */ (active_effect); - // If the effect is immediately made dirty again, mark it as maybe dirty to emulate legacy behaviour - if ((effect.f & DIRTY) !== 0) { - let filename = "a file (we can't know which one)"; - if (DEV) { - // @ts-ignore - filename = dev_current_component_function?.[FILENAME] ?? filename; + + // we need this to prevent it from re-running + effect.f |= EFFECT_LEGACY; + + fn(); + + if (untracked_writes !== null) { + /** @type {Set} */ + const all_deps = new Set([...(effect.deps ?? [].slice(skipped_deps)), ...(new_deps ?? [])]); + + /** @param {Value} dep */ + const add_deps = (dep) => { + all_deps.add(dep); + + if ((dep.f & DERIVED) !== 0) { + const deps = /** @type {Derived} */ (dep).deps; + if (deps !== null) deps.forEach(add_deps); + } + }; + + for (const dep of all_deps) { + if (untracked_writes.includes(dep)) { + let filename = "a file (we can't know which one)"; + if (DEV) { + // @ts-ignore + filename = dev_current_component_function?.[FILENAME] ?? filename; + } + w.legacy_recursive_reactive_block(filename); + } } - w.legacy_recursive_reactive_block(filename); - set_signal_status(effect, MAYBE_DIRTY); } }); } diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index 927ce2e665..c9cecee200 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -599,7 +599,7 @@ describe('signals', () => { return () => { flushSync(); - assert.deepEqual(log, [20]); + assert.deepEqual(log, [20, 20]); }; });