Getting "document is not defined" when used inside onDestroy

[SiegfriedEhret]

Describe the bug

Hello,
I have a component to handle a few keyboard events, using onMount and onDestroy to registrer / remove the handler.
I noticed a few days ago (I think I was running @sveltejs/kit 180 or something) that I was getting some document is not defined. I just upgraded to 193 and still see the issue.

Workaround (development mode only):

• Comment import Keyboard from '$lib/Keyboard.svelte'; and <Keyboard/> in https://github.com/SiegfriedEhret/svelte-document/blob/main/src/routes/__layout.svelte
• Start the server
• Uncomment these 2 lines, everything works (hit the escape key to get an alert)

Thanks.

Reproduction

https://github.com/SiegfriedEhret/svelte-document

Logs

In the terminal:

❯ npm run dev

> [email protected] dev
> svelte-kit dev


  SvelteKit v1.0.0-next.193

  local:   http://localhost:3000
  network: not exposed

  Use --host to expose server to other devices on this network


document is not defined
ReferenceError: document is not defined
    at eval (/src/lib/Keyboard.svelte:30:3)
    at run (/Users/sehret/dev/perso/svelte-document/node_modules/svelte/internal/index.js:22:12)
    at Array.forEach (<anonymous>)
    at run_all (/Users/sehret/dev/perso/svelte-document/node_modules/svelte/internal/index.js:28:9)
    at Object.render (/Users/sehret/dev/perso/svelte-document/node_modules/svelte/internal/index.js:1693:13)
    at render_response (file:///Users/sehret/dev/perso/svelte-document/node_modules/@sveltejs/kit/dist/ssr.js:561:28)
    at async respond_with_error (file:///Users/sehret/dev/perso/svelte-document/node_modules/@sveltejs/kit/dist/ssr.js:1174:10)
    at async resolve (file:///Users/sehret/dev/perso/svelte-document/node_modules/@sveltejs/kit/dist/ssr.js:1768:12)
    at async Object.handle (/Users/sehret/dev/perso/svelte-document/src/hooks.ts:14:24)
    at async respond (file:///Users/sehret/dev/perso/svelte-document/node_modules/@sveltejs/kit/dist/ssr.js:1722:10)

The same stacktrace is visible in the browser.

System Info

❯ npx envinfo --system --binaries --browsers --npmPackages "{svelte,@sveltejs/*,vite}"
Need to install the following packages:
  envinfo
Ok to proceed? (y)

  System:
    OS: macOS 12.0.1
    CPU: (8) x64 Apple M1
    Memory: 22.14 MB / 16.00 GB
    Shell: 3.2.2 - /usr/local/bin/fish
  Binaries:
    Node: 16.13.0 - ~/.local/share/nvm/v16.13.0/bin/node
    npm: 8.1.0 - ~/.local/share/nvm/v16.13.0/bin/npm
  Browsers:
    Chrome: 95.0.4638.69
    Firefox Developer Edition: 95.0
    Safari: 15.1
  npmPackages:
    @sveltejs/kit: next => 1.0.0-next.193
    svelte: ^3.34.0 => 3.44.1

Severity

blocking all usage of SvelteKit

Additional Information

The workaround is possible only in development, but I can't publish my app for the users because of this issue.

[dummdidumm]

onMount runs only in the browser, but onDestroy also runs on the server side (during SSR), and in your code there's a check missing if this is a browser environment. This should solve it:

<script>
  import { browser } from '$app/environment'; // (was '$app/env' in a pre 1.0 SvelteKit version)
  // ...
  onDestroy(() => {
    if (browser) {
      document.removeEventListener('keydown', onKeydown);
    }
  });

https://kit.svelte.dev/docs#modules-$app-env

Edit: you can also use onMount for this, see the subanswer below.

[SiegfriedEhret]

Oh I missed this part.
Thank you so much! Sorry for the noise!

[CaptainCodeman]

You can also return a function from onMount to remove the event listeners (rather than using onDestroy + the browser check)

onMount(() => {
  document.addEventListener('keydown', onKeydown);

  return () => document.removeEventListener('keydown', onKeydown);
});

[SiegfriedEhret]

Super cool, thanks!

[Explosion-Scratch]

You can also return a function from onMount to remove the event listeners (rather than using onDestroy + the browser check)

onMount(() => {
  document.addEventListener('keydown', onKeydown);

  return () => document.removeEventListener('keydown', onKeydown);
});

Wait what?!? Thanks!

[bitdom8]

onMount runs only in the browser, but onDestroy also runs on the server side (during SSR), and in your code there's a check missing if this is a browser environment. This should solve it:

<script>
  import { browser } from '$app/env';
  // ...
  onDestroy(() => {
    if (browser) {
      document.removeEventListener('keydown', onKeydown);
    }
  });

https://kit.svelte.dev/docs#modules-$app-env

Edit: you can also use onMount for this, see the subanswer below.

should be import { browser } from '$app/environment';