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

onDestroy hook not called on SSR since version 3.39.0 in production mode #6676

Closed
cudr opened this issue Aug 24, 2021 · 9 comments · Fixed by #6677
Closed

onDestroy hook not called on SSR since version 3.39.0 in production mode #6676

cudr opened this issue Aug 24, 2021 · 9 comments · Fixed by #6677
Labels
bug compiler Changes relating to the compiler

Comments

@cudr
Copy link
Contributor

cudr commented Aug 24, 2021

Describe the bug

According documentation onDestroy hook must be called on server when component is destroyed. It looks like since version 3.39.0 it never gets called.

Reproduction

Clone repo: https://github.com/cudr/svelte-onDestroy-bug-repro then install dependencies and build.
Start project npm run start, go to main page and check terminal output (there are no console here).

If svelte is downgraded to 3.38.3 this will work fine and the onDestroy hook will be called.

Logs

No response

System Info

System:
    OS: Linux 5.8 Ubuntu 20.04.2 LTS (Focal Fossa)
  Binaries:
    Node: 16.6.1 - ~/.nvm/versions/node/v16.6.1/bin/node
    Yarn: 1.22.11 - ~/.nvm/versions/node/v16.6.1/bin/yarn
    npm: 7.20.3 - ~/.nvm/versions/node/v16.6.1/bin/npm
  Browsers:
    Firefox: 91.0.1
  npmPackages:
    svelte: ^3.42.2 => 3.39.0

Severity

blocking an upgrade

@cudr
Copy link
Contributor Author

cudr commented Aug 24, 2021

Maybe related #6416

@dummdidumm
Copy link
Member

Lifecycle hooks should not be called during SSR/on the server, they are meant for the browser only. What's your use case for relying on them getting called on the server? I'm honestly a bit surprised, I don't think they ever were called on the server.

@dummdidumm dummdidumm added the awaiting submitter needs a reproduction, or clarification label Aug 24, 2021
@bluwy
Copy link
Member

bluwy commented Aug 24, 2021

I had a usecase for needing onDestroy called in SSR from a blog post I wrote. TLDR, I'm using using onDestroy as a hook to do cleanup after the app is rendered in the server, and before Sapper serializes the session, because I was doing some hacky workarounds to get Apollo Client's cache to work. I think it's a breaking change that we remove that functionality now though.

@dummdidumm
Copy link
Member

Interesting ... As you see I'm not that well-versed with SSR + Hooks, but if onDestroy was called before but isn't know then that's somewhat of a breaking change, yes. Does this happen for other hooks as well?

@bluwy
Copy link
Member

bluwy commented Aug 24, 2021

Should be onDestroy only. Per the documentation it's the only hook that's called in the server-side.

@dummdidumm dummdidumm added bug compiler Changes relating to the compiler and removed awaiting submitter needs a reproduction, or clarification labels Aug 24, 2021
dummdidumm added a commit that referenced this issue Aug 24, 2021
Via #6416, lifecycle hooks became noops for SvelteKit. But onDestroy is said to be called suring SSR according to the docs.

Fixes #6676
@dummdidumm
Copy link
Member

@Rich-Harris @benmccann this seems to have slipped while designing the noops for SvelteKit. Is this something we need to revert or do we go "works as designed for SvelteKit"-route here and adjust the documentation?

@dominikg
Copy link
Member

dominikg commented Aug 24, 2021

i think this one slipped through, the original request describing the feature explicitly mentions onMount, beforeUpdate and afterUpdate #6372

#6677 looks like the proper fix to restore the documented behavior

@benmccann
Copy link
Member

I'm okay to revert it. The original motivation was to not call onMount on the server. I think it just slipped by us all and no one realized onDestroy was called on the server

dummdidumm added a commit that referenced this issue Aug 24, 2021
Via #6416, lifecycle hooks became noops for SvelteKit. But onDestroy is said to be called suring SSR according to the docs.

Fixes #6676
@Conduitry
Copy link
Member

This should be fixed in 3.42.3. Thanks for the speedy PR @dummdidumm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug compiler Changes relating to the compiler
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants