Skip to content

feat(no-unnecessary-state-wrap): support string array in allowReassign, default to ['SvelteSet'], and improve messages #1202

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
32 changes: 29 additions & 3 deletions docs/rules/no-unnecessary-state-wrap.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,17 @@ Therefore, wrapping them with `$state` is unnecessary and can lead to confusion.
"error",
{
"additionalReactiveClasses": [],
"allowReassign": false
"allowReassign": ["SvelteSet"]
}
]
}
```

- `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 `[]`.
- `allowReassign` ... If `true`, allows `$state` wrapping of reactive classes when the variable is reassigned. Default is `false`.
- `allowReassign` ... Can be either a boolean or an array of class names:
- If `true`, allows `$state` wrapping of any reactive classes when the variable is reassigned.
- If an array, allows `$state` wrapping for the specified reactive classes when the variable is reassigned.
- Default is `["SvelteSet"]`.

### Examples with Options

Expand All @@ -97,13 +100,36 @@ Therefore, wrapping them with `$state` is unnecessary and can lead to confusion.
```svelte
<script>
/* eslint svelte/no-unnecessary-state-wrap: ["error", { "allowReassign": true }] */
import { SvelteSet } from 'svelte/reactivity';
import { SvelteSet, SvelteMap } from 'svelte/reactivity';

// ✓ GOOD
let set1 = $state(new SvelteSet());
set1 = new SvelteSet([1, 2, 3]); // Variable is reassigned

let map1 = $state(new SvelteMap());
map1 = new SvelteMap(); // Variable is reassigned

// ✗ BAD
const set2 = $state(new SvelteSet()); // const cannot be reassigned
let set3 = $state(new SvelteSet()); // Variable is never reassigned
</script>
```

#### `allowReassign` as array

```svelte
<script>
/* eslint svelte/no-unnecessary-state-wrap: ["error", { "allowReassign": ["SvelteSet"] }] */
import { SvelteSet, SvelteMap } from 'svelte/reactivity';

// ✓ GOOD
let set = $state(new SvelteSet());
set = new SvelteSet([1, 2, 3]); // SvelteSet is in allowReassign and variable is reassigned

// ✗ BAD
let map = $state(new SvelteMap()); // SvelteMap is not in allowReassign
map = new SvelteMap();

const set2 = $state(new SvelteSet()); // const cannot be reassigned
let set3 = $state(new SvelteSet()); // Variable is never reassigned
</script>
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin-svelte/src/rule-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ type SvelteNoUnknownStyleDirectiveProperty = []|[{
// ----- svelte/no-unnecessary-state-wrap -----
type SvelteNoUnnecessaryStateWrap = []|[{
additionalReactiveClasses?: string[]
allowReassign?: boolean
allowReassign?: (boolean | string[])
}]
// ----- svelte/no-unused-class-name -----
type SvelteNoUnusedClassName = []|[{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,28 @@ export default createRule('no-unnecessary-state-wrap', {
uniqueItems: true
},
allowReassign: {
type: 'boolean'
oneOf: [
{
type: 'boolean'
},
{
type: 'array',
items: {
type: 'string'
},
uniqueItems: true
}
]
}
},
additionalProperties: false
}
],
messages: {
unnecessaryStateWrap: '{{className}} is already reactive, $state wrapping is unnecessary.',
suggestRemoveStateWrap: 'Remove unnecessary $state wrapping'
unnecessaryStateWrap:
'{{className}} is already reactive. `$state` wrapping is unnecessary. Update it with mutations (e.g., .add(), .delete(), .push(), .splice()).',
suggestRemoveStateWrap:
'Remove the unnecessary `$state` and update the value via mutations (e.g., .add(), .delete(), .push(), .splice()).'
},
type: 'suggestion',
hasSuggestions: true,
Expand All @@ -52,7 +65,7 @@ export default createRule('no-unnecessary-state-wrap', {
create(context) {
const options = context.options[0] ?? {};
const additionalReactiveClasses = options.additionalReactiveClasses ?? [];
const allowReassign = options.allowReassign ?? false;
const allowReassign = options.allowReassign ?? ['SvelteSet'];
const { globalScope } = context.sourceCode.scopeManager;
if (globalScope == null) {
return {};
Expand Down Expand Up @@ -91,13 +104,27 @@ export default createRule('no-unnecessary-state-wrap', {
});
}

function isAllowedReassign(className: string, identifier?: TSESTree.Identifier): boolean {
if (!identifier) return false;

if (typeof allowReassign === 'boolean') {
return allowReassign && isReassigned(identifier);
}

return (
Array.isArray(allowReassign) &&
allowReassign.includes(className) &&
isReassigned(identifier)
);
}

function reportUnnecessaryStateWrap(
stateNode: TSESTree.Node,
targetNode: TSESTree.Node,
className: string,
identifier?: TSESTree.Identifier
) {
if (allowReassign && identifier && isReassigned(identifier)) {
if (isAllowedReassign(className, identifier)) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
- message: CustomReactiveClass1 is already reactive, $state wrapping is unnecessary.
- message: CustomReactiveClass1 is already reactive. `$state` wrapping is unnecessary. Update it with mutations (e.g., .add(), .delete(), .push(), .splice()).
line: 5
column: 25
suggestions:
- desc: Remove unnecessary $state wrapping
- desc: Remove the unnecessary `$state` and update the value via mutations (e.g., .add(), .delete(), .push(), .splice()).
messageId: suggestRemoveStateWrap
output: |
<script>
Expand All @@ -15,11 +15,11 @@
// Regular state usage is still valid
const regularState = $state(42);
</script>
- message: CustomReactiveClass2 is already reactive, $state wrapping is unnecessary.
- message: CustomReactiveClass2 is already reactive. `$state` wrapping is unnecessary. Update it with mutations (e.g., .add(), .delete(), .push(), .splice()).
line: 6
column: 25
suggestions:
- desc: Remove unnecessary $state wrapping
- desc: Remove the unnecessary `$state` and update the value via mutations (e.g., .add(), .delete(), .push(), .splice()).
messageId: suggestRemoveStateWrap
output: |
<script>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"options": [
{
"allowReassign": ["SvelteSet"]
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
- message: SvelteMap is already reactive. `$state` wrapping is unnecessary. Update it with mutations (e.g., .add(), .delete(), .push(), .splice()).
line: 11
column: 19
suggestions:
- desc: Remove the unnecessary `$state` and update the value via mutations (e.g., .add(), .delete(), .push(), .splice()).
messageId: suggestRemoveStateWrap
output: |
<script>
import { SvelteSet, SvelteMap } from 'svelte/reactivity';

// This should NOT be reported as unnecessary $state wrapping
// because SvelteSet is in the allowReassign array and is reassigned
let set = $state(new SvelteSet());
set = new SvelteSet([1, 2, 3]);

// This should be reported as unnecessary $state wrapping
// because SvelteMap is not in the allowReassign array
let map = new SvelteMap();
map = new SvelteMap();
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<script>
import { SvelteSet, SvelteMap } from 'svelte/reactivity';
// This should NOT be reported as unnecessary $state wrapping
// because SvelteSet is in the allowReassign array and is reassigned
let set = $state(new SvelteSet());
set = new SvelteSet([1, 2, 3]);
// This should be reported as unnecessary $state wrapping
// because SvelteMap is not in the allowReassign array
let map = $state(new SvelteMap());
map = new SvelteMap();
</script>
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
- message: SvelteSet is already reactive, $state wrapping is unnecessary.
- message: SvelteSet is already reactive. `$state` wrapping is unnecessary. Update it with mutations (e.g., .add(), .delete(), .push(), .splice()).
line: 6
column: 21
suggestions:
- desc: Remove unnecessary $state wrapping
- desc: Remove the unnecessary `$state` and update the value via mutations (e.g., .add(), .delete(), .push(), .splice()).
messageId: suggestRemoveStateWrap
output: |
<script>
Expand All @@ -13,11 +13,11 @@
const set = new SvelteSet();
let map = $state(new SvelteMap());
</script>
- message: SvelteMap is already reactive, $state wrapping is unnecessary.
- message: SvelteMap is already reactive. `$state` wrapping is unnecessary. Update it with mutations (e.g., .add(), .delete(), .push(), .splice()).
line: 7
column: 19
suggestions:
- desc: Remove unnecessary $state wrapping
- desc: Remove the unnecessary `$state` and update the value via mutations (e.g., .add(), .delete(), .push(), .splice()).
messageId: suggestRemoveStateWrap
output: |
<script>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
- message: SvelteSet is already reactive, $state wrapping is unnecessary.
- message: SvelteSet is already reactive. `$state` wrapping is unnecessary. Update it with mutations (e.g., .add(), .delete(), .push(), .splice()).
line: 12
column: 21
suggestions:
- desc: Remove unnecessary $state wrapping
- desc: Remove the unnecessary `$state` and update the value via mutations (e.g., .add(), .delete(), .push(), .splice()).
messageId: suggestRemoveStateWrap
output: |
<script>
Expand All @@ -27,11 +27,11 @@
const regularState = $state(42);
const stateObject = $state({ foo: 'bar' });
</script>
- message: SvelteMap is already reactive, $state wrapping is unnecessary.
- message: SvelteMap is already reactive. `$state` wrapping is unnecessary. Update it with mutations (e.g., .add(), .delete(), .push(), .splice()).
line: 13
column: 21
suggestions:
- desc: Remove unnecessary $state wrapping
- desc: Remove the unnecessary `$state` and update the value via mutations (e.g., .add(), .delete(), .push(), .splice()).
messageId: suggestRemoveStateWrap
output: |
<script>
Expand All @@ -56,11 +56,11 @@
const regularState = $state(42);
const stateObject = $state({ foo: 'bar' });
</script>
- message: SvelteURL is already reactive, $state wrapping is unnecessary.
- message: SvelteURL is already reactive. `$state` wrapping is unnecessary. Update it with mutations (e.g., .add(), .delete(), .push(), .splice()).
line: 14
column: 21
suggestions:
- desc: Remove unnecessary $state wrapping
- desc: Remove the unnecessary `$state` and update the value via mutations (e.g., .add(), .delete(), .push(), .splice()).
messageId: suggestRemoveStateWrap
output: |
<script>
Expand All @@ -85,11 +85,11 @@
const regularState = $state(42);
const stateObject = $state({ foo: 'bar' });
</script>
- message: SvelteURLSearchParams is already reactive, $state wrapping is unnecessary.
- message: SvelteURLSearchParams is already reactive. `$state` wrapping is unnecessary. Update it with mutations (e.g., .add(), .delete(), .push(), .splice()).
line: 15
column: 24
suggestions:
- desc: Remove unnecessary $state wrapping
- desc: Remove the unnecessary `$state` and update the value via mutations (e.g., .add(), .delete(), .push(), .splice()).
messageId: suggestRemoveStateWrap
output: |
<script>
Expand All @@ -114,11 +114,11 @@
const regularState = $state(42);
const stateObject = $state({ foo: 'bar' });
</script>
- message: SvelteDate is already reactive, $state wrapping is unnecessary.
- message: SvelteDate is already reactive. `$state` wrapping is unnecessary. Update it with mutations (e.g., .add(), .delete(), .push(), .splice()).
line: 16
column: 22
suggestions:
- desc: Remove unnecessary $state wrapping
- desc: Remove the unnecessary `$state` and update the value via mutations (e.g., .add(), .delete(), .push(), .splice()).
messageId: suggestRemoveStateWrap
output: |
<script>
Expand All @@ -143,11 +143,11 @@
const regularState = $state(42);
const stateObject = $state({ foo: 'bar' });
</script>
- message: MediaQuery is already reactive, $state wrapping is unnecessary.
- message: MediaQuery is already reactive. `$state` wrapping is unnecessary. Update it with mutations (e.g., .add(), .delete(), .push(), .splice()).
line: 17
column: 28
suggestions:
- desc: Remove unnecessary $state wrapping
- desc: Remove the unnecessary `$state` and update the value via mutations (e.g., .add(), .delete(), .push(), .splice()).
messageId: suggestRemoveStateWrap
output: |
<script>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
- message: SvelteSet is already reactive, $state wrapping is unnecessary.
- message: SvelteSet is already reactive. `$state` wrapping is unnecessary. Update it with mutations (e.g., .add(), .delete(), .push(), .splice()).
line: 5
column: 21
suggestions:
- desc: Remove unnecessary $state wrapping
- desc: Remove the unnecessary `$state` and update the value via mutations (e.g., .add(), .delete(), .push(), .splice()).
messageId: suggestRemoveStateWrap
output: >
<script>
Expand All @@ -12,11 +12,11 @@
const set = new CustomSet();
const map = $state(new CustomMap());
</script>
- message: SvelteMap is already reactive, $state wrapping is unnecessary.
- message: SvelteMap is already reactive. `$state` wrapping is unnecessary. Update it with mutations (e.g., .add(), .delete(), .push(), .splice()).
line: 6
column: 21
suggestions:
- desc: Remove unnecessary $state wrapping
- desc: Remove the unnecessary `$state` and update the value via mutations (e.g., .add(), .delete(), .push(), .splice()).
messageId: suggestRemoveStateWrap
output: >
<script>
Expand Down
Loading