Skip to content
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: (@vue/apollo-option) memory leak in wrapped ssrRender #1553

Merged

Conversation

deleteme
Copy link
Contributor

@deleteme deleteme commented May 9, 2024

Fixes #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.


❓ I don't know why a cleanup function like destroy is called inside of a render function. Usually I'd expect to see that happen from a conventional lifecycle hook, like unmounted. Maybe this wrapped ssrRender should not happen at all?

@deleteme deleteme force-pushed the fix-memory-leak-in-vue-apollo-option-beforeCreate branch from 5088c7e to 1e54b67 Compare May 9, 2024 01:10
@deleteme deleteme changed the title Fix(@vue/apollo-option) memory leak in wrapped ssrRender fix: (@vue/apollo-option) memory leak in wrapped ssrRender May 9, 2024
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.
@Akryum Akryum force-pushed the fix-memory-leak-in-vue-apollo-option-beforeCreate branch from 1e54b67 to bec7e51 Compare August 14, 2024 14:12
@Akryum Akryum merged commit fceb40c into vuejs:v4 Aug 14, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@vue/apollo-option - memory leak on SSR render
2 participants