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: add prefer-writable-derived rule #1170

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
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/ready-views-burn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'eslint-plugin-svelte': minor
---

feat: add `prefer-writable-derived` rule
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,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 | :star::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-writable-derived](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-writable-derived/) | Prefer using writable $derived instead of $state and $effect | :star::bulb: |
| [svelte/require-each-key](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-each-key/) | require keyed `{#each}` block | :star: |
| [svelte/require-event-dispatcher-types](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-event-dispatcher-types/) | require type parameters for `createEventDispatcher` | :star: |
| [svelte/require-optimized-style-attribute](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-optimized-style-attribute/) | require style attributes that can be optimized | |
Expand Down
1 change: 1 addition & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | :star::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-writable-derived](./rules/prefer-writable-derived.md) | Prefer using writable $derived instead of $state and $effect | :star::bulb: |
| [svelte/require-each-key](./rules/require-each-key.md) | require keyed `{#each}` block | :star: |
| [svelte/require-event-dispatcher-types](./rules/require-event-dispatcher-types.md) | require type parameters for `createEventDispatcher` | :star: |
| [svelte/require-optimized-style-attribute](./rules/require-optimized-style-attribute.md) | require style attributes that can be optimized | |
Expand Down
60 changes: 60 additions & 0 deletions docs/rules/prefer-writable-derived.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
---
pageClass: 'rule-details'
sidebarDepth: 0
title: 'svelte/prefer-writable-derived'
description: 'Prefer using writable $derived instead of $state and $effect'
---

# svelte/prefer-writable-derived

> Prefer using writable $derived instead of $state and $effect

- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>
- :gear: This rule is included in `"plugin:svelte/recommended"`.
- :bulb: Some problems reported by this rule are manually fixable by editor [suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).

## :book: Rule Details

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.

<!--eslint-skip-->

```svelte
<script>
/* eslint svelte/prefer-writable-derived: "error" */
const { initialValue } = $props();

// ✓ GOOD
let value1 = $derived(initialValue);

// ✗ BAD
let value2 = $state(initialValue);
$effect(() => {
value2 = initialValue;
});
</script>
```

The rule specifically looks for patterns where:

1. You initialize a variable with `$state()`
2. You then use `$effect()` or `$effect.pre()` to assign a new value to that same variable
3. The effect function contains only a single assignment statement

When this pattern is detected, the rule suggests refactoring to use `$derived()` instead, which provides the same functionality in a more concise way.

## :wrench: Options

Nothing.

- This rule has no options.

## :books: Further Reading

- [Svelte Documentation on Reactivity Primitives](https://svelte.dev/docs/svelte-components#script-2-assignments-are-reactive)
- [Svelte RFC for Reactivity Primitives](https://github.com/sveltejs/rfcs/blob/rfc-better-primitives/text/0000-better-primitives.md)

## :mag: Implementation

- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/src/rules/prefer-writable-derived.ts)
- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/tests/src/rules/prefer-writable-derived.ts)
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const config: Linter.Config[] = [
'svelte/no-unused-svelte-ignore': 'error',
'svelte/no-useless-children-snippet': 'error',
'svelte/no-useless-mustaches': 'error',
'svelte/prefer-writable-derived': 'error',
'svelte/require-each-key': 'error',
'svelte/require-event-dispatcher-types': 'error',
'svelte/require-store-reactive-access': 'error',
Expand Down
5 changes: 5 additions & 0 deletions packages/eslint-plugin-svelte/src/rule-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,11 @@ export interface RuleOptions {
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-style-directive/
*/
'svelte/prefer-style-directive'?: Linter.RuleEntry<[]>
/**
* Prefer using writable $derived instead of $state and $effect
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-writable-derived/
*/
'svelte/prefer-writable-derived'?: Linter.RuleEntry<[]>
/**
* require keyed `{#each}` block
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/require-each-key/
Expand Down
147 changes: 147 additions & 0 deletions packages/eslint-plugin-svelte/src/rules/prefer-writable-derived.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
import type { TSESTree } from '@typescript-eslint/types';
import { createRule } from '../utils/index.js';
import { getScope } from '../utils/ast-utils.js';
import { VERSION as SVELTE_VERSION } from 'svelte/compiler';
import semver from 'semver';

// Writable derived were introduced in Svelte 5.25.0
const shouldRun = semver.satisfies(SVELTE_VERSION, '>=5.25.0');

type ValidFunctionType = TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression;
type ValidFunction = ValidFunctionType & {
body: TSESTree.BlockStatement;
};

type ValidAssignmentExpression = TSESTree.AssignmentExpression & {
operator: '=';
left: TSESTree.Identifier;
};

type ValidExpressionStatement = TSESTree.ExpressionStatement & {
expression: ValidAssignmentExpression;
};

function isEffectOrEffectPre(node: TSESTree.CallExpression) {
if (node.callee.type === 'Identifier') {
return node.callee.name === '$effect';
}
if (node.callee.type === 'MemberExpression') {
return (
node.callee.object.type === 'Identifier' &&
node.callee.object.name === '$effect' &&
node.callee.property.type === 'Identifier' &&
node.callee.property.name === 'pre'
);
}

return false;
}

function isValidFunctionArgument(argument: TSESTree.Node): argument is ValidFunction {
if (
(argument.type !== 'FunctionExpression' && argument.type !== 'ArrowFunctionExpression') ||
argument.params.length !== 0
) {
return false;
}

if (argument.body.type !== 'BlockStatement') {
return false;
}

return argument.body.body.length === 1;
}

function isValidAssignment(statement: TSESTree.Statement): statement is ValidExpressionStatement {
if (statement.type !== 'ExpressionStatement') return false;

const { expression } = statement;
return (
expression.type === 'AssignmentExpression' &&
expression.operator === '=' &&
expression.left.type === 'Identifier'
);
}

function isStateVariable(init: TSESTree.Expression | null): init is TSESTree.CallExpression {
return (
init?.type === 'CallExpression' &&
init.callee.type === 'Identifier' &&
init.callee.name === '$state'
);
}

export default createRule('prefer-writable-derived', {
meta: {
docs: {
description: 'Prefer using writable $derived instead of $state and $effect',
category: 'Best Practices',
recommended: true
},
schema: [],
messages: {
unexpected: 'Prefer using writable $derived instead of $state and $effect',
suggestRewrite: 'Rewrite $state and $effect to $derived'
},
type: 'suggestion',
conditions: [
{
svelteVersions: ['5'],
runes: [true, 'undetermined']
}
],
hasSuggestions: true
},
create(context) {
if (!shouldRun) {
return {};
}
return {
CallExpression: (node: TSESTree.CallExpression) => {
if (!isEffectOrEffectPre(node) || node.arguments.length !== 1) {
return;
}

const argument = node.arguments[0];
if (!isValidFunctionArgument(argument)) {
return;
}

const statement = argument.body.body[0];
if (!isValidAssignment(statement)) {
return;
}

const { left, right } = statement.expression;
const scope = getScope(context, statement);
const reference = scope.references.find(
(ref) => ref.identifier.type === 'Identifier' && ref.identifier.name === left.name
);

const def = reference?.resolved?.defs?.[0];
if (!def || def.type !== 'Variable' || def.node.type !== 'VariableDeclarator') {
return;
}

const { init } = def.node;
if (!isStateVariable(init)) {
return;
}

context.report({
node: def.node,
messageId: 'unexpected',
suggest: [
{
messageId: 'suggestRewrite',
fix: (fixer) => {
const rightCode = context.sourceCode.getText(right);
return [fixer.replaceText(init, `$derived(${rightCode})`), fixer.remove(node)];
}
}
]
});
}
};
}
});
2 changes: 2 additions & 0 deletions packages/eslint-plugin-svelte/src/utils/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ 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 preferStyleDirective from '../rules/prefer-style-directive.js';
import preferWritableDerived from '../rules/prefer-writable-derived.js';
import requireEachKey from '../rules/require-each-key.js';
import requireEventDispatcherTypes from '../rules/require-event-dispatcher-types.js';
import requireOptimizedStyleAttribute from '../rules/require-optimized-style-attribute.js';
Expand Down Expand Up @@ -135,6 +136,7 @@ export const rules = [
preferConst,
preferDestructuredStoreProps,
preferStyleDirective,
preferWritableDerived,
requireEachKey,
requireEventDispatcherTypes,
requireOptimizedStyleAttribute,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"svelte": ">=5.0.0-0"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
- message: Prefer using writable $derived instead of $state and $effect
line: 4
column: 6
suggestions:
- desc: Rewrite $state and $effect to $derived
messageId: suggestRewrite
output: |
<script>
const { albumName } = $props();

let newAlbumName = $derived(albumName);
;
</script>

<input bind:value={newAlbumName} />
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<script>
const { albumName } = $props();

let newAlbumName = $state(albumName);
$effect(() => {
newAlbumName = albumName;
});
</script>

<input bind:value={newAlbumName} />
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
- message: Prefer using writable $derived instead of $state and $effect
line: 4
column: 6
suggestions:
- desc: Rewrite $state and $effect to $derived
messageId: suggestRewrite
output: |
<script>
const { albumName } = $props();

let newAlbumName = $derived(albumName + albumName);
;
</script>

<input bind:value={newAlbumName} />
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<script>
const { albumName } = $props();

let newAlbumName = $state(albumName);
$effect(() => {
newAlbumName = albumName + albumName;
});
</script>

<input bind:value={newAlbumName} />
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
- message: Prefer using writable $derived instead of $state and $effect
line: 4
column: 6
suggestions:
- desc: Rewrite $state and $effect to $derived
messageId: suggestRewrite
output: |
<script>
const { albumName } = $props();

let newAlbumName = $derived(albumName);
;
</script>

<input bind:value={newAlbumName} />
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<script>
const { albumName } = $props();

let newAlbumName = $state(albumName);
$effect.pre(() => {
newAlbumName = albumName;
});
</script>

<input bind:value={newAlbumName} />
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
- message: Prefer using writable $derived instead of $state and $effect
line: 4
column: 6
suggestions:
- desc: Rewrite $state and $effect to $derived
messageId: suggestRewrite
output: |
<script>
const { albumName } = $props();

let newAlbumName = $derived(albumName + albumName);
;
</script>

<input bind:value={newAlbumName} />
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<script>
const { albumName } = $props();

let newAlbumName = $state(albumName);
$effect.pre(() => {
newAlbumName = albumName + albumName;
});
</script>

<input bind:value={newAlbumName} />
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
- message: Prefer using writable $derived instead of $state and $effect
line: 4
column: 6
suggestions:
- desc: Rewrite $state and $effect to $derived
messageId: suggestRewrite
output: |
<script>
const { albumName } = $props();

let newAlbumName = $derived(albumName);
;

setInterval(() => {
newAlbumName = albumName + albumName;
}, 1000);
</script>

<input bind:value={newAlbumName} />
Loading
Loading