sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
80.24k stars 4.27k forks source link

DOM lib types are leaking into global scope #10153

Open vonBrax opened 10 months ago

vonBrax commented 10 months ago

Describe the bug

This bug is about global type scope pollution, but I wasn't sure if I should open the issue here, or in the svelte repo, or in the svelte-check repo, or in the svelte language-tools repo (svelte-check? svelte-vscode?), so I apologize if this is not the correct place.

In my project, I have some files that will only be executed in a backend environment (Node JS). I can let SvelteKit know about that by either using the lib/server folder or a .svelte.ts file. I then setup some typescript projects for the different environments (using different default libraries) to also let Typescript know about it and check the code accordingly, but I realized it wasn't working as I expected. As an example, I was using the Worker class in a server file without importing it, which should trigger an error, but it wasn't, since Worker is global in the browser and the DOM lib types were leaking in the server files. The result is an uncaught runtime error.

The problem is in this line: https://github.com/sveltejs/svelte/blob/05f99d20f9030f56b4991d0a07c3919be5f529ee/packages/svelte/svelte-html.d.ts#L1

The triple-slash reference makes the DOM types available globally if this file is included in a project.

The type error is suppressed (and it should NOT) if I check my code with svelte-check or in VSCode with the svelte vscode extension, but not when I'm running the tsc command in the command line, which leads me to believe that some of the toolings around the framework are directly or indirectly including that file.

Reproduction

I've put a small app togetherhere to try to demonstrate the issue, where I toggle the triple-slash reference in the file above on or off, and show the difference in the output of tsc and svelte-check.

Logs

No response

System Info

System:
  OS: Linux 5.15 Ubuntu 20.04.6 LTS (Focal Fossa)
  CPU: (2) x64 Intel(R) Core(TM) i5-6200U CPU @ 2.30GHz
  Memory: 453.38 MB / 3.83 GB
  Container: Yes
  Shell: 5.0.17 - /bin/bash
Binaries:
  Node: 20.10.0 - ~/.nvm/versions/node/v20.10.0/bin/node
  npm: 10.2.3 - ~/.nvm/versions/node/v20.10.0/bin/npm
npmPackages:
  @sveltejs/adapter-auto: 3.1.0 => 3.1.0
  @sveltejs/kit: 2.2.0 => 2.2.0
  @sveltejs/vite-plugin-svelte: 3.0.1 => 3.0.1
  svelte: 4.2.8 => 4.2.8
  vite: 5.0.11 => 5.0.11

Severity

annoyance

Additional Information

Related to:

dummdidumm commented 10 months ago

@jasonlyu123 I vaguely remember a similar issue from a while ago, so you remember what it was? Maybe we can reuse the solution.

jasonlyu123 commented 10 months ago

Probably is https://github.com/sveltejs/language-tools/issues/1733. The workaround is outdated since we might load it from the Svelte core in Svelte 4.2. For svelte-check, it's hard to differentiate if we need to remove it or not load the svelte-html.d.ts file; it probably makes more sense to just use tsc to check.

dummdidumm commented 10 months ago

Mhm I don't see how we can then solve this then, since we can't import the dom lib explicitly, and mapElementTag is pretty fundamental to the types