Skip to content

Commit bc69211

Browse files
committed
fix: ensure eager effects don't break reactions chain
Execution of eager effects didn't set `is_updating_effect`, which meant the logic in `get` would wrongfully prevent dependencies being added to `reactions` of sources/deriveds. Fixes #17133
1 parent e238e66 commit bc69211

File tree

4 files changed

+71
-9
lines changed

4 files changed

+71
-9
lines changed

.changeset/silent-teeth-invent.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: ensure eager effects don't break reactions chain

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

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ import {
1414
is_dirty,
1515
untracking,
1616
is_destroying_effect,
17-
push_reaction_value
17+
push_reaction_value,
18+
set_is_updating_effect,
19+
is_updating_effect
1820
} from '../runtime.js';
1921
import { equals, safe_equals } from './equality.js';
2022
import {
@@ -246,19 +248,25 @@ export function internal_set(source, value) {
246248

247249
export function flush_eager_effects() {
248250
eager_effects_deferred = false;
251+
var prev_is_updating_effect = is_updating_effect;
252+
set_is_updating_effect(true);
249253

250254
const inspects = Array.from(eager_effects);
251255

252-
for (const effect of inspects) {
253-
// Mark clean inspect-effects as maybe dirty and then check their dirtiness
254-
// instead of just updating the effects - this way we avoid overfiring.
255-
if ((effect.f & CLEAN) !== 0) {
256-
set_signal_status(effect, MAYBE_DIRTY);
257-
}
256+
try {
257+
for (const effect of inspects) {
258+
// Mark clean inspect-effects as maybe dirty and then check their dirtiness
259+
// instead of just updating the effects - this way we avoid overfiring.
260+
if ((effect.f & CLEAN) !== 0) {
261+
set_signal_status(effect, MAYBE_DIRTY);
262+
}
258263

259-
if (is_dirty(effect)) {
260-
update_effect(effect);
264+
if (is_dirty(effect)) {
265+
update_effect(effect);
266+
}
261267
}
268+
} finally {
269+
set_is_updating_effect(prev_is_updating_effect);
262270
}
263271

264272
eager_effects.clear();
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { tick } from 'svelte';
2+
import { test } from '../../test';
3+
import { normalise_inspect_logs } from '../../../helpers';
4+
5+
export default test({
6+
compileOptions: {
7+
dev: true
8+
},
9+
10+
async test({ assert, target, logs }) {
11+
const [b] = target.querySelectorAll('button');
12+
13+
b.click();
14+
await tick();
15+
assert.htmlEqual(target.innerHTML, `<button>first unseen: 1</button>`);
16+
17+
b.click();
18+
await tick();
19+
assert.htmlEqual(target.innerHTML, `<button>first unseen: 2</button>`);
20+
21+
b.click();
22+
await tick();
23+
assert.htmlEqual(target.innerHTML, `<button>first unseen:</button>`);
24+
25+
assert.deepEqual(normalise_inspect_logs(logs), [
26+
[0, 1, 2],
27+
[1, 2],
28+
'at SvelteSet.add',
29+
[2],
30+
'at SvelteSet.add',
31+
[],
32+
'at SvelteSet.add'
33+
]);
34+
}
35+
});
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<script>
2+
import {SvelteSet} from "svelte/reactivity";
3+
const ids = [0,1,2];
4+
const seenIds = new SvelteSet();
5+
6+
const unseenIds = $derived(ids.filter((id) => !seenIds.has(id)));
7+
8+
const currentId = $derived(unseenIds.at(0));
9+
$inspect(unseenIds)
10+
</script>
11+
12+
<button onclick={() => seenIds.add(currentId)}>
13+
first unseen: {currentId}
14+
</button>

0 commit comments

Comments
 (0)