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(prefer-let): add rule #985

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/small-months-exercise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'eslint-plugin-svelte': major
---

Adds `prefer-let` rule
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -370,6 +370,7 @@ These rules relate to better ways of doing things to help you avoid problems:
| [svelte/no-useless-mustaches](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-useless-mustaches/) | disallow unnecessary mustache interpolations | :wrench: |
| [svelte/prefer-const](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-const/) | Require `const` declarations for variables that are never reassigned after declared | :wrench: |
| [svelte/prefer-destructured-store-props](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-destructured-store-props/) | destructure values from object stores for better change tracking & fewer redraws | :bulb: |
| [svelte/prefer-let](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-let/) | Prefer `let` over `const` for Svelte 5 reactive variable declarations. | :wrench: |
| [svelte/require-each-key](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-each-key/) | require keyed `{#each}` block | |
| [svelte/require-event-dispatcher-types](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-event-dispatcher-types/) | require type parameters for `createEventDispatcher` | |
| [svelte/require-optimized-style-attribute](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-optimized-style-attribute/) | require style attributes that can be optimized | |
1 change: 1 addition & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
@@ -67,6 +67,7 @@ These rules relate to better ways of doing things to help you avoid problems:
| [svelte/no-useless-mustaches](./rules/no-useless-mustaches.md) | disallow unnecessary mustache interpolations | :wrench: |
| [svelte/prefer-const](./rules/prefer-const.md) | Require `const` declarations for variables that are never reassigned after declared | :wrench: |
| [svelte/prefer-destructured-store-props](./rules/prefer-destructured-store-props.md) | destructure values from object stores for better change tracking & fewer redraws | :bulb: |
| [svelte/prefer-let](./rules/prefer-let.md) | Prefer `let` over `const` for Svelte 5 reactive variable declarations. | :wrench: |
| [svelte/require-each-key](./rules/require-each-key.md) | require keyed `{#each}` block | |
| [svelte/require-event-dispatcher-types](./rules/require-event-dispatcher-types.md) | require type parameters for `createEventDispatcher` | |
| [svelte/require-optimized-style-attribute](./rules/require-optimized-style-attribute.md) | require style attributes that can be optimized | |
58 changes: 58 additions & 0 deletions docs/rules/prefer-let.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
---
pageClass: 'rule-details'
sidebarDepth: 0
title: 'svelte/prefer-let'
description: 'Prefer `let` over `const` for Svelte 5 reactive variable declarations.'
---

# svelte/prefer-let

> Prefer `let` over `const` for Svelte 5 reactive variable declarations.
- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>
- :wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.

## :book: Rule Details

This rule reports usages of `const` variable declarations on Svelte reactive
function assignments. While values may not be reassigned in the code itself,
they are reassigned by Svelte.

<!--eslint-skip-->

```svelte
<script>
/* eslint svelte/prefer-let: "error" */
// ✓ GOOD
let { a, b } = $props();
let c = $state('');
let d = $derived(a * 2);
let e = $derived.by(() => b * 2);
// ✗ BAD
const g = $state(0);
const h = $derived({ count: g });
</script>
```

## :wrench: Options

```json
{
"svelte/prefer-const": [
"error",
{
"exclude": ["$props", "$derived", "$derived.by", "$state", "$state.raw"]
}
]
}
```

- `exclude`: The reactive assignments that you want to exclude from being
reported.

## :mag: Implementation

- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/src/rules/prefer-let.ts)
- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/tests/src/rules/prefer-let.ts)
9 changes: 9 additions & 0 deletions packages/eslint-plugin-svelte/src/rule-types.ts
Original file line number Diff line number Diff line change
@@ -285,6 +285,11 @@ export interface RuleOptions {
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-destructured-store-props/
*/
'svelte/prefer-destructured-store-props'?: Linter.RuleEntry<[]>
/**
* Prefer `let` over `const` for Svelte 5 reactive variable declarations.
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-let/
*/
'svelte/prefer-let'?: Linter.RuleEntry<SveltePreferLet>
/**
* require style directives instead of style attribute
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-style-directive/
@@ -513,6 +518,10 @@ type SveltePreferConst = []|[{
destructuring?: ("any" | "all")
ignoreReadBeforeAssign?: boolean
}]
// ----- svelte/prefer-let -----
type SveltePreferLet = []|[{
exclude?: ("$props" | "$derived" | "$derived.by" | "$state" | "$state.raw")[]
}]
// ----- svelte/shorthand-attribute -----
type SvelteShorthandAttribute = []|[{
prefer?: ("always" | "never")
90 changes: 90 additions & 0 deletions packages/eslint-plugin-svelte/src/rules/prefer-let.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import type { TSESTree } from '@typescript-eslint/types';

import { createRule } from '../utils/index.js';

type ReactiveFunction = '$props' | '$derived' | '$derived.by' | '$state' | '$state.raw';
const DEFAULT_FUNCTIONS: ReactiveFunction[] = [
'$props',
'$derived',
'$derived.by',
'$state',
'$state.raw'
];

function getReactiveFunction(callExpr: TSESTree.CallExpression, validNames: string[]) {
if (callExpr.callee.type === 'Identifier') {
if (validNames.includes(callExpr.callee.name)) {
return callExpr.callee.name as ReactiveFunction;
}
} else if (
callExpr.callee.type === 'MemberExpression' &&
callExpr.callee.object.type === 'Identifier' &&
callExpr.callee.property.type === 'Identifier'
) {
const fullName = `${callExpr.callee.object.name}.${callExpr.callee.property.name}`;

if (validNames.includes(fullName)) {
return fullName as ReactiveFunction;
}
}

return null;
}

export default createRule('prefer-let', {
meta: {
docs: {
description: 'Prefer `let` over `const` for Svelte 5 reactive variable declarations.',
category: 'Best Practices',
recommended: false
},
schema: [
{
type: 'object',
properties: {
exclude: {
type: 'array',
items: {
enum: ['$props', '$derived', '$derived.by', '$state', '$state.raw']
},
uniqueItems: true
}
},
additionalProperties: false
}
],
messages: {
'use-let': "'const' is used for a reactive declaration from {{rune}}. Use 'let' instead."
},
type: 'suggestion',
fixable: 'code'
},
create(context) {
const exclude = context.options[0]?.exclude ?? [];
const allowedNames = DEFAULT_FUNCTIONS.filter((name) => !exclude.includes(name));

return {
VariableDeclaration(node: TSESTree.VariableDeclaration) {
if (node.kind === 'const') {
node.declarations.forEach((declarator) => {
const init = declarator.init;

if (!init || init.type !== 'CallExpression') {
return;
}

const rune = getReactiveFunction(init, allowedNames);
if (rune) {
context.report({
node,
messageId: 'use-let',
data: { rune },
fix: (fixer) => fixer.replaceTextRange([node.range[0], node.range[0] + 5], 'let')
});
}
});
}
}
};
}
});
2 changes: 2 additions & 0 deletions packages/eslint-plugin-svelte/src/utils/rules.ts
Original file line number Diff line number Diff line change
@@ -56,6 +56,7 @@ import noUselessMustaches from '../rules/no-useless-mustaches.js';
import preferClassDirective from '../rules/prefer-class-directive.js';
import preferConst from '../rules/prefer-const.js';
import preferDestructuredStoreProps from '../rules/prefer-destructured-store-props.js';
import preferLet from '../rules/prefer-let.js';
import preferStyleDirective from '../rules/prefer-style-directive.js';
import requireEachKey from '../rules/require-each-key.js';
import requireEventDispatcherTypes from '../rules/require-event-dispatcher-types.js';
@@ -127,6 +128,7 @@ export const rules = [
preferClassDirective,
preferConst,
preferDestructuredStoreProps,
preferLet,
preferStyleDirective,
requireEachKey,
requireEventDispatcherTypes,
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ "options": [{ "exclude": ["$derived", "$derived.by"] }] }
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- message: "'const' is used for a reactive declaration from $state. Use 'let' instead."
line: 2
column: 2
suggestions: null
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
const a = $state();
const b = $derived(a);
const c = $derived.by(() => b);
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
let a = $state();
const b = $derived(a);
const c = $derived.by(() => b);
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
- message: "'const' is used for a reactive declaration from $props. Use 'let' instead."
line: 2
column: 2
suggestions: null
- message: "'const' is used for a reactive declaration from $state. Use 'let' instead."
line: 3
column: 2
suggestions: null
- message: "'const' is used for a reactive declaration from $state.raw. Use 'let'
instead."
line: 4
column: 2
suggestions: null
- message: "'const' is used for a reactive declaration from $derived. Use 'let'
instead."
line: 5
column: 2
suggestions: null
- message: "'const' is used for a reactive declaration from $derived.by. Use 'let'
instead."
line: 6
column: 2
suggestions: null
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
const { prop } = $props();
const state = $state();
const raw = $state.raw();
const derived = $derived(state + 1);
const derivedBy = $derived.by(() => prop + state);
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
let { prop } = $props();
let state = $state();
let raw = $state.raw();
let derived = $derived(state + 1);
let derivedBy = $derived.by(() => prop + state);
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
let { prop } = $props();
let state = $state();
let raw = $state.raw();
let derived = $derived(state + 1);
let derivedBy = $derived.by(() => prop + state);
</script>
6 changes: 3 additions & 3 deletions packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { RuleTester } from '../../utils/eslint-compat';
import rule from '../../../src/rules/prefer-const';
import { loadTestCases } from '../../utils/utils';
import { RuleTester } from '../../utils/eslint-compat.js';
import rule from '../../../src/rules/prefer-const.js';
import { loadTestCases } from '../../utils/utils.js';

const tester = new RuleTester({
languageOptions: {
12 changes: 12 additions & 0 deletions packages/eslint-plugin-svelte/tests/src/rules/prefer-let.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { RuleTester } from '../../utils/eslint-compat.js';
import rule from '../../../src/rules/prefer-let.js';
import { loadTestCases } from '../../utils/utils.js';

const tester = new RuleTester({
languageOptions: {
ecmaVersion: 2020,
sourceType: 'module'
},
});

tester.run('prefer-let', rule as any, loadTestCases('prefer-let'));

Unchanged files with check annotations Beta

description:
"Svelte runtime prevents calling the same reactive statement twice in a microtask. But between different microtask, it doesn't prevent.",
category: 'Possible Errors',
// TODO Switch to recommended in the major version.

Check warning on line 372 in packages/eslint-plugin-svelte/src/rules/infinite-reactive-loop.ts

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO Switch to recommended in the major...'
recommended: false
},
schema: [],
docs: {
description: 'Recommends not using raw special elements in Svelte versions previous to 5.',
category: 'Possible Errors',
// TODO: Switch to recommended in the major version

Check warning on line 12 in packages/eslint-plugin-svelte/src/rules/no-deprecated-raw-special-elements.ts

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO: Switch to recommended in the major...'
recommended: false
},
schema: [],
description:
'disallow exporting load functions in `*.svelte` module in SvelteKit page components.',
category: 'Possible Errors',
// TODO Switch to recommended in the major version.

Check warning on line 11 in packages/eslint-plugin-svelte/src/rules/no-export-load-in-svelte-module-in-kit-pages.ts

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO Switch to recommended in the major...'
recommended: false
},
schema: [],
docs: {
description: "disallow reactive statements that don't reference reactive values.",
category: 'Best Practices',
// TODO Switch to recommended in the major version.

Check warning on line 13 in packages/eslint-plugin-svelte/src/rules/no-immutable-reactive-statements.ts

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO Switch to recommended in the major...'
recommended: false
},
schema: [],
docs: {
description: 'Warns against the use of `$inspect` directive',
category: 'Best Practices',
// TODO: Enable recommended in major version

Check warning on line 10 in packages/eslint-plugin-svelte/src/rules/no-inspect.ts

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO: Enable recommended in major...'
recommended: false,
default: 'warn'
},
docs: {
description: 'disallow reassigning reactive values',
category: 'Possible Errors',
// TODO Switch to recommended in the major version.

Check warning on line 12 in packages/eslint-plugin-svelte/src/rules/no-reactive-reassign.ts

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO Switch to recommended in the major...'
recommended: false
},
schema: [
description:
'disallow using async/await inside svelte stores because it causes issues with the auto-unsubscribing features',
category: 'Possible Errors',
// TODO Switch to recommended in the major version.

Check warning on line 10 in packages/eslint-plugin-svelte/src/rules/no-store-async.ts

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO Switch to recommended in the major...'
// recommended: true,
recommended: false,
default: 'error'
docs: {
description: 'svelte/internal will be removed in Svelte 6.',
category: 'Best Practices',
// TODO Switch to recommended in the major version.

Check warning on line 9 in packages/eslint-plugin-svelte/src/rules/no-svelte-internal.ts

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO Switch to recommended in the major...'
// recommended: true,
recommended: false
},
description:
'disallow to use of the store itself as an operand. Need to use $ prefix or get function.',
category: 'Possible Errors',
// TODO Switch to recommended in the major version.

Check warning on line 13 in packages/eslint-plugin-svelte/src/rules/require-store-reactive-access.ts

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO Switch to recommended in the major...'
// recommended: true,
recommended: false
},
docs: {
description: 'enforce keys to use variables defined in the `{#each}` block',
category: 'Best Practices',
// TODO Switch to recommended in the major version.

Check warning on line 10 in packages/eslint-plugin-svelte/src/rules/valid-each-key.ts

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO Switch to recommended in the major...'
recommended: false
},
schema: [],