Skip to content

Commit ee2e386

Browse files
committed
fix: fix missing effects if derived added, removed, and re-added as dependency
Add HAS_EFFECTS flag to mark derived reactions that have side effects. Update effects.js to set this flag when effects are added to derived. Modify runtime.js to re-run derived computations if effects were once present but are now missing, ensuring effects are properly recreated. This improves reactivity consistency and prevents lost side effects.
1 parent d2ba258 commit ee2e386

File tree

4 files changed

+69
-2
lines changed

4 files changed

+69
-2
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export const REACTION_IS_UPDATING = 1 << 21;
2525
export const ASYNC = 1 << 22;
2626

2727
export const ERROR_VALUE = 1 << 23;
28+
export const HAS_EFFECTS = 1 << 24;
2829

2930
export const STATE_SYMBOL = Symbol('$state');
3031
export const LEGACY_PROPS = Symbol('legacy props');

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ import {
3333
EFFECT_PRESERVED,
3434
STALE_REACTION,
3535
USER_EFFECT,
36-
ASYNC
36+
ASYNC,
37+
HAS_EFFECTS
3738
} from '#client/constants';
3839
import * as e from '../errors.js';
3940
import { DEV } from 'esm-env';
@@ -156,6 +157,8 @@ function create_effect(type, fn, sync, push = true) {
156157
) {
157158
var derived = /** @type {Derived} */ (active_reaction);
158159
(derived.effects ??= []).push(effect);
160+
// Mark the derived as having effects
161+
derived.f |= HAS_EFFECTS;
159162
}
160163
}
161164

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ import {
2020
DISCONNECTED,
2121
REACTION_IS_UPDATING,
2222
STALE_REACTION,
23-
ERROR_VALUE
23+
ERROR_VALUE,
24+
HAS_EFFECTS
2425
} from './constants.js';
2526
import { old_values } from './reactivity/sources.js';
2627
import {
@@ -671,6 +672,10 @@ export function get(signal) {
671672

672673
if (is_dirty(derived)) {
673674
update_derived(derived);
675+
} else if ((derived.f & HAS_EFFECTS) !== 0 && (derived.effects === null || derived.effects.length === 0)) {
676+
// If the derived once had effects but they're now missing (destroyed),
677+
// we need to re-execute to recreate them
678+
update_derived(derived);
674679
}
675680
}
676681

packages/svelte/tests/signals/test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1441,4 +1441,62 @@ describe('signals', () => {
14411441
assert.deepEqual(log, ['inner destroyed', 'inner destroyed']);
14421442
};
14431443
});
1444+
1445+
test('derived effects reconnect after disconnection', () => {
1446+
return () => {
1447+
const effectLog: string[] = [];
1448+
let result!: Derived<number>;
1449+
1450+
// Create derived with effects inside an effect root to give it proper context
1451+
const derivedRoot = effect_root(() => {
1452+
result = derived(() => {
1453+
user_effect(() => {
1454+
effectLog.push('effect-executed');
1455+
});
1456+
return 42;
1457+
});
1458+
});
1459+
1460+
// First render effect that accesses the derived
1461+
const firstRoot = effect_root(() => {
1462+
render_effect(() => {
1463+
$.get(result); // This creates the derived's effects but doesn't execute them yet
1464+
});
1465+
});
1466+
1467+
// DON'T flush here - we want to simulate the effect never getting a chance to execute
1468+
// before the component unmounts
1469+
1470+
// Verify no effects have executed yet (they're scheduled but not flushed)
1471+
assert.equal(effectLog.length, 0, 'Effect should not have executed before flush');
1472+
assert.equal(result.effects?.length, 1, 'Derived should have one effect created');
1473+
1474+
// Simulate disconnection by destroying the first render context
1475+
firstRoot();
1476+
flushSync();
1477+
1478+
// Verify the derived is now disconnected (effects destroyed)
1479+
assert.equal(result.effects, null, 'Effects should be destroyed after disconnection');
1480+
assert.equal(effectLog.length, 0, 'Effect should still not have executed');
1481+
1482+
// Create second render effect in completely new context
1483+
const secondRoot = effect_root(() => {
1484+
render_effect(() => {
1485+
const value = $.get(result); // Should schedule the desired effect
1486+
assert.equal(value, 42);
1487+
});
1488+
});
1489+
1490+
flushSync();
1491+
1492+
// The critical test: our fix should have detected missing effects and re-executed
1493+
// Without the fix: effectLog.length would be 0 (effect never executes)
1494+
// With the fix: effectLog.length should be 1 (effect executes on reconnection)
1495+
assert.equal(effectLog.length, 1, 'Effect should execute on reconnection');
1496+
assert.equal(result.effects?.length, 1, 'Derived should have effects again');
1497+
1498+
secondRoot();
1499+
derivedRoot();
1500+
};
1501+
});
14441502
});

0 commit comments

Comments
 (0)