Skip to content

no-top-level-browser-globals rule #1203

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.

Sign up for GitHub

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

Jump to bottom
Open
benmccann opened this issue Apr 22, 2025 · 4 comments · May be fixed by #1210
Open

no-top-level-browser-globals rule #1203

benmccann opened this issue Apr 22, 2025 · 4 comments · May be fixed by #1210
Labels
enhancement New feature or request new rule

Comments

@benmccann
Copy link
Member

benmccann commented Apr 22, 2025 •
edited
Loading

Motivation

You can add a browser global to a .svelte file that will cause it to error during SSR and often nothing will catch it right now. Sometimes the .svelte file never gets loaded on the server during development making this easy to miss until it is shipped to production. I.e. if you navigate to the page via client-side routing you won't see the error. You have to refresh the page to see it

Description

browser globals should not be allowed directly inside a script block. There are a lot of other cases you can use them that are valid and so we can't catch all invalid cases, but hopefully we could catch the most obviously wrong ones

Examples

<!-- ✓ GOOD -->
<script>
	import { onMount } from 'svelte';

	onMount(() => {
		const a = window.localStorage.getItem('myCat');
		console.log(a);
	});
</script>

<h1>Welcome to SvelteKit</h1>
<p>Visit <a href="https://svelte.dev/docs/kit">svelte.dev/docs/kit</a> to read the documentation</p>

<!-- ✓ GOOD -->
<script>
	import { BROWSER } from 'esm-env';

	if (BROWSER) {
		const a = window.localStorage.getItem('myCat');
		console.log(a);
	};
</script>

<!-- ✓ GOOD -->
<script>
	import { browser } from '$app/environment';

	if (browser) {
		const a = window.localStorage.getItem('myCat');
		console.log(a);
	};
</script>

<!-- ✓ GOOD -->
<script>
	$effect(() => {
		const a = window.localStorage.getItem('myCat');
		console.log(a);
	});
</script>

<h1>Welcome to SvelteKit</h1>
<p>Visit <a href="https://svelte.dev/docs/kit">svelte.dev/docs/kit</a> to read the documentation</p>

<!-- ✗ BAD -->
<script>
	const a = window.localStorage.getItem('myCat');
	console.log(a);
</script>

<h1>Welcome to SvelteKit</h1>
<p>Visit <a href="https://svelte.dev/docs/kit">svelte.dev/docs/kit</a> to read the documentation</p>

Additional comments

Users may want to disable this rule if building a SPA. I think it's generally good practice even if you're building a SPA to leave this rule enabled in case you later want to enable SSR, but many users would feel that to be unnecessary overhead
@benmccann benmccann added enhancement New feature or request new rule labels Apr 22, 2025
@benmccann benmccann changed the title no-browser-globals rule no-browser-globals-at-top-level rule Apr 22, 2025
@marekdedic
Copy link
Contributor

Great idea, how about no-top-level-browser-globals to make it more succinct?

@benmccann benmccann changed the title no-browser-globals-at-top-level rule no-top-level-browser-globals rule Apr 23, 2025
@ota-meshi ota-meshi linked a pull request May 2, 2025 that will close this issue
Draft
@JounQin
Copy link
Collaborator

JounQin commented May 2, 2025 •
edited
Loading

Question: would node globals work on SSR/client?

@ota-meshi
Copy link
Member

Hi @benmccann.
I have a question: Do you think the following code is safe?

if (import.meta.env.SSR) {
  // ... server only logic
} else {
  localStorage.getItem('myCat');
}

https://vite.dev/guide/ssr

@ota-meshi
Copy link
Member

Question: would node globals work on SSR/client?

I think the node-specific global variables are:

  • 'Buffer'
  • 'clearImmediate'
  • 'global'
  • 'process'
  • 'setImmediate'

I think it's not a problem since they are rarely used or the bundler injects them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request new rule
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: add svelte/no-top-level-browser-globals rule sveltejs/eslint-plugin-svelte
4 participants
@benmccann @marekdedic @JounQin @ota-meshi