Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
justin-schroeder
left a comment
justin-schroeder
left a comment
There was a problem hiding this comment.
Hey!
Thanks for looking into this. Currently this pr assumes that there is only 1 auto animate instance but the module is sharing the same resize observer and mutation observer for all auto animate instances on the page, so we need to do a bit more scoping. Heading in the right direction though!
| el, | ||
| setInterval(() => lowPriority(updatePos.bind(null, el)), 2000) | ||
| ) | ||
| intervalIds.push(setInterval(() => lowPriority(updatePos.bind(null, el)), 2000)); |
There was a problem hiding this comment.
These intervals are not scoped to the current element. This means if any unmount is called all the intervals for any animations on the page will be cleared. We should continue to use the intervals WeakSet.
| parents.delete(this.parent); | ||
| mutations?.disconnect(); | ||
| resize?.disconnect(); |
There was a problem hiding this comment.
We don’t want to disconnect the mutation observer or the resize observer. These are global instantiations (1 per load). Destroying them would destroy all other observers on the page. parents is the only non-weak set/map here so we should keep that one.
| intervalIds.forEach(id => clearInterval(id)); | ||
| intervalIds = []; |
There was a problem hiding this comment.
These need to be scoped to the current parent using the intervals weakset.
|
@pyongchen did you want to make the changes shown in the review? |
|
resolved in latest |
2 participants
When the animation-mounted component is destroyed, the corresponding variables such as parents, mutations, resize, and timers are not cleared, which will cause memory leaks. Therefore, add a destroy method to autoAnimate, which is called when the component is destroyed. Memory leak related information is as follows:
Memory keeps rising as page routing switches