sveltejs / eslint-plugin-svelte

ESLint plugin for Svelte using AST
https://sveltejs.github.io/eslint-plugin-svelte/
MIT License
276 stars 29 forks source link

feat: signal-prefer-let rule #806

Open bfanger opened 5 days ago

bfanger commented 5 days ago

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()
changeset-bot[bot] commented 5 days ago

🦋 Changeset detected

Latest commit: 642e4dd8a6cc23bcbb05d8af0205d5789d968b9a

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

baseballyama commented 5 days ago

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 commented 4 days ago

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?

bfanger commented 4 days ago

@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 commented 4 days ago

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

bfanger commented 4 days ago

$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 commented 3 days ago

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.