-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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(gatsby-image): fix memory leak and use more appropriate data structures for cache and listeners #10278
fix(gatsby-image): fix memory leak and use more appropriate data structures for cache and listeners #10278
Conversation
…ctures for cache and listeners
Hmm can someone re-run the circleci e2e test? Can't reproduce it locally 🤔 |
I did rerun, but it still happened - didn't code yet, but maybe it's not false negative if it happened 2 times in a row? |
Yeah I had forgotten to change the |
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.
This one looks good to me. Thanks for fixing this one. Memory leaks are hard to catch 👍
I added a few nitpicks 🙈
I might need to give this another proper look after I merged master into this again, there were some issues that I thought the tests would've picked up 🤔 |
Could you elaborate which tests? And mabye add them ^^ |
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 removed the createRef part as it's a breaking change, we will revisit this when we want to publish gatsby v3
Set
andMap
instead of objects and arrays.listeners
when element is no longer observed (was probably a memory leak before, since we pushed items to the array with out ever removing them?).componentDidMount
instead of in the refHandler.componentWillUnmount
.