Skip to content

Commit 1640da2

Browse files
baseballyamabaseballyama
committed
feat: add no-unnecessary-state-wrap rule
1 parent 20a2f32 commit 1640da2

21 files changed

+591
-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
@@ -361,6 +361,7 @@ These rules relate to better ways of doing things to help you avoid problems:
361361
| [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: |
362362
| [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: |
363363
| [svelte/no-svelte-internal](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-svelte-internal/) | svelte/internal will be removed in Svelte 6. | :star: |
364+
| [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: |
364365
| [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 | |
365366
| [svelte/no-unused-svelte-ignore](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: |
366367
| [svelte/no-useless-children-snippet](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-useless-children-snippet/) | disallow explicit children snippet where it's not needed | :star: |

‎docs/rules.md

+1
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ These rules relate to better ways of doing things to help you avoid problems:
5858
| [svelte/no-reactive-functions](./rules/no-reactive-functions.md) | it's not necessary to define functions in reactive statements | :star::bulb: |
5959
| [svelte/no-reactive-literals](./rules/no-reactive-literals.md) | don't assign literal values in reactive statements | :star::bulb: |
6060
| [svelte/no-svelte-internal](./rules/no-svelte-internal.md) | svelte/internal will be removed in Svelte 6. | :star: |
61+
| [svelte/no-unnecessary-state-wrap](./rules/no-unnecessary-state-wrap.md) | Disallow unnecessary $state wrapping of reactive classes | :star::bulb: |
6162
| [svelte/no-unused-class-name](./rules/no-unused-class-name.md) | disallow the use of a class in the template without a corresponding style | |
6263
| [svelte/no-unused-svelte-ignore](./rules/no-unused-svelte-ignore.md) | disallow unused svelte-ignore comments | :star: |
6364
| [svelte/no-useless-children-snippet](./rules/no-useless-children-snippet.md) | disallow explicit children snippet where it's not needed | :star: |

‎docs/rules/no-unnecessary-state-wrap.md

+96
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
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+
}
70+
]
71+
}
72+
```
73+
74+
- `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 `[]`.
75+
76+
### Examples with Options
77+
78+
#### `additionalReactiveClasses`
79+
80+
```svelte
81+
<script>
82+
/* eslint svelte/no-unnecessary-state-wrap: ["error", { "additionalReactiveClasses": ["MyReactiveClass"] }] */
83+
import { MyReactiveClass } from './foo';
84+
85+
// ✓ GOOD
86+
const myState1 = new MyReactiveClass();
87+
88+
// ✗ BAD
89+
const myState2 = $state(new MyReactiveClass());
90+
</script>
91+
```
92+
93+
## :mag: Implementation
94+
95+
- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/src/rules/no-unnecessary-state-wrap.ts)
96+
- [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-svelte-ignore': 'error',
3637
'svelte/no-useless-children-snippet': 'error',
3738
'svelte/no-useless-mustaches': 'error',

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

+9
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/
@@ -508,6 +513,10 @@ type SvelteNoUnknownStyleDirectiveProperty = []|[{
508513
ignoreProperties?: [string, ...(string)[]]
509514
ignorePrefixed?: boolean
510515
}]
516+
// ----- svelte/no-unnecessary-state-wrap -----
517+
type SvelteNoUnnecessaryStateWrap = []|[{
518+
additionalReactiveClasses?: string[]
519+
}]
511520
// ----- svelte/no-unused-class-name -----
512521
type SvelteNoUnusedClassName = []|[{
513522
allowedClassNames?: string[]

‎packages/eslint-plugin-svelte/src/rules/no-unnecessary-state-wrap.ts

+138
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
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+
},
34+
additionalProperties: false
35+
}
36+
],
37+
messages: {
38+
unnecessaryStateWrap: '{{className}} is already reactive, $state wrapping is unnecessary.',
39+
suggestRemoveStateWrap: 'Remove unnecessary $state wrapping'
40+
},
41+
type: 'suggestion',
42+
hasSuggestions: true,
43+
conditions: [
44+
{
45+
svelteVersions: ['5'],
46+
runes: [true, 'undetermined']
47+
}
48+
]
49+
},
50+
create(context) {
51+
const options = context.options[0] ?? {};
52+
const additionalReactiveClasses = options.additionalReactiveClasses ?? [];
53+
54+
const referenceTracker = new ReferenceTracker(getSourceCode(context).scopeManager.globalScope!);
55+
const traceMap: Record<string, Record<string, boolean>> = {};
56+
for (const reactiveClass of REACTIVE_CLASSES) {
57+
traceMap[reactiveClass] = {
58+
[ReferenceTracker.CALL]: true,
59+
[ReferenceTracker.CONSTRUCT]: true
60+
};
61+
}
62+
63+
// Track all reactive class imports and their aliases
64+
const references = referenceTracker.iterateEsmReferences({
65+
'svelte/reactivity': {
66+
[ReferenceTracker.ESM]: true,
67+
...traceMap
68+
}
69+
});
70+
71+
const referenceNodeAndNames = Array.from(references).map(({ node, path }) => {
72+
return {
73+
node,
74+
name: path[path.length - 1]
75+
};
76+
});
77+
78+
function reportUnnecessaryStateWrap(
79+
stateNode: TSESTree.Node,
80+
targetNode: TSESTree.Node,
81+
className: string
82+
) {
83+
context.report({
84+
node: targetNode,
85+
messageId: 'unnecessaryStateWrap',
86+
data: {
87+
className
88+
},
89+
suggest: [
90+
{
91+
messageId: 'suggestRemoveStateWrap',
92+
fix(fixer) {
93+
return fixer.replaceText(stateNode, getSourceCode(context).getText(targetNode));
94+
}
95+
}
96+
]
97+
});
98+
}
99+
100+
return {
101+
CallExpression(node: TSESTree.CallExpression) {
102+
if (node.callee.type !== 'Identifier' || node.callee.name !== '$state') {
103+
return;
104+
}
105+
106+
for (const arg of node.arguments) {
107+
if (
108+
(arg.type === 'NewExpression' || arg.type === 'CallExpression') &&
109+
arg.callee.type === 'Identifier'
110+
) {
111+
const name = arg.callee.name;
112+
if (additionalReactiveClasses.includes(name)) {
113+
const parent = node.parent;
114+
if (parent?.type === 'VariableDeclarator' && parent.id.type === 'Identifier') {
115+
reportUnnecessaryStateWrap(node, arg, name);
116+
}
117+
}
118+
}
119+
}
120+
},
121+
122+
'Program:exit': () => {
123+
for (const { node, name } of referenceNodeAndNames) {
124+
if (
125+
node.parent?.type === 'CallExpression' &&
126+
node.parent.callee.type === 'Identifier' &&
127+
node.parent.callee.name === '$state'
128+
) {
129+
const parent = node.parent.parent;
130+
if (parent?.type === 'VariableDeclarator' && parent.id.type === 'Identifier') {
131+
reportUnnecessaryStateWrap(node.parent, node, name);
132+
}
133+
}
134+
}
135+
}
136+
};
137+
}
138+
});

‎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 noUnusedSvelteIgnore from '../rules/no-unused-svelte-ignore.js';
5556
import noUselessChildrenSnippet from '../rules/no-useless-children-snippet.js';
@@ -122,6 +123,7 @@ export const rules = [
122123
noTargetBlank,
123124
noTrailingSpaces,
124125
noUnknownStyleDirectiveProperty,
126+
noUnnecessaryStateWrap,
125127
noUnusedClassName,
126128
noUnusedSvelteIgnore,
127129
noUselessChildrenSnippet,

‎packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/_requirements.json

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"svelte": ">=5.0.0"
3+
}

‎packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-config.json

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"options": [
3+
{
4+
"additionalReactiveClasses": ["CustomReactiveClass1", "CustomReactiveClass2"]
5+
}
6+
]
7+
}

‎packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-errors.yaml

+34
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>

‎packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-input.svelte

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<script>
2+
import { CustomReactiveClass1, CustomReactiveClass2 } from 'foo';
3+
4+
// These should be reported as unnecessary $state wrapping
5+
const custom1 = $state(new CustomReactiveClass1());
6+
const custom2 = $state(new CustomReactiveClass2());
7+
8+
// Regular state usage is still valid
9+
const regularState = $state(42);
10+
</script>

0 commit comments

Comments
 (0)