fix: (@vue/apollo-option) memory leak in wrapped ssrRender #1553
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes vuejs#1550 Two leaks were fixed: 1) Prevents repeatedly wrapping `ssrRender` by checking if it's already been wrapped. Added a `__IS_VUE_APOLLO_WRAPPED` boolean to track this. I verified that this was actually happening by throwing an error if it was already wrapped, and I observed the error. 2) `this.$options.ssrRender` doesn't always exist, but this.$apollo does. When the new wrapped `ssrRender` was called, it would throw, which prevented the `destroy.call(this)` line from running. The fix here was to not create a wrapped `ssrRender` if there isn't an original one.
deleteme
force-pushed
the
fix-memory-leak-in-vue-apollo-option-beforeCreate
branch
from
5088c7e
to
1e54b67
Compare
deleteme
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1550
Two leaks were fixed:
Prevents repeatedly wrapping
ssrRenderby checking if it's already been wrapped. Added a__IS_VUE_APOLLO_WRAPPEDboolean to track this. I verified that this was actually happening by throwing an error if it was already wrapped, and I observed the error.this.$options.ssrRenderdoesn't always exist, but this.$apollo does. When the new wrappedssrRenderwas called, it would throw, which prevented thedestroy.call(this)line from running. The fix here was to not create a wrappedssrRenderif there isn't an original one.❓ I don't know why a cleanup function like
destroyis called inside of a render function. Usually I'd expect to see that happen from a conventional lifecycle hook, likeunmounted. Maybe this wrappedssrRendershould not happen at all?