-
Notifications
You must be signed in to change notification settings - Fork 344
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
Fix(computed): destroy helper vm of computed to prevent memleak (fix #270) #277
Conversation
upon host component destroy. avoids a memory leak. fix #270
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.
Tested locally and it does fix the leak.
I was concerned about the vm
being destroyed while the computed still being used but it doesn't seem to be an issue 👍
As just discussed on Dicsord, this might break stuff in specific cirtumstances. We should not merge this now and do further investigations. |
This won't work. It will break any computed refs that are meant to persist after their original component has been destroyed. Will close and investigate other possible ways. |
Reopened, see here: #270 (comment) |
It seems this pr has been waiting for a long time to be merged. |
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.
Maybe we can get this merged?
I think shared computed should be declared outside of a vm's lifecycle, so the problem blocking this might not exist. (mentioned in #270 (comment)) Any thoughts?
Yeah I think it's fine as well @antfu |
We use a Vue instancefor each computed(), as a helper to actually use Vue's
computed:
option.However we don't destroy it when the host component is destroyed. When the computed property references reactive state that's external to the host instance, the helper instance stays in memory as it's watching the external state.
Properly destroying witht the host component fixes this.
fix #270