fix: prevent untracked derived reads from retaining dependencies (#18829)

Fixes https://github.com/sveltejs/svelte/issues/18827.

A disconnected derived re-evaluated through `untrack` inside an effect
could connect descendant deriveds because `is_updating_effect` was true,
even though the active derived reader was not connected. Those
descendants registered on long-lived dependencies without gaining a
reaction that could later trigger their disconnect cascade.

This changes `get()` to connect a derived only when its active reader is
itself `CONNECTED`. Effects already carry that flag, and deriveds
reached through connected readers receive it before evaluation, so
tracked behavior remains intact. The now-unused `is_updating_effect`
state is removed.
pull/18871/merge
svelte-triage-bot[bot] 5 days ago committed by GitHub
parent a956c2bd1f
commit f908535659
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: prevent untracked derived reads from retaining disconnected dependencies

@ -60,11 +60,6 @@ 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';
/**
* True if updating in an effect context that is reactive (i.e. not branch/root effects)
*/
let is_updating_effect = false;
export let is_destroying_effect = false;
/** @param {boolean} value */
@ -462,10 +457,8 @@ export function update_effect(effect) {
set_signal_status(effect, CLEAN);
var previous_effect = active_effect;
var was_updating_effect = is_updating_effect;
active_effect = effect;
is_updating_effect = (flags & (BRANCH_EFFECT | ROOT_EFFECT)) === 0; // Branch/root effects are not reactive contexts
if (DEV) {
var previous_component_fn = dev_current_component_function;
@ -498,7 +491,6 @@ export function update_effect(effect) {
}
}
} finally {
is_updating_effect = was_updating_effect;
active_effect = previous_effect;
if (DEV) {
@ -680,13 +672,12 @@ export function get(signal) {
return value;
}
// connect disconnected deriveds if we are reading them inside an effect,
// or inside another derived that is already connected
// connect disconnected deriveds when reading them inside a connected reaction
var should_connect =
(derived.f & CONNECTED) === 0 &&
!untracking &&
active_reaction !== null &&
(is_updating_effect || (active_reaction.f & CONNECTED) !== 0);
(active_reaction.f & CONNECTED) !== 0;
var is_new = (derived.f & REACTION_RAN) === 0;

@ -15,7 +15,7 @@ import { proxy } from '../../src/internal/client/proxy';
import { derived } from '../../src/internal/client/reactivity/deriveds';
import { snapshot } from '../../src/internal/shared/clone.js';
import { SvelteSet } from '../../src/reactivity/set';
import { DESTROYED } from '../../src/internal/client/constants';
import { CONNECTED, DESTROYED } from '../../src/internal/client/constants';
import { noop } from 'svelte/internal/client';
import { disable_async_mode_flag, enable_async_mode_flag } from '../../src/internal/flags';
@ -1516,12 +1516,68 @@ describe('signals', () => {
destroy();
// a was spuriously added to s.reactions via is_updating_effect
// a was spuriously added to s.reactions
// even though the entire derived chain was read in an untracked context
assert.equal(s.reactions, null);
};
});
test('untracked derived reads inside effects do not reconnect disconnected dependencies', () => {
return () => {
const source = state({ n: 1, items: [1] });
const data = derived(() => $.get(source));
const items = derived(() => $.get(data).items);
const count = derived(() => Math.max(1, $.get(items).length));
const snapshot = derived(() => ({ n: $.get(data).n, count: $.get(count) }));
const show = state(true);
const trigger = state(0);
let rendered = -1;
let seen: { n: number; count: number } | undefined;
const destroy = effect_root(() => {
render_effect(() => {
if ($.get(show)) {
render_effect(() => {
rendered = $.get(snapshot).count;
});
}
});
render_effect(() => {
$.get(trigger);
seen = $.untrack(() => $.get(snapshot));
});
});
flushSync();
assert.equal(rendered, 1);
flushSync(() => set(show, false));
assert.equal(source.reactions, null);
flushSync(() => set(source, { n: 2, items: [1, 2] }));
flushSync(() => set(trigger, 1));
assert.deepEqual(seen, { n: 2, count: 2 });
assert.equal(source.reactions, null);
assert.equal(items.reactions, null);
assert.equal(count.reactions, null);
assert.equal(items.f & CONNECTED, 0);
assert.equal(count.f & CONNECTED, 0);
flushSync(() => set(show, true));
assert.equal(rendered, 2);
assert.equal(source.reactions?.length, 1);
flushSync(() => set(source, { n: 3, items: [1] }));
assert.equal(rendered, 1);
destroy();
flushSync();
assert.equal(source.reactions, null);
};
});
// https://github.com/sveltejs/svelte/issues/18414
test('a reaction that throws after first-reading a fresh derived does not leak it', () => {
const src = state(0);

Loading…
Cancel
Save