fix: freeze deriveds once their containing effects are destroyed (#17921)

Per
https://github.com/sveltejs/svelte/pull/17862#issuecomment-4049752548,
this freezes the value of a derived if it was created inside a parent
effect that is now destroyed. This prevents the sort of bug where a
derived reads `foo.bar` even though `foo` is now `undefined`.

If the derived is dirty, a warning will be printed.

This PR also gets rid of some weirdness around `derived.parent` — it can
only ever be an `Effect | null`, and there's no need for
`get_derived_parent_effect`.

Blocked on https://github.com/sveltejs/kit/pull/15533 and a follow-up
that switches remote functions to use `$effect.root` (since this
effectively undoes #17171), hence draft.

### Before submitting the PR, please make sure you do the following

- [x] It's really useful if your PR references an issue where it is
discussed ahead of time. In many cases, features are absent for a
reason. For large changes, please create an RFC:
https://github.com/sveltejs/rfcs
- [x] Prefix your PR title with `feat:`, `fix:`, `chore:`, or `docs:`.
- [x] This message body should clearly illustrate what problems it
solves.
- [x] Ideally, include a test that fails without this PR but passes with
it.
- [x] If this PR changes code within `packages/svelte/src`, add a
changeset (`npx changeset`).

### Tests and linting

- [x] Run the tests with `pnpm test` and lint the project with `pnpm
lint`

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
pull/18085/head
Rich Harris 2 weeks ago committed by GitHub
parent 3937ec03bd
commit 0e9e76f292
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: freeze deriveds once their containing effects are destroyed

@ -134,6 +134,14 @@ When logging a [proxy](https://developer.mozilla.org/en-US/docs/Web/JavaScript/R
The easiest way to log a value as it changes over time is to use the [`$inspect`](/docs/svelte/$inspect) rune. Alternatively, to log things on a one-off basis (for example, inside an event handler) you can use [`$state.snapshot`](/docs/svelte/$state#$state.snapshot) to take a snapshot of the current value.
### derived_inert
```
Reading a derived belonging to a now-destroyed effect may result in stale values
```
A `$derived` value created inside an effect will stop updating when the effect is destroyed. You should create the `$derived` outside the effect, or inside an `$effect.root`.
### event_handler_invalid
```

@ -120,6 +120,12 @@ When logging a [proxy](https://developer.mozilla.org/en-US/docs/Web/JavaScript/R
The easiest way to log a value as it changes over time is to use the [`$inspect`](/docs/svelte/$inspect) rune. Alternatively, to log things on a one-off basis (for example, inside an event handler) you can use [`$state.snapshot`](/docs/svelte/$state#$state.snapshot) to take a snapshot of the current value.
## derived_inert
> Reading a derived belonging to a now-destroyed effect may result in stale values
A `$derived` value created inside an effect will stop updating when the effect is destroyed. You should create the `$derived` outside the effect, or inside an `$effect.root`.
## event_handler_invalid
> %handler% should be a function. Did you mean to %suggestion%?

@ -12,7 +12,8 @@ import {
WAS_MARKED,
DESTROYED,
CLEAN,
REACTION_RAN
REACTION_RAN,
INERT
} from '#client/constants';
import {
active_reaction,
@ -67,10 +68,6 @@ export const recent_async_deriveds = new Set();
/*#__NO_SIDE_EFFECTS__*/
export function derived(fn) {
var flags = DERIVED | DIRTY;
var parent_derived =
active_reaction !== null && (active_reaction.f & DERIVED) !== 0
? /** @type {Derived} */ (active_reaction)
: null;
if (active_effect !== null) {
// Since deriveds are evaluated lazily, any effects created inside them are
@ -90,7 +87,7 @@ export function derived(fn) {
rv: 0,
v: /** @type {V} */ (UNINITIALIZED),
wv: 0,
parent: parent_derived ?? active_effect,
parent: active_effect,
ac: null
};
@ -320,23 +317,6 @@ export function destroy_derived_effects(derived) {
*/
let stack = [];
/**
* @param {Derived} derived
* @returns {Effect | null}
*/
function get_derived_parent_effect(derived) {
var parent = derived.parent;
while (parent !== null) {
if ((parent.f & DERIVED) === 0) {
// The original parent effect might've been destroyed but the derived
// is used elsewhere now - do not return the destroyed effect in that case
return (parent.f & DESTROYED) === 0 ? /** @type {Effect} */ (parent) : null;
}
parent = parent.parent;
}
return null;
}
/**
* @template T
* @param {Derived} derived
@ -345,8 +325,15 @@ function get_derived_parent_effect(derived) {
export function execute_derived(derived) {
var value;
var prev_active_effect = active_effect;
var parent = derived.parent;
if (!is_destroying_effect && parent !== null && (parent.f & (DESTROYED | INERT)) !== 0) {
w.derived_inert();
return derived.v;
}
set_active_effect(get_derived_parent_effect(derived));
set_active_effect(parent);
if (DEV) {
let prev_eager_effects = eager_effects;

@ -57,8 +57,8 @@ export interface Derived<V = unknown> extends Value<V>, Reaction {
fn: () => V;
/** Effects created inside this signal. Used to destroy those effects when the derived reruns or is cleaned up */
effects: null | Effect[];
/** Parent effect or derived */
parent: Effect | Derived | null;
/** Parent effect */
parent: Effect | null;
}
export interface EffectNodes {

@ -74,6 +74,17 @@ export function console_log_state(method) {
}
}
/**
* Reading a derived belonging to a now-destroyed effect may result in stale values
*/
export function derived_inert() {
if (DEV) {
console.warn(`%c[svelte] derived_inert\n%cReading a derived belonging to a now-destroyed effect may result in stale values\nhttps://svelte.dev/e/derived_inert`, bold, normal);
} else {
console.warn(`https://svelte.dev/e/derived_inert`);
}
}
/**
* %handler% should be a function. Did you mean to %suggestion%?
* @param {string} handler

@ -1391,8 +1391,12 @@ describe('signals', () => {
};
});
test('derived whose original parent effect has been destroyed keeps updating', () => {
test('derived whose original parent effect has been destroyed no longer updates', () => {
return () => {
const warn = console.warn;
const warnings: string[] = [];
console.warn = (...args) => warnings.push(...args);
let count: Source<number>;
let double: Derived<number>;
const destroy = effect_root(() => {
@ -1404,17 +1408,23 @@ describe('signals', () => {
flushSync();
assert.equal($.get(double!), 0);
destroy();
flushSync();
set(count!, 1);
flushSync();
assert.equal($.get(double!), 2);
destroy();
assert.equal($.get(double!), 2);
assert.equal(warnings.length, 0); // value is unchanged, no warning yet
set(count!, 2);
flushSync();
assert.equal($.get(double!), 4);
assert.equal($.get(double!), 2);
assert.ok(warnings.some((str) => str.includes('derived_inert')));
console.warn = warn;
};
});

Loading…
Cancel
Save