Skip to content

Commit 2e9dab6

Browse files
authored
fix: more robust re-subscribe detection for fromStore (#13995)
Only count down after timeout, else we would reach 0 before our own render effect reruns, but reach 1 again when the tick callback of the prior teardown runs. That would mean we re-subcribe unnecessarily and create a memory leak because the old subscription is never cleaned up.
1 parent cbc2ca3 commit 2e9dab6

File tree

3 files changed

+37
-2
lines changed

3 files changed

+37
-2
lines changed

.changeset/eighty-pianos-sip.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: more robust re-subscribe detection for `fromStore`

packages/svelte/src/store/index-client.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,12 @@ export function fromStore(store) {
130130
subscribers += 1;
131131

132132
return () => {
133-
subscribers -= 1;
134-
135133
tick().then(() => {
134+
// Only count down after timeout, else we would reach 0 before our own render effect reruns,
135+
// but reach 1 again when the tick callback of the prior teardown runs. That would mean we
136+
// re-subcribe unnecessarily and create a memory leak because the old subscription is never cleaned up.
137+
subscribers -= 1;
138+
136139
if (subscribers === 0) {
137140
unsubscribe();
138141
}

packages/svelte/tests/store/test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,33 @@ describe('fromStore', () => {
666666
teardown();
667667
});
668668

669+
it('creates state from a writable store that updates after timeout', async () => {
670+
const wait = () => new Promise((resolve) => setTimeout(resolve, 100));
671+
const store = {
672+
subscribe: (cb: any) => {
673+
// new object each time to force updates of underlying signals,
674+
// to test the whole thing doesn't rerun more often than it should
675+
setTimeout(() => cb({}), 0);
676+
return () => {};
677+
}
678+
};
679+
680+
const count = fromStore(store);
681+
const log: any[] = [];
682+
683+
const teardown = effect_root(() => {
684+
render_effect(() => {
685+
log.push(count.current);
686+
});
687+
});
688+
689+
await wait();
690+
691+
assert.deepEqual(log, [undefined, {}]);
692+
693+
teardown();
694+
});
695+
669696
it('creates state from a readable store', () => {
670697
const store = readable(0);
671698

0 commit comments

Comments
 (0)