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

feat: rune-prefer-let rule #806

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bfanger
Copy link

@bfanger bfanger commented Jun 26, 2024

Svelte 5 allows using both let and const for signals, in regular JavaScript const means:

The const declaration creates an immutable reference to a value.
The variable identifier cannot be reassigned.
You should understand const declarations as "create a variable whose identity remains constant"

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/const

But signals do change and are reassigned by Svelte's reactivity system.

This ESLint rule converts:

const { value } = $props()

into

let { value } = $props()

Copy link

changeset-bot bot commented Jun 26, 2024

🦋 Changeset detected

Latest commit: 642e4dd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
eslint-plugin-svelte Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@bfanger bfanger force-pushed the feature/signal-prefer-let branch from 9cfb0d8 to f5775b7 Compare June 26, 2024 07:32
@baseballyama
Copy link
Member

baseballyama commented Jun 26, 2024

Thank you for the PR!

This is just small point but return value of $effect.root should use const according to the docs, so I think rune-declaration-kind or something like that is better rule name and we can check $effect.root also.

	const cleanup = $effect.root(() => {
		$effect(() => {
			console.log(count);
		});

		return () => {
			console.log('effect root cleanup');
		};
	});

https://svelte-5-preview.vercel.app/docs/runes#$effect-root

@ota-meshi
Copy link
Member

Thank you for the rule suggestion and opening this PR!
However, I think this rule will conflict with the prefer-const rule in ESLint core as it is.
I think we need to consider how to avoid the conflict.
Could you please propose the rule in a new issue first?

@ota-meshi ota-meshi marked this pull request as draft June 27, 2024 00:24
@bfanger
Copy link
Author

bfanger commented Jun 27, 2024

@ota-meshi I was thinking about implementing a svelte/prefer-const rule which would extend prefer-const but ignore signals declarations.

@baseballyama $effect.root is a rune, but not a signal, the svelte/prefer-const rule should convert it to a const

@baseballyama
Copy link
Member

$derived is also rune. Not signal. So I think we need to use rune instead of signal for the rule name.

@bfanger
Copy link
Author

bfanger commented Jun 27, 2024

$derived returns a signal: https://github.com/sveltejs/svelte/blob/8904e0f6cca7c423cd6232ba38870362cf840e63/packages/svelte/src/internal/client/reactivity/deriveds.js#L23

If the svelte compiler injects $.get() when you're using a variable then it's a signal.

@baseballyama
Copy link
Member

I know. But we don't use the word "signals" for Svelte developers. We use "runes" instead. We need to use word which mentioned in the docs as much as possible to reduce cognitive cost.

@bfanger bfanger changed the title feat: signal-prefer-let rule feat: rune-prefer-let rule Jul 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants