diff --git a/.changeset/ninety-emus-drop.md b/.changeset/ninety-emus-drop.md new file mode 100644 index 0000000000..94c7a75148 --- /dev/null +++ b/.changeset/ninety-emus-drop.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: freeze effects-inside-deriveds when disconnecting, unfreeze on reconnect diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index a1bdb8a985..451f8191ae 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -27,10 +27,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; // Flags exclusive to effects -/** Set once an effect that should run synchronously has run */ -export const EFFECT_RAN = 1 << 15; /** * 'Transparent' effects do not create a transition boundary. * This is on a block effect 99% of the time but may also be on a branch effect if its parent block effect was pruned @@ -48,7 +48,7 @@ export const EFFECT_OFFSCREEN = 1 << 25; * Will be lifted during execution of the derived and during checking its dirty state (both are necessary * because a derived might be checked but not executed). */ -export const WAS_MARKED = 1 << 15; +export const WAS_MARKED = 1 << 16; // Flags used for async export const REACTION_IS_UPDATING = 1 << 21; diff --git a/packages/svelte/src/internal/client/context.js b/packages/svelte/src/internal/client/context.js index ffdb342adb..36c735272c 100644 --- a/packages/svelte/src/internal/client/context.js +++ b/packages/svelte/src/internal/client/context.js @@ -5,7 +5,7 @@ import { active_effect, active_reaction } from './runtime.js'; import { create_user_effect } from './reactivity/effects.js'; import { async_mode_flag, legacy_mode_flag } from '../flags/index.js'; import { FILENAME } from '../../constants.js'; -import { BRANCH_EFFECT, EFFECT_RAN } from './constants.js'; +import { BRANCH_EFFECT, REACTION_RAN } from './constants.js'; /** @type {ComponentContext | null} */ export let component_context = null; diff --git a/packages/svelte/src/internal/client/dom/elements/transitions.js b/packages/svelte/src/internal/client/dom/elements/transitions.js index 170b40780a..0c5431f030 100644 --- a/packages/svelte/src/internal/client/dom/elements/transitions.js +++ b/packages/svelte/src/internal/client/dom/elements/transitions.js @@ -5,7 +5,7 @@ import { active_effect, untrack } from '../../runtime.js'; import { loop } from '../../loop.js'; import { should_intro } from '../../render.js'; import { TRANSITION_GLOBAL, TRANSITION_IN, TRANSITION_OUT } from '../../../../constants.js'; -import { BLOCK_EFFECT, EFFECT_RAN, EFFECT_TRANSPARENT } from '#client/constants'; +import { BLOCK_EFFECT, REACTION_RAN, EFFECT_TRANSPARENT } from '#client/constants'; import { queue_micro_task } from '../task.js'; import { without_reactive_context } from './bindings/shared.js'; @@ -289,7 +289,7 @@ export function transition(flags, element, get_fn, get_params) { } } - run = !block || (block.f & EFFECT_RAN) !== 0; + run = !block || (block.f & REACTION_RAN) !== 0; } if (run) { diff --git a/packages/svelte/src/internal/client/dom/operations.js b/packages/svelte/src/internal/client/dom/operations.js index 876f039a0e..4036aa2d61 100644 --- a/packages/svelte/src/internal/client/dom/operations.js +++ b/packages/svelte/src/internal/client/dom/operations.js @@ -5,7 +5,7 @@ import { init_array_prototype_warnings } from '../dev/equality.js'; import { get_descriptor, is_extensible } from '../../shared/utils.js'; import { active_effect } from '../runtime.js'; import { async_mode_flag } from '../../flags/index.js'; -import { TEXT_NODE, EFFECT_RAN } from '#client/constants'; +import { TEXT_NODE, REACTION_RAN } from '#client/constants'; import { eager_block_effects } from '../reactivity/batch.js'; import { NAMESPACE_HTML } from '../../../constants.js'; @@ -228,7 +228,7 @@ export function should_defer_append() { if (eager_block_effects !== null) return false; var flags = /** @type {Effect} */ (active_effect).f; - return (flags & EFFECT_RAN) !== 0; + return (flags & REACTION_RAN) !== 0; } /** diff --git a/packages/svelte/src/internal/client/dom/template.js b/packages/svelte/src/internal/client/dom/template.js index 9c2f2fe82f..b0801d7959 100644 --- a/packages/svelte/src/internal/client/dom/template.js +++ b/packages/svelte/src/internal/client/dom/template.js @@ -22,7 +22,7 @@ import { TEMPLATE_USE_MATHML, TEMPLATE_USE_SVG } from '../../../constants.js'; -import { COMMENT_NODE, DOCUMENT_FRAGMENT_NODE, EFFECT_RAN, TEXT_NODE } from '#client/constants'; +import { COMMENT_NODE, DOCUMENT_FRAGMENT_NODE, REACTION_RAN, TEXT_NODE } from '#client/constants'; /** * @param {TemplateNode} start @@ -353,7 +353,7 @@ export function append(anchor, dom) { // When hydrating and outer component and an inner component is async, i.e. blocked on a promise, // then by the time the inner resolves we have already advanced to the end of the hydrated nodes // of the parent component. Check for defined for that reason to avoid rewinding the parent's end marker. - if ((effect.f & EFFECT_RAN) === 0 || effect.nodes.end === null) { + if ((effect.f & REACTION_RAN) === 0 || effect.nodes.end === null) { effect.nodes.end = hydrate_node; } diff --git a/packages/svelte/src/internal/client/error-handling.js b/packages/svelte/src/internal/client/error-handling.js index dcbbf14e20..4736333d1a 100644 --- a/packages/svelte/src/internal/client/error-handling.js +++ b/packages/svelte/src/internal/client/error-handling.js @@ -3,7 +3,7 @@ import { DEV } from 'esm-env'; import { FILENAME } from '../../constants.js'; import { is_firefox } from './dom/operations.js'; -import { ERROR_VALUE, BOUNDARY_EFFECT, EFFECT_RAN } from './constants.js'; +import { ERROR_VALUE, BOUNDARY_EFFECT, REACTION_RAN } from './constants.js'; import { define_property, get_descriptor } from '../shared/utils.js'; import { active_effect, active_reaction } from './runtime.js'; @@ -25,7 +25,7 @@ export function handle_error(error) { adjustments.set(error, get_adjustments(error, effect)); } - if ((effect.f & EFFECT_RAN) === 0) { + if ((effect.f & REACTION_RAN) === 0) { // if the error occurred while creating this subtree, we let it // bubble up until it hits a boundary that can handle it if ((effect.f & BOUNDARY_EFFECT) === 0) { diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 9bf93c873f..82a2d4f484 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -701,16 +701,15 @@ function flush_queued_effects(effects) { // don't know if we need to keep them until they are executed. Doing the check // here (rather than in `update_effect`) allows us to skip the work for // immediate effects. - if (effect.deps === null && effect.first === null && effect.nodes === null) { - // if there's no teardown or abort controller we completely unlink - // the effect from the graph - if (effect.teardown === null && effect.ac === null) { - // remove this effect from the graph - unlink_effect(effect); - } else { - // keep the effect in the graph, but free up some memory - effect.fn = null; - } + if ( + effect.deps === null && + effect.first === null && + effect.nodes === null && + effect.teardown === null && + effect.ac === null + ) { + // remove this effect from the graph + unlink_effect(effect); } // If update_effect() has a flushSync() in it, we may have flushed another flush_queued_effects(), diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index bb732bdd1e..d11854fc91 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -19,12 +19,20 @@ import { increment_write_version, set_active_effect, push_reaction_value, - is_destroying_effect + is_destroying_effect, + update_effect, + remove_reactions } 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, effect_tracking, teardown } from './effects.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 { get_error } from '../../shared/dev.js'; import { async_mode_flag, tracing_mode_flag } from '../../flags/index.js'; @@ -33,7 +41,7 @@ import { component_context } from '../context.js'; import { UNINITIALIZED } from '../../../constants.js'; import { batch_values, current_batch } from './batch.js'; import { unset_context } from './async.js'; -import { deferred, includes } from '../../shared/utils.js'; +import { deferred, includes, noop } from '../../shared/utils.js'; import { set_signal_status, update_derived_status } from './status.js'; /** @type {Effect | null} */ @@ -392,3 +400,43 @@ export function update_derived(derived) { update_derived_status(derived); } } + +/** + * @param {Derived} derived + */ +export function freeze_derived_effects(derived) { + if (derived.effects === null) return; + + for (const e of derived.effects) { + // if the effect has a teardown function or abort signal, call it + if (e.teardown || e.ac) { + e.teardown?.(); + e.ac?.abort(STALE_REACTION); + + // make it a noop so it doesn't get called again if the derived + // is unfrozen. we don't set it to `null`, because the existence + // of a teardown function is what determines whether the + // effect runs again during unfreezing + e.teardown = noop; + e.ac = null; + + remove_reactions(e, 0); + destroy_effect_children(e); + } + } +} + +/** + * @param {Derived} derived + */ +export function unfreeze_derived_effects(derived) { + if (derived.effects === null) return; + + for (const e of derived.effects) { + // if the effect was previously frozen — indicated by the presence + // of a teardown function — unfreeze it + if (e.teardown) { + update_effect(e); + } + } +} diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 5b8dee07b9..512c435a27 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -19,7 +19,7 @@ import { EFFECT, DESTROYED, INERT, - EFFECT_RAN, + REACTION_RAN, BLOCK_EFFECT, ROOT_EFFECT, EFFECT_TRANSPARENT, @@ -122,7 +122,6 @@ function create_effect(type, fn, sync) { if (sync) { try { update_effect(effect); - effect.f |= EFFECT_RAN; } catch (e) { destroy_effect(effect); throw e; @@ -206,7 +205,7 @@ export function user_effect(fn) { // Non-nested `$effect(...)` in a component should be deferred // until the component is mounted var flags = /** @type {Effect} */ (active_effect).f; - var defer = !active_reaction && (flags & BRANCH_EFFECT) !== 0 && (flags & EFFECT_RAN) === 0; + var defer = !active_reaction && (flags & BRANCH_EFFECT) !== 0 && (flags & REACTION_RAN) === 0; if (defer) { // Top-level `$effect(...)` in an unmounted component — defer until mount diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 70eeabb789..e745f7f137 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -22,13 +22,16 @@ import { STALE_REACTION, ERROR_VALUE, WAS_MARKED, - MANAGED_EFFECT + MANAGED_EFFECT, + REACTION_RAN } from './constants.js'; import { old_values } from './reactivity/sources.js'; import { destroy_derived_effects, execute_derived, + freeze_derived_effects, recent_async_deriveds, + unfreeze_derived_effects, update_derived } from './reactivity/deriveds.js'; import { async_mode_flag, tracing_mode_flag } from '../flags/index.js'; @@ -253,6 +256,7 @@ export function update_reaction(reaction) { reaction.f |= REACTION_IS_UPDATING; var fn = /** @type {Function} */ (reaction.fn); var result = fn(); + reaction.f |= REACTION_RAN; var deps = reaction.deps; // Don't remove reactions during fork; @@ -396,8 +400,10 @@ function remove_reaction(signal, dependency) { update_derived_status(derived); + // freeze any effects inside this derived + freeze_derived_effects(derived); + // Disconnect any reactions owned by this reaction - destroy_derived_effects(derived); remove_reactions(derived, 0); } } @@ -643,7 +649,7 @@ export function get(signal) { active_reaction !== null && (is_updating_effect || (active_reaction.f & CONNECTED) !== 0); - var is_new = derived.deps === null; + var is_new = (derived.f & REACTION_RAN) === 0; if (is_dirty(derived)) { if (should_connect) { @@ -656,6 +662,7 @@ export function get(signal) { } if (should_connect && !is_new) { + unfreeze_derived_effects(derived); reconnect(derived); } } @@ -677,14 +684,15 @@ export function get(signal) { * @param {Derived} derived */ function reconnect(derived) { - if (derived.deps === null) return; - derived.f |= CONNECTED; + if (derived.deps === null) return; + for (const dep of derived.deps) { (dep.reactions ??= []).push(derived); if ((dep.f & DERIVED) !== 0 && (dep.f & CONNECTED) === 0) { + unfreeze_derived_effects(/** @type {Derived} */ (dep)); reconnect(/** @type {Derived} */ (dep)); } } diff --git a/packages/svelte/tests/runtime-runes/samples/effect-inside-derived-frozen/_config.js b/packages/svelte/tests/runtime-runes/samples/effect-inside-derived-frozen/_config.js new file mode 100644 index 0000000000..fe87a9a89f --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-inside-derived-frozen/_config.js @@ -0,0 +1,42 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + test({ assert, target, logs }) { + const [toggle, run] = target.querySelectorAll('button'); + + assert.htmlEqual( + target.innerHTML, + '

hello: 0

' + ); + + flushSync(() => run.click()); + assert.deepEqual(logs, []); + assert.htmlEqual( + target.innerHTML, + '

hello: 1

' + ); + + flushSync(() => toggle.click()); + assert.deepEqual(logs, ['aborted']); + assert.htmlEqual(target.innerHTML, ''); + + flushSync(() => run.click()); + flushSync(() => run.click()); + flushSync(() => run.click()); + + flushSync(() => toggle.click()); + assert.htmlEqual( + target.innerHTML, + '

hello: 1

' + ); + + flushSync(() => run.click()); + assert.htmlEqual( + target.innerHTML, + '

hello: 2

' + ); + + assert.deepEqual(logs, ['aborted']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/effect-inside-derived-frozen/main.svelte b/packages/svelte/tests/runtime-runes/samples/effect-inside-derived-frozen/main.svelte new file mode 100644 index 0000000000..1796c99a78 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-inside-derived-frozen/main.svelte @@ -0,0 +1,53 @@ + + + + + +{#if visible} +

{timer.text}

+{/if}