Skip to content

Commit d2e7932

Browse files
authored
fix: don't depend on deriveds created inside the current reaction (#15564)
* WIP * WIP * add test * update test * changeset * oops * lint
1 parent 1d10a65 commit d2e7932

File tree

10 files changed

+114
-42
lines changed

10 files changed

+114
-42
lines changed

.changeset/young-poets-wait.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: don't depend on deriveds created inside the current reaction

packages/svelte/src/internal/client/index.js

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ export {
101101
text,
102102
props_id
103103
} from './dom/template.js';
104-
export { derived, derived_safe_equal } from './reactivity/deriveds.js';
104+
export { user_derived as derived, derived_safe_equal } from './reactivity/deriveds.js';
105105
export {
106106
effect_tracking,
107107
effect_root,
@@ -113,14 +113,7 @@ export {
113113
user_effect,
114114
user_pre_effect
115115
} from './reactivity/effects.js';
116-
export {
117-
mutable_source,
118-
mutate,
119-
set,
120-
source as state,
121-
update,
122-
update_pre
123-
} from './reactivity/sources.js';
116+
export { mutable_source, mutate, set, state, update, update_pre } from './reactivity/sources.js';
124117
export {
125118
prop,
126119
rest_props,

packages/svelte/src/internal/client/proxy.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
object_prototype
1111
} from '../shared/utils.js';
1212
import { check_ownership, widen_ownership } from './dev/ownership.js';
13-
import { source, set } from './reactivity/sources.js';
13+
import { state as source, set } from './reactivity/sources.js';
1414
import { STATE_SYMBOL, STATE_SYMBOL_METADATA } from './constants.js';
1515
import { UNINITIALIZED } from '../../constants.js';
1616
import * as e from './errors.js';

packages/svelte/src/internal/client/reactivity/deriveds.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ import {
88
skip_reaction,
99
update_reaction,
1010
increment_write_version,
11-
set_active_effect
11+
set_active_effect,
12+
push_reaction_value
1213
} from '../runtime.js';
1314
import { equals, safe_equals } from './equality.js';
1415
import * as e from '../errors.js';
@@ -61,6 +62,19 @@ export function derived(fn) {
6162
return signal;
6263
}
6364

65+
/**
66+
* @template V
67+
* @param {() => V} fn
68+
* @returns {Derived<V>}
69+
*/
70+
export function user_derived(fn) {
71+
const d = derived(fn);
72+
73+
push_reaction_value(d);
74+
75+
return d;
76+
}
77+
6478
/**
6579
* @template V
6680
* @param {() => V} fn

packages/svelte/src/internal/client/reactivity/sources.js

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ import {
1515
set_reaction_sources,
1616
check_dirtiness,
1717
untracking,
18-
is_destroying_effect
18+
is_destroying_effect,
19+
push_reaction_value
1920
} from '../runtime.js';
2021
import { equals, safe_equals } from './equality.js';
2122
import {
@@ -64,14 +65,6 @@ export function source(v, stack) {
6465
wv: 0
6566
};
6667

67-
if (active_reaction !== null && active_reaction.f & EFFECT_IS_UPDATING) {
68-
if (reaction_sources === null) {
69-
set_reaction_sources([signal]);
70-
} else {
71-
reaction_sources.push(signal);
72-
}
73-
}
74-
7568
if (DEV && tracing_mode_flag) {
7669
signal.created = stack ?? get_stack('CreatedAt');
7770
signal.debug = null;
@@ -80,6 +73,19 @@ export function source(v, stack) {
8073
return signal;
8174
}
8275

76+
/**
77+
* @template V
78+
* @param {V} v
79+
* @param {Error | null} [stack]
80+
*/
81+
export function state(v, stack) {
82+
const s = source(v, stack);
83+
84+
push_reaction_value(s);
85+
86+
return s;
87+
}
88+
8389
/**
8490
* @template V
8591
* @param {V} initial_value

packages/svelte/src/internal/client/runtime.js

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,17 @@ export function set_reaction_sources(sources) {
101101
reaction_sources = sources;
102102
}
103103

104+
/** @param {Value} value */
105+
export function push_reaction_value(value) {
106+
if (active_reaction !== null && active_reaction.f & EFFECT_IS_UPDATING) {
107+
if (reaction_sources === null) {
108+
set_reaction_sources([value]);
109+
} else {
110+
reaction_sources.push(value);
111+
}
112+
}
113+
}
114+
104115
/**
105116
* The dependencies of the reaction that is currently being executed. In many cases,
106117
* the dependencies are unchanged between runs, and so this will be `null` unless
@@ -875,21 +886,23 @@ export function get(signal) {
875886

876887
// Register the dependency on the current reaction signal.
877888
if (active_reaction !== null && !untracking) {
878-
var deps = active_reaction.deps;
879-
if (signal.rv < read_version) {
880-
signal.rv = read_version;
881-
// If the signal is accessing the same dependencies in the same
882-
// order as it did last time, increment `skipped_deps`
883-
// rather than updating `new_deps`, which creates GC cost
884-
if (new_deps === null && deps !== null && deps[skipped_deps] === signal) {
885-
skipped_deps++;
886-
} else if (new_deps === null) {
887-
new_deps = [signal];
888-
} else if (!skip_reaction || !new_deps.includes(signal)) {
889-
// Normally we can push duplicated dependencies to `new_deps`, but if we're inside
890-
// an unowned derived because skip_reaction is true, then we need to ensure that
891-
// we don't have duplicates
892-
new_deps.push(signal);
889+
if (!reaction_sources?.includes(signal)) {
890+
var deps = active_reaction.deps;
891+
if (signal.rv < read_version) {
892+
signal.rv = read_version;
893+
// If the signal is accessing the same dependencies in the same
894+
// order as it did last time, increment `skipped_deps`
895+
// rather than updating `new_deps`, which creates GC cost
896+
if (new_deps === null && deps !== null && deps[skipped_deps] === signal) {
897+
skipped_deps++;
898+
} else if (new_deps === null) {
899+
new_deps = [signal];
900+
} else if (!skip_reaction || !new_deps.includes(signal)) {
901+
// Normally we can push duplicated dependencies to `new_deps`, but if we're inside
902+
// an unowned derived because skip_reaction is true, then we need to ensure that
903+
// we don't have duplicates
904+
new_deps.push(signal);
905+
}
893906
}
894907
}
895908
} else if (

packages/svelte/tests/runtime-runes/samples/effect-cleanup/_config.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,6 @@ export default test({
1010
flushSync(() => {
1111
b1.click();
1212
});
13-
assert.deepEqual(logs, ['init 0', 'cleanup 2', null, 'init 2', 'cleanup 4', null, 'init 4']);
13+
assert.deepEqual(logs, ['init 0']);
1414
}
1515
});
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { flushSync } from 'svelte';
2+
import { test } from '../../test';
3+
4+
export default test({
5+
test({ assert, target, logs }) {
6+
const button = target.querySelector('button');
7+
8+
flushSync(() => button?.click());
9+
10+
assert.htmlEqual(
11+
target.innerHTML,
12+
`
13+
<button>increment</button>
14+
<p>1/2</p
15+
`
16+
);
17+
18+
assert.deepEqual(logs, [0, 0]);
19+
}
20+
});
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<script>
2+
class Foo {
3+
value = $state(0);
4+
double = $derived(this.value * 2);
5+
6+
constructor() {
7+
console.log(this.value, this.double);
8+
}
9+
10+
increment() {
11+
this.value++;
12+
}
13+
}
14+
15+
let foo = $state();
16+
17+
$effect(() => {
18+
foo = new Foo();
19+
});
20+
</script>
21+
22+
<button onclick={() => foo.increment()}>increment</button>
23+
24+
{#if foo}
25+
<p>{foo.value}/{foo.double}</p>
26+
{/if}

packages/svelte/tests/signals/test.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,7 @@ import {
88
render_effect,
99
user_effect
1010
} from '../../src/internal/client/reactivity/effects';
11-
import {
12-
source as state,
13-
set,
14-
update,
15-
update_pre
16-
} from '../../src/internal/client/reactivity/sources';
11+
import { state, set, update, update_pre } from '../../src/internal/client/reactivity/sources';
1712
import type { Derived, Effect, Value } from '../../src/internal/client/types';
1813
import { proxy } from '../../src/internal/client/proxy';
1914
import { derived } from '../../src/internal/client/reactivity/deriveds';

0 commit comments

Comments
 (0)