-
-
Notifications
You must be signed in to change notification settings - Fork 522
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
@vue/apollo-option - memory leak on SSR render #1550
Comments
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.
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.
Hi @deleteme, thanks a lot for the suggestion! 🙏 I'll try to test this asap on a test environment, however like said in my original message, we have only been able to reproduce this on production for the moment |
Hi again, I just tried with your suggestion, however I've got an error during the build on dev mode: ERROR Pre-transform error: Failed to resolve entry for package "@vue/apollo-option". The package may have incorrect main/module/exports specified in its package.json. which lead to following Nuxt error: I tried the suggestion using a yarn override in my "resolutions": {
"@vue/apollo-option": "git+https://github.com/deleteme/vuejs-apollo.git#fix-memory-leak-in-vue-apollo-option-beforeCreate"
} the relaunching the dev mode (after reinstalling my deps): I've also tried to manually build the deps (directly in my node_modules) by following the contribution guide but it leads to the same error. am I missing something here? |
@amenella Sorry, I am not familiar with the build process in this repository. You can use |
hi again @deleteme , we've just tested the fix (not in production though) and the memory usage seems stable with the fix! 🙌 plus, looking at the code, changes seem good to me, however, I'm not maintainer in this project so this is not my word to say thanks again for your time 🙏 |
Hi vuejs team! any news on integrating this? @Akryum perhaps? 🙏 |
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.
Description
Hi,
We've encountered some memory leaks using @vue/apollo-option (through the related Nuxt 3 plugin : https://github.com/nuxt-modules/apollo).
After some debug we found that the context of following method is constantly increasing and seems never released:
My best call would be to first destroy the context then re-render, but this is only a feeling and I did not tried (on those lines).
This behaviour has only been seen on production (maybe due production build + traffic).
Here are some screenshot of Node.js debug mode on production, on which we can see that the memory allocation is constantly increasing:
The line referred in previous screenshot ☝️ led us to those particular lines:
Reproduction
Unfortunately I have no easy example to reproduce this issue (we are not even able to reproduce it with the same stack on a non-production environment...)
Expected behavior
No memory leak
Versions
vue - 3.3.11
@nuxtjs/apollo - 5.0.0-alpha.11
@apollo/client: 3.8.8
@vue/apollo-option: 4.0.0-beta.12
@vue/apollo-composable - 4.0.0-beta.12
Additional context
This issue seems related to some old ones (which should have fixed it):
We also have a specific usage of apollo client within the Nuxt app using a specific mixin to allow update of endpoint at runtime using env var, which is highly inspired from nuxt-modules/apollo#442 (comment)
Any help or hint would be appreciated, thanks a lot
The text was updated successfully, but these errors were encountered: