Skip to content

Commit a4d3b29

Browse files
feat: add no-unnecessary-state-wrap rule (#1062)
Co-authored-by: Yosuke Ota <otameshiyo23@gmail.com>
1 parent 268a372 commit a4d3b29

26 files changed

+691
-0
lines changed

.changeset/popular-needles-remain.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 `no-unnecessary-state-wrap` rule

README.md

+1
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,7 @@ These rules relate to better ways of doing things to help you avoid problems:
302302
| [svelte/no-reactive-functions](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-functions/) | it's not necessary to define functions in reactive statements | :star::bulb: |
303303
| [svelte/no-reactive-literals](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-literals/) | don't assign literal values in reactive statements | :star::bulb: |
304304
| [svelte/no-svelte-internal](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-svelte-internal/) | svelte/internal will be removed in Svelte 6. | :star: |
305+
| [svelte/no-unnecessary-state-wrap](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unnecessary-state-wrap/) | Disallow unnecessary $state wrapping of reactive classes | :star::bulb: |
305306
| [svelte/no-unused-class-name](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-class-name/) | disallow the use of a class in the template without a corresponding style | |
306307
| [svelte/no-unused-props](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-props/) | Warns about defined Props properties that are unused | :star: |
307308
| [svelte/no-unused-svelte-ignore](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: |

docs/rules.md

+1
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ These rules relate to better ways of doing things to help you avoid problems:
5959
| [svelte/no-reactive-functions](./rules/no-reactive-functions.md) | it's not necessary to define functions in reactive statements | :star::bulb: |
6060
| [svelte/no-reactive-literals](./rules/no-reactive-literals.md) | don't assign literal values in reactive statements | :star::bulb: |
6161
| [svelte/no-svelte-internal](./rules/no-svelte-internal.md) | svelte/internal will be removed in Svelte 6. | :star: |
62+
| [svelte/no-unnecessary-state-wrap](./rules/no-unnecessary-state-wrap.md) | Disallow unnecessary $state wrapping of reactive classes | :star::bulb: |
6263
| [svelte/no-unused-class-name](./rules/no-unused-class-name.md) | disallow the use of a class in the template without a corresponding style | |
6364
| [svelte/no-unused-props](./rules/no-unused-props.md) | Warns about defined Props properties that are unused | :star: |
6465
| [svelte/no-unused-svelte-ignore](./rules/no-unused-svelte-ignore.md) | disallow unused svelte-ignore comments | :star: |
+115
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
---
2+
pageClass: 'rule-details'
3+
sidebarDepth: 0
4+
title: 'svelte/no-unnecessary-state-wrap'
5+
description: 'Disallow unnecessary $state wrapping of reactive classes'
6+
---
7+
8+
# svelte/no-unnecessary-state-wrap
9+
10+
> Disallow unnecessary $state wrapping of reactive classes
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+
In Svelte 5, several built-in classes from `svelte/reactivity` are already reactive by default:
19+
20+
- `SvelteSet`
21+
- `SvelteMap`
22+
- `SvelteURL`
23+
- `SvelteURLSearchParams`
24+
- `SvelteDate`
25+
- `MediaQuery`
26+
27+
Therefore, wrapping them with `$state` is unnecessary and can lead to confusion.
28+
29+
<!--eslint-skip-->
30+
31+
```svelte
32+
<script>
33+
/* eslint svelte/no-unnecessary-state-wrap: "error" */
34+
import {
35+
SvelteSet,
36+
SvelteMap,
37+
SvelteURL,
38+
SvelteURLSearchParams,
39+
SvelteDate,
40+
MediaQuery
41+
} from 'svelte/reactivity';
42+
43+
// ✓ GOOD
44+
const set1 = new SvelteSet();
45+
const map1 = new SvelteMap();
46+
const url1 = new SvelteURL('https://example.com');
47+
const params1 = new SvelteURLSearchParams('key=value');
48+
const date1 = new SvelteDate();
49+
const mediaQuery1 = new MediaQuery('(min-width: 800px)');
50+
51+
// ✗ BAD
52+
const set2 = $state(new SvelteSet());
53+
const map2 = $state(new SvelteMap());
54+
const url2 = $state(new SvelteURL('https://example.com'));
55+
const params2 = $state(new SvelteURLSearchParams('key=value'));
56+
const date2 = $state(new SvelteDate());
57+
const mediaQuery2 = $state(new MediaQuery('(min-width: 800px)'));
58+
</script>
59+
```
60+
61+
## :wrench: Options
62+
63+
```json
64+
{
65+
"svelte/no-unnecessary-state-wrap": [
66+
"error",
67+
{
68+
"additionalReactiveClasses": [],
69+
"allowReassign": false
70+
}
71+
]
72+
}
73+
```
74+
75+
- `additionalReactiveClasses` ... An array of class names that should also be considered reactive. This is useful when you have custom classes that are inherently reactive. Default is `[]`.
76+
- `allowReassign` ... If `true`, allows `$state` wrapping of reactive classes when the variable is reassigned. Default is `false`.
77+
78+
### Examples with Options
79+
80+
#### `additionalReactiveClasses`
81+
82+
```svelte
83+
<script>
84+
/* eslint svelte/no-unnecessary-state-wrap: ["error", { "additionalReactiveClasses": ["MyReactiveClass"] }] */
85+
import { MyReactiveClass } from './foo';
86+
87+
// ✓ GOOD
88+
const myState1 = new MyReactiveClass();
89+
90+
// ✗ BAD
91+
const myState2 = $state(new MyReactiveClass());
92+
</script>
93+
```
94+
95+
#### `allowReassign`
96+
97+
```svelte
98+
<script>
99+
/* eslint svelte/no-unnecessary-state-wrap: ["error", { "allowReassign": true }] */
100+
import { SvelteSet } from 'svelte/reactivity';
101+
102+
// ✓ GOOD
103+
let set1 = $state(new SvelteSet());
104+
set1 = new SvelteSet([1, 2, 3]); // Variable is reassigned
105+
106+
// ✗ BAD
107+
const set2 = $state(new SvelteSet()); // const cannot be reassigned
108+
let set3 = $state(new SvelteSet()); // Variable is never reassigned
109+
</script>
110+
```
111+
112+
## :mag: Implementation
113+
114+
- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/src/rules/no-unnecessary-state-wrap.ts)
115+
- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/tests/src/rules/no-unnecessary-state-wrap.ts)

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

+1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const config: Linter.Config[] = [
3232
'svelte/no-store-async': 'error',
3333
'svelte/no-svelte-internal': 'error',
3434
'svelte/no-unknown-style-directive-property': 'error',
35+
'svelte/no-unnecessary-state-wrap': 'error',
3536
'svelte/no-unused-props': 'error',
3637
'svelte/no-unused-svelte-ignore': 'error',
3738
'svelte/no-useless-children-snippet': 'error',

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

+10
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,11 @@ export interface RuleOptions {
256256
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unknown-style-directive-property/
257257
*/
258258
'svelte/no-unknown-style-directive-property'?: Linter.RuleEntry<SvelteNoUnknownStyleDirectiveProperty>
259+
/**
260+
* Disallow unnecessary $state wrapping of reactive classes
261+
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unnecessary-state-wrap/
262+
*/
263+
'svelte/no-unnecessary-state-wrap'?: Linter.RuleEntry<SvelteNoUnnecessaryStateWrap>
259264
/**
260265
* disallow the use of a class in the template without a corresponding style
261266
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-class-name/
@@ -518,6 +523,11 @@ type SvelteNoUnknownStyleDirectiveProperty = []|[{
518523
ignoreProperties?: [string, ...(string)[]]
519524
ignorePrefixed?: boolean
520525
}]
526+
// ----- svelte/no-unnecessary-state-wrap -----
527+
type SvelteNoUnnecessaryStateWrap = []|[{
528+
additionalReactiveClasses?: string[]
529+
allowReassign?: boolean
530+
}]
521531
// ----- svelte/no-unused-class-name -----
522532
type SvelteNoUnusedClassName = []|[{
523533
allowedClassNames?: string[]
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
import { createRule } from '../utils/index.js';
2+
import { getSourceCode } from '../utils/compat.js';
3+
import { ReferenceTracker } from '@eslint-community/eslint-utils';
4+
import type { TSESTree } from '@typescript-eslint/types';
5+
6+
const REACTIVE_CLASSES = [
7+
'SvelteSet',
8+
'SvelteMap',
9+
'SvelteURL',
10+
'SvelteURLSearchParams',
11+
'SvelteDate',
12+
'MediaQuery'
13+
];
14+
15+
export default createRule('no-unnecessary-state-wrap', {
16+
meta: {
17+
docs: {
18+
description: 'Disallow unnecessary $state wrapping of reactive classes',
19+
category: 'Best Practices',
20+
recommended: true
21+
},
22+
schema: [
23+
{
24+
type: 'object',
25+
properties: {
26+
additionalReactiveClasses: {
27+
type: 'array',
28+
items: {
29+
type: 'string'
30+
},
31+
uniqueItems: true
32+
},
33+
allowReassign: {
34+
type: 'boolean'
35+
}
36+
},
37+
additionalProperties: false
38+
}
39+
],
40+
messages: {
41+
unnecessaryStateWrap: '{{className}} is already reactive, $state wrapping is unnecessary.',
42+
suggestRemoveStateWrap: 'Remove unnecessary $state wrapping'
43+
},
44+
type: 'suggestion',
45+
hasSuggestions: true,
46+
conditions: [
47+
{
48+
svelteVersions: ['5'],
49+
runes: [true, 'undetermined']
50+
}
51+
]
52+
},
53+
create(context) {
54+
const options = context.options[0] ?? {};
55+
const additionalReactiveClasses = options.additionalReactiveClasses ?? [];
56+
const allowReassign = options.allowReassign ?? false;
57+
58+
const referenceTracker = new ReferenceTracker(getSourceCode(context).scopeManager.globalScope!);
59+
const traceMap: Record<string, Record<string, boolean>> = {};
60+
for (const reactiveClass of REACTIVE_CLASSES) {
61+
traceMap[reactiveClass] = {
62+
[ReferenceTracker.CALL]: true,
63+
[ReferenceTracker.CONSTRUCT]: true
64+
};
65+
}
66+
67+
// Track all reactive class imports and their aliases
68+
const references = referenceTracker.iterateEsmReferences({
69+
'svelte/reactivity': {
70+
[ReferenceTracker.ESM]: true,
71+
...traceMap
72+
}
73+
});
74+
75+
const referenceNodeAndNames = Array.from(references).map(({ node, path }) => {
76+
return {
77+
node,
78+
name: path[path.length - 1]
79+
};
80+
});
81+
82+
function isReassigned(identifier: TSESTree.Identifier): boolean {
83+
const variable = getSourceCode(context).scopeManager.getDeclaredVariables(
84+
identifier.parent
85+
)[0];
86+
return variable.references.some((ref) => {
87+
return ref.isWrite() && ref.identifier !== identifier;
88+
});
89+
}
90+
91+
function reportUnnecessaryStateWrap(
92+
stateNode: TSESTree.Node,
93+
targetNode: TSESTree.Node,
94+
className: string,
95+
identifier?: TSESTree.Identifier
96+
) {
97+
if (allowReassign && identifier && isReassigned(identifier)) {
98+
return;
99+
}
100+
101+
context.report({
102+
node: targetNode,
103+
messageId: 'unnecessaryStateWrap',
104+
data: {
105+
className
106+
},
107+
suggest: [
108+
{
109+
messageId: 'suggestRemoveStateWrap',
110+
fix(fixer) {
111+
return fixer.replaceText(stateNode, getSourceCode(context).getText(targetNode));
112+
}
113+
}
114+
]
115+
});
116+
}
117+
118+
return {
119+
CallExpression(node: TSESTree.CallExpression) {
120+
if (node.callee.type !== 'Identifier' || node.callee.name !== '$state') {
121+
return;
122+
}
123+
124+
for (const arg of node.arguments) {
125+
if (
126+
(arg.type === 'NewExpression' || arg.type === 'CallExpression') &&
127+
arg.callee.type === 'Identifier'
128+
) {
129+
const name = arg.callee.name;
130+
if (additionalReactiveClasses.includes(name)) {
131+
const parent = node.parent;
132+
if (parent?.type === 'VariableDeclarator' && parent.id.type === 'Identifier') {
133+
reportUnnecessaryStateWrap(node, arg, name, parent.id);
134+
}
135+
}
136+
}
137+
}
138+
},
139+
140+
'Program:exit': () => {
141+
for (const { node, name } of referenceNodeAndNames) {
142+
if (
143+
node.parent?.type === 'CallExpression' &&
144+
node.parent.callee.type === 'Identifier' &&
145+
node.parent.callee.name === '$state'
146+
) {
147+
const parent = node.parent.parent;
148+
if (parent?.type === 'VariableDeclarator' && parent.id.type === 'Identifier') {
149+
reportUnnecessaryStateWrap(node.parent, node, name, parent.id);
150+
}
151+
}
152+
}
153+
}
154+
};
155+
}
156+
});

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

+2
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import noSvelteInternal from '../rules/no-svelte-internal.js';
5050
import noTargetBlank from '../rules/no-target-blank.js';
5151
import noTrailingSpaces from '../rules/no-trailing-spaces.js';
5252
import noUnknownStyleDirectiveProperty from '../rules/no-unknown-style-directive-property.js';
53+
import noUnnecessaryStateWrap from '../rules/no-unnecessary-state-wrap.js';
5354
import noUnusedClassName from '../rules/no-unused-class-name.js';
5455
import noUnusedProps from '../rules/no-unused-props.js';
5556
import noUnusedSvelteIgnore from '../rules/no-unused-svelte-ignore.js';
@@ -124,6 +125,7 @@ export const rules = [
124125
noTargetBlank,
125126
noTrailingSpaces,
126127
noUnknownStyleDirectiveProperty,
128+
noUnnecessaryStateWrap,
127129
noUnusedClassName,
128130
noUnusedProps,
129131
noUnusedSvelteIgnore,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"svelte": ">=5.0.0"
3+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"options": [
3+
{
4+
"additionalReactiveClasses": ["CustomReactiveClass1", "CustomReactiveClass2"]
5+
}
6+
]
7+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
- message: CustomReactiveClass1 is already reactive, $state wrapping is unnecessary.
2+
line: 5
3+
column: 25
4+
suggestions:
5+
- desc: Remove unnecessary $state wrapping
6+
messageId: suggestRemoveStateWrap
7+
output: |
8+
<script>
9+
import { CustomReactiveClass1, CustomReactiveClass2 } from 'foo';
10+
11+
// These should be reported as unnecessary $state wrapping
12+
const custom1 = new CustomReactiveClass1();
13+
const custom2 = $state(new CustomReactiveClass2());
14+
15+
// Regular state usage is still valid
16+
const regularState = $state(42);
17+
</script>
18+
- message: CustomReactiveClass2 is already reactive, $state wrapping is unnecessary.
19+
line: 6
20+
column: 25
21+
suggestions:
22+
- desc: Remove unnecessary $state wrapping
23+
messageId: suggestRemoveStateWrap
24+
output: |
25+
<script>
26+
import { CustomReactiveClass1, CustomReactiveClass2 } from 'foo';
27+
28+
// These should be reported as unnecessary $state wrapping
29+
const custom1 = $state(new CustomReactiveClass1());
30+
const custom2 = new CustomReactiveClass2();
31+
32+
// Regular state usage is still valid
33+
const regularState = $state(42);
34+
</script>

0 commit comments

Comments
 (0)