From 42743da6274fbd191bb27a8473ea98752cf6a467 Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Sat, 10 Sep 2022 17:51:08 -0400 Subject: [PATCH 1/3] Remove root signal (fixes memory leak) --- packages/core/src/index.ts | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index c01da22d6..c12b6d969 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,7 +1,5 @@ -let ROOT: Signal; - /** This tracks subscriptions of signals read inside a computed */ -let currentSignal: Signal; +let currentSignal: Signal | undefined; let commitError: Error | null = null; const pending = new Set(); @@ -45,24 +43,21 @@ export class Signal { } peek() { - if (currentSignal._canActivate && this._deps.size === 0) { + if (currentSignal && currentSignal._canActivate && this._deps.size === 0) { activate(this); } return this._value; } get value() { + if (!currentSignal) { + return this._value; + } // If we read a signal outside of a computed we have no way // to unsubscribe from that. So we assume that the user wants // to get the value immediately like for testing. if (currentSignal._canActivate && this._deps.size === 0) { activate(this); - - // The ROOT signal cannot track dependencies as it's never - // subscribed to - if (currentSignal === ROOT) { - return this._value; - } } // subscribe the current computed to this signal: @@ -85,7 +80,7 @@ export class Signal { set value(value) { if (this._readonly) { - throw new Error("Computed signals are readonly"); + throw Error("Computed signals are readonly"); } if (this._value !== value) { @@ -180,7 +175,7 @@ function sweep(subs: Set>) { if (--signal._pending === 0) { if (signal._isComputing) { - throw new Error("Cycle detected"); + throw Error("Cycle detected"); } signal._requiresUpdate = false; @@ -248,9 +243,6 @@ function activate(signal: Signal) { } } -ROOT = currentSignal = new Signal(undefined); -ROOT._canActivate = true; - export function signal(value: T): Signal { return new Signal(value); } From bf6af3b5bcddde6b49c324a8e180a32f075a69dd Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Sat, 10 Sep 2022 17:54:22 -0400 Subject: [PATCH 2/3] Create two-mails-sip.md --- .changeset/two-mails-sip.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/two-mails-sip.md diff --git a/.changeset/two-mails-sip.md b/.changeset/two-mails-sip.md new file mode 100644 index 000000000..1f177f941 --- /dev/null +++ b/.changeset/two-mails-sip.md @@ -0,0 +1,5 @@ +--- +"@preact/signals-core": patch +--- + +- Fix a memory leak when computed signals and effects are removed From ad5d6f430b46b95f3ef61cc7f7b7e7e925ffeeaf Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Sat, 10 Sep 2022 18:01:35 -0400 Subject: [PATCH 3/3] fix logic --- packages/core/src/index.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index c12b6d969..3b3e8316c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -43,21 +43,24 @@ export class Signal { } peek() { - if (currentSignal && currentSignal._canActivate && this._deps.size === 0) { + const shouldActivate = !currentSignal || currentSignal._canActivate; + if (shouldActivate && this._deps.size === 0) { activate(this); } return this._value; } get value() { - if (!currentSignal) { - return this._value; + const shouldActivate = !currentSignal || currentSignal._canActivate; + if (shouldActivate && this._deps.size === 0) { + activate(this); } + // If we read a signal outside of a computed we have no way // to unsubscribe from that. So we assume that the user wants // to get the value immediately like for testing. - if (currentSignal._canActivate && this._deps.size === 0) { - activate(this); + if (!currentSignal) { + return this._value; } // subscribe the current computed to this signal: