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

Potential ViewTransitions memory leak with Svelte components #8435

Closed
1 task
jerzakm opened this issue Sep 6, 2023 · 3 comments · Fixed by #8448
Closed
1 task

Potential ViewTransitions memory leak with Svelte components #8435

jerzakm opened this issue Sep 6, 2023 · 3 comments · Fixed by #8448
Assignees
Labels
feat: view transitions Related to the View Transitions feature (scope)

Comments

@jerzakm
Copy link
Contributor

jerzakm commented Sep 6, 2023

Astro Info

Astro                    v3.0.9
Node                     v19.7.0
System                   Windows (x64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             auto-import
                         @astrojs/tailwind
                         @astrojs/svelte
                         @astrojs/mdx
                         @astrojs/preact

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

We're having issues with a potential memory leak - if you navigate in the docs several times (especially on an intensive route like /extras examples) - the amount of DOM nodes and JS listeners jumps up drastically.

It seems that Svelte components and listeners aren't destroyed on navigation. I thought it'd get fixed by this PR but I doesn't help our case
#8264

Let me know if it's maybe something that we're doing wrong or if I can help in any way to pinpoint it better.

image

I created this PR to anchor the docs in time as a reproduction. The fastest way to cause the leak is to click through the sidebar of Extras

What's the expected result?

Dom/listener numbers closer to this

image

Link to Minimal Reproducible Example

threlte/threlte#601

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Sep 6, 2023
@jerzakm jerzakm changed the title Potential ViewTransitions leak with Svelte components Potential ViewTransitions memory leak with Svelte components Sep 6, 2023
@matthewp
Copy link
Contributor

matthewp commented Sep 6, 2023

Thanks for the heads up. Will create a demo project and see what happens.

@matthewp matthewp self-assigned this Sep 6, 2023
@matthewp matthewp added feat: view transitions Related to the View Transitions feature (scope) and removed needs triage Issue needs to be triaged labels Sep 6, 2023
@roryhen
Copy link

roryhen commented Sep 7, 2023

Hey guys, I just happened to be reading the code for the Svelte integration out of interest for my own use-case, and I noticed that there was a missing reference to element in the file, so the unmount event never gets fired. Which led me to find this issue. Hope the info helps:

element.addEventListener('astro:unmount', () => component.$destroy(), { once: true });

@matthewp
Copy link
Contributor

matthewp commented Sep 7, 2023

Good catch, @natemoo-re should this be the target variable instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: view transitions Related to the View Transitions feature (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants