Skip to content

Commit 3ddbd83

Browse files
authored
feat: add prefer-writable-derived rule (#1170)
1 parent 2c74993 commit 3ddbd83

27 files changed

+488
-0
lines changed

.changeset/ready-views-burn.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'eslint-plugin-svelte': minor
3+
---
4+
5+
feat: add `prefer-writable-derived` rule

README.md

+1
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,7 @@ These rules relate to better ways of doing things to help you avoid problems:
310310
| [svelte/no-useless-mustaches](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-useless-mustaches/) | disallow unnecessary mustache interpolations | :star::wrench: |
311311
| [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: |
312312
| [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: |
313+
| [svelte/prefer-writable-derived](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-writable-derived/) | Prefer using writable $derived instead of $state and $effect | :star::bulb: |
313314
| [svelte/require-each-key](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-each-key/) | require keyed `{#each}` block | :star: |
314315
| [svelte/require-event-dispatcher-types](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-event-dispatcher-types/) | require type parameters for `createEventDispatcher` | :star: |
315316
| [svelte/require-optimized-style-attribute](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-optimized-style-attribute/) | require style attributes that can be optimized | |

docs/rules.md

+1
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ These rules relate to better ways of doing things to help you avoid problems:
6767
| [svelte/no-useless-mustaches](./rules/no-useless-mustaches.md) | disallow unnecessary mustache interpolations | :star::wrench: |
6868
| [svelte/prefer-const](./rules/prefer-const.md) | Require `const` declarations for variables that are never reassigned after declared | :wrench: |
6969
| [svelte/prefer-destructured-store-props](./rules/prefer-destructured-store-props.md) | destructure values from object stores for better change tracking & fewer redraws | :bulb: |
70+
| [svelte/prefer-writable-derived](./rules/prefer-writable-derived.md) | Prefer using writable $derived instead of $state and $effect | :star::bulb: |
7071
| [svelte/require-each-key](./rules/require-each-key.md) | require keyed `{#each}` block | :star: |
7172
| [svelte/require-event-dispatcher-types](./rules/require-event-dispatcher-types.md) | require type parameters for `createEventDispatcher` | :star: |
7273
| [svelte/require-optimized-style-attribute](./rules/require-optimized-style-attribute.md) | require style attributes that can be optimized | |

docs/rules/prefer-writable-derived.md

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
---
2+
pageClass: 'rule-details'
3+
sidebarDepth: 0
4+
title: 'svelte/prefer-writable-derived'
5+
description: 'Prefer using writable $derived instead of $state and $effect'
6+
---
7+
8+
# svelte/prefer-writable-derived
9+
10+
> Prefer using writable $derived instead of $state and $effect
11+
12+
- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>
13+
- :gear: This rule is included in `"plugin:svelte/recommended"`.
14+
- :bulb: Some problems reported by this rule are manually fixable by editor [suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).
15+
16+
## :book: Rule Details
17+
18+
This rule reports when you use a combination of `$state` and `$effect` to create a derived value that can be written to. It encourages using the more concise and clearer `$derived` syntax instead.
19+
20+
<!--eslint-skip-->
21+
22+
```svelte
23+
<script>
24+
/* eslint svelte/prefer-writable-derived: "error" */
25+
const { initialValue } = $props();
26+
27+
// ✓ GOOD
28+
let value1 = $derived(initialValue);
29+
30+
// ✗ BAD
31+
let value2 = $state(initialValue);
32+
$effect(() => {
33+
value2 = initialValue;
34+
});
35+
</script>
36+
```
37+
38+
The rule specifically looks for patterns where:
39+
40+
1. You initialize a variable with `$state()`
41+
2. You then use `$effect()` or `$effect.pre()` to assign a new value to that same variable
42+
3. The effect function contains only a single assignment statement
43+
44+
When this pattern is detected, the rule suggests refactoring to use `$derived()` instead, which provides the same functionality in a more concise way.
45+
46+
## :wrench: Options
47+
48+
Nothing.
49+
50+
- This rule has no options.
51+
52+
## :books: Further Reading
53+
54+
- [Svelte Documentation on Reactivity Primitives](https://svelte.dev/docs/svelte-components#script-2-assignments-are-reactive)
55+
- [Svelte RFC for Reactivity Primitives](https://github.com/sveltejs/rfcs/blob/rfc-better-primitives/text/0000-better-primitives.md)
56+
57+
## :mag: Implementation
58+
59+
- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/src/rules/prefer-writable-derived.ts)
60+
- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/tests/src/rules/prefer-writable-derived.ts)

packages/eslint-plugin-svelte/src/configs/flat/recommended.ts

+1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const config: Linter.Config[] = [
3737
'svelte/no-unused-svelte-ignore': 'error',
3838
'svelte/no-useless-children-snippet': 'error',
3939
'svelte/no-useless-mustaches': 'error',
40+
'svelte/prefer-writable-derived': 'error',
4041
'svelte/require-each-key': 'error',
4142
'svelte/require-event-dispatcher-types': 'error',
4243
'svelte/require-store-reactive-access': 'error',

packages/eslint-plugin-svelte/src/rule-types.ts

+5
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,11 @@ export interface RuleOptions {
306306
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-style-directive/
307307
*/
308308
'svelte/prefer-style-directive'?: Linter.RuleEntry<[]>
309+
/**
310+
* Prefer using writable $derived instead of $state and $effect
311+
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-writable-derived/
312+
*/
313+
'svelte/prefer-writable-derived'?: Linter.RuleEntry<[]>
309314
/**
310315
* require keyed `{#each}` block
311316
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/require-each-key/
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
import type { TSESTree } from '@typescript-eslint/types';
2+
import { createRule } from '../utils/index.js';
3+
import { getScope } from '../utils/ast-utils.js';
4+
import { VERSION as SVELTE_VERSION } from 'svelte/compiler';
5+
import semver from 'semver';
6+
7+
// Writable derived were introduced in Svelte 5.25.0
8+
const shouldRun = semver.satisfies(SVELTE_VERSION, '>=5.25.0');
9+
10+
type ValidFunctionType = TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression;
11+
type ValidFunction = ValidFunctionType & {
12+
body: TSESTree.BlockStatement;
13+
};
14+
15+
type ValidAssignmentExpression = TSESTree.AssignmentExpression & {
16+
operator: '=';
17+
left: TSESTree.Identifier;
18+
};
19+
20+
type ValidExpressionStatement = TSESTree.ExpressionStatement & {
21+
expression: ValidAssignmentExpression;
22+
};
23+
24+
function isEffectOrEffectPre(node: TSESTree.CallExpression) {
25+
if (node.callee.type === 'Identifier') {
26+
return node.callee.name === '$effect';
27+
}
28+
if (node.callee.type === 'MemberExpression') {
29+
return (
30+
node.callee.object.type === 'Identifier' &&
31+
node.callee.object.name === '$effect' &&
32+
node.callee.property.type === 'Identifier' &&
33+
node.callee.property.name === 'pre'
34+
);
35+
}
36+
37+
return false;
38+
}
39+
40+
function isValidFunctionArgument(argument: TSESTree.Node): argument is ValidFunction {
41+
if (
42+
(argument.type !== 'FunctionExpression' && argument.type !== 'ArrowFunctionExpression') ||
43+
argument.params.length !== 0
44+
) {
45+
return false;
46+
}
47+
48+
if (argument.body.type !== 'BlockStatement') {
49+
return false;
50+
}
51+
52+
return argument.body.body.length === 1;
53+
}
54+
55+
function isValidAssignment(statement: TSESTree.Statement): statement is ValidExpressionStatement {
56+
if (statement.type !== 'ExpressionStatement') return false;
57+
58+
const { expression } = statement;
59+
return (
60+
expression.type === 'AssignmentExpression' &&
61+
expression.operator === '=' &&
62+
expression.left.type === 'Identifier'
63+
);
64+
}
65+
66+
function isStateVariable(init: TSESTree.Expression | null): init is TSESTree.CallExpression {
67+
return (
68+
init?.type === 'CallExpression' &&
69+
init.callee.type === 'Identifier' &&
70+
init.callee.name === '$state'
71+
);
72+
}
73+
74+
export default createRule('prefer-writable-derived', {
75+
meta: {
76+
docs: {
77+
description: 'Prefer using writable $derived instead of $state and $effect',
78+
category: 'Best Practices',
79+
recommended: true
80+
},
81+
schema: [],
82+
messages: {
83+
unexpected: 'Prefer using writable $derived instead of $state and $effect',
84+
suggestRewrite: 'Rewrite $state and $effect to $derived'
85+
},
86+
type: 'suggestion',
87+
conditions: [
88+
{
89+
svelteVersions: ['5'],
90+
runes: [true, 'undetermined']
91+
}
92+
],
93+
hasSuggestions: true
94+
},
95+
create(context) {
96+
if (!shouldRun) {
97+
return {};
98+
}
99+
return {
100+
CallExpression: (node: TSESTree.CallExpression) => {
101+
if (!isEffectOrEffectPre(node) || node.arguments.length !== 1) {
102+
return;
103+
}
104+
105+
const argument = node.arguments[0];
106+
if (!isValidFunctionArgument(argument)) {
107+
return;
108+
}
109+
110+
const statement = argument.body.body[0];
111+
if (!isValidAssignment(statement)) {
112+
return;
113+
}
114+
115+
const { left, right } = statement.expression;
116+
const scope = getScope(context, statement);
117+
const reference = scope.references.find(
118+
(ref) => ref.identifier.type === 'Identifier' && ref.identifier.name === left.name
119+
);
120+
121+
const def = reference?.resolved?.defs?.[0];
122+
if (!def || def.type !== 'Variable' || def.node.type !== 'VariableDeclarator') {
123+
return;
124+
}
125+
126+
const { init } = def.node;
127+
if (!isStateVariable(init)) {
128+
return;
129+
}
130+
131+
context.report({
132+
node: def.node,
133+
messageId: 'unexpected',
134+
suggest: [
135+
{
136+
messageId: 'suggestRewrite',
137+
fix: (fixer) => {
138+
const rightCode = context.sourceCode.getText(right);
139+
return [fixer.replaceText(init, `$derived(${rightCode})`), fixer.remove(node)];
140+
}
141+
}
142+
]
143+
});
144+
}
145+
};
146+
}
147+
});

packages/eslint-plugin-svelte/src/utils/rules.ts

+2
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ import preferClassDirective from '../rules/prefer-class-directive.js';
6060
import preferConst from '../rules/prefer-const.js';
6161
import preferDestructuredStoreProps from '../rules/prefer-destructured-store-props.js';
6262
import preferStyleDirective from '../rules/prefer-style-directive.js';
63+
import preferWritableDerived from '../rules/prefer-writable-derived.js';
6364
import requireEachKey from '../rules/require-each-key.js';
6465
import requireEventDispatcherTypes from '../rules/require-event-dispatcher-types.js';
6566
import requireEventPrefix from '../rules/require-event-prefix.js';
@@ -136,6 +137,7 @@ export const rules = [
136137
preferConst,
137138
preferDestructuredStoreProps,
138139
preferStyleDirective,
140+
preferWritableDerived,
139141
requireEachKey,
140142
requireEventDispatcherTypes,
141143
requireEventPrefix,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"svelte": ">=5.0.0-0"
3+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
- message: Prefer using writable $derived instead of $state and $effect
2+
line: 4
3+
column: 6
4+
suggestions:
5+
- desc: Rewrite $state and $effect to $derived
6+
messageId: suggestRewrite
7+
output: |
8+
<script>
9+
const { albumName } = $props();
10+
11+
let newAlbumName = $derived(albumName);
12+
;
13+
</script>
14+
15+
<input bind:value={newAlbumName} />
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<script>
2+
const { albumName } = $props();
3+
4+
let newAlbumName = $state(albumName);
5+
$effect(() => {
6+
newAlbumName = albumName;
7+
});
8+
</script>
9+
10+
<input bind:value={newAlbumName} />
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
- message: Prefer using writable $derived instead of $state and $effect
2+
line: 4
3+
column: 6
4+
suggestions:
5+
- desc: Rewrite $state and $effect to $derived
6+
messageId: suggestRewrite
7+
output: |
8+
<script>
9+
const { albumName } = $props();
10+
11+
let newAlbumName = $derived(albumName + albumName);
12+
;
13+
</script>
14+
15+
<input bind:value={newAlbumName} />
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<script>
2+
const { albumName } = $props();
3+
4+
let newAlbumName = $state(albumName);
5+
$effect(() => {
6+
newAlbumName = albumName + albumName;
7+
});
8+
</script>
9+
10+
<input bind:value={newAlbumName} />
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
- message: Prefer using writable $derived instead of $state and $effect
2+
line: 4
3+
column: 6
4+
suggestions:
5+
- desc: Rewrite $state and $effect to $derived
6+
messageId: suggestRewrite
7+
output: |
8+
<script>
9+
const { albumName } = $props();
10+
11+
let newAlbumName = $derived(albumName);
12+
;
13+
</script>
14+
15+
<input bind:value={newAlbumName} />
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<script>
2+
const { albumName } = $props();
3+
4+
let newAlbumName = $state(albumName);
5+
$effect.pre(() => {
6+
newAlbumName = albumName;
7+
});
8+
</script>
9+
10+
<input bind:value={newAlbumName} />
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
- message: Prefer using writable $derived instead of $state and $effect
2+
line: 4
3+
column: 6
4+
suggestions:
5+
- desc: Rewrite $state and $effect to $derived
6+
messageId: suggestRewrite
7+
output: |
8+
<script>
9+
const { albumName } = $props();
10+
11+
let newAlbumName = $derived(albumName + albumName);
12+
;
13+
</script>
14+
15+
<input bind:value={newAlbumName} />
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<script>
2+
const { albumName } = $props();
3+
4+
let newAlbumName = $state(albumName);
5+
$effect.pre(() => {
6+
newAlbumName = albumName + albumName;
7+
});
8+
</script>
9+
10+
<input bind:value={newAlbumName} />
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
- message: Prefer using writable $derived instead of $state and $effect
2+
line: 4
3+
column: 6
4+
suggestions:
5+
- desc: Rewrite $state and $effect to $derived
6+
messageId: suggestRewrite
7+
output: |
8+
<script>
9+
const { albumName } = $props();
10+
11+
let newAlbumName = $derived(albumName);
12+
;
13+
14+
setInterval(() => {
15+
newAlbumName = albumName + albumName;
16+
}, 1000);
17+
</script>
18+
19+
<input bind:value={newAlbumName} />

0 commit comments

Comments
 (0)