-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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(watch): memory leak #590
Conversation
Did you run into a real memory leak with this? I think this is actually fine because watch effects are only recorded if |
Oh nvm, it's possible with |
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.
It's ok for computed
because there is no API to create instance-bound computed
outside of setup()
.
const effects = currentInstance.effects || (currentInstance.effects = []) | ||
effects.push(effect) | ||
return () => { | ||
const index = effects.findIndex(_effect => _effect === effect) |
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.
const index = effects.findIndex(_effect => _effect === effect) | |
const index = effects.indexOf(effect) |
// record effects created during a component's setup() so that they can be | ||
// stopped when the component unmounts | ||
export function recordEffect(effect: ReactiveEffect) { | ||
export function recordEffect(effect: ReactiveEffect): RemoveRecord | void { |
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.
export function recordEffect(effect: ReactiveEffect): RemoveRecord | void { | |
export function recordEffect(effect: ReactiveEffect): (() => void) | void { |
@@ -33,11 +33,20 @@ import { | |||
|
|||
import { currentInstance } from './component' | |||
|
|||
type RemoveRecord = () => void |
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.
type RemoveRecord = () => void |
No, I didn't run into a real memory leak, I just thought it have risk, actually I didn't think about But |
Forget it, I break |
I think
computed
have same problem, but unless expose a stop api, I don't think there is an easy way to fix it.