fix(store-devtools): resolve memory leak when devtools are destroyed #3965
Conversation
✅ Deploy Preview for ngrx-io canceled.Built without sensitive environment variables
|
|
The failing CIrcle is unrelated since it fails to install the Chrome driver. |
|
Thanks @arturovt. I pushed up a fix for CI to run the browser tests correctly. |
| this.extensionStartSubscription = extensionStartSubscription; | ||
| this.stateSubscription = liftedStateSubscription; |
There was a problem hiding this comment.
For reference, why not just unsubscribe from these in the ngOnDestroy instead of adding a destroy Subject?
There was a problem hiding this comment.
Well, my VSCode initially marked them as unused, so I removed them first. Then I added takeUntil since I saw it's already being used in other places too.
There was a problem hiding this comment.
I can revert properties back and unsubscribe.
There was a problem hiding this comment.
I think that approach would be better in this case because the subscription references are already there.
This commit updates the implementation of the `StoreDevtools` and adds the `ngOnDestroy` hook, which ensures the teardown of internal subscriptions. Previously, this caused an excessive memory leak because subscription callbacks created closures that captured `this`, preventing `this` from being garbage collected.
arturovt
force-pushed
the
fix/store-devtools-leak
branch
from
8488acb
to
b8d85a3
Compare
brandonroberts
changed the title
StoreDevtools
brandonroberts
changed the title
This commit updates the implementation of the
StoreDevtoolsand adds thengOnDestroyhook, which ensures the teardown of internal subscriptions. Previously, this caused an excessive memory leak because subscription callbacks created closures that capturedthis, preventingthisfrom being garbage collected.