-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: computed signal memory leak when reusing effect subscriber in loop #8254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: df3ed1a The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
5c41ee2 to
e71937f
Compare
gioboa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
| if (ctx) { | ||
| const effectSubscriber = getSubscriber(this, EffectProperty.VNODE); | ||
| // clear existing back refs | ||
| effectSubscriber.backRef?.clear(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I think the backrefs can be cleared as soon as the signal is marked dirty, because nothing else can make it dirtier
- But most importantly, when clearing backrefs, each signal must also have this subscriber removed
e71937f to
df3ed1a
Compare
wmertens
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.

Clear back refs for effect subscriber when reusing in computed signal computation