feat: add no-unnecessary-state-wrap rule

[baseballyama]

close: #1052

[changeset-bot]

🦋 Changeset detected

Latest commit: fe121fd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
eslint-plugin-svelte	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

Try the Instant Preview in Online Playground

ESLint Online Playground

Install the Instant Preview to Your Local

npm i https://pkg.pr.new/eslint-plugin-svelte@1062

Published Instant Preview Packages:

• eslint-plugin-svelte: https://pkg.pr.new/eslint-plugin-svelte@1062

View Commit

[marekdedic]

Hi,
sorry if this is unfinished and you're planning to add this, but atm, this ignores #1052 (comment)

[baseballyama]

Since the clear function can be used, $state is usually unnecessary. If there is a reason to use $state(new SvelteMap()), this rule can simply be disabled. ESLint is fully configurable.

[marekdedic]

I still have to disagree - the rule now has a clear false positive that can be detected by its implementation, but just isn't.
And I'm sorry, but yeah, you can disable any rule for any line, but if the FP can be detected by the rule, it should be. Only when it's not possible (like what we're discussing in #910 ) should users be forced to manually disable rules...

[baseballyama]

Can you provide an example of fp?

[marekdedic]

Of course, see #1052 (comment) for a simple example or here for a real-world example (this one is in $derived, but the same semantics apply)

[baseballyama]

Then, let’s add an option to not report an error when a reassignment exists.

[marekdedic]

Thanks :) I don't even think it needs to be an option - if a $state(SvelteMap) is reassigned, the $state is always necessary...

[baseballyama]

I don’t think so. I believe it might simply be a mistake. At least in my project, I want to consistently treat it as an error. For example, in the following case, it can clearly use “clear”.

  let map = $state(new SvelteMap());
  map = new SvelteMap();

[marekdedic]

Hmm, you are absolutely right that the code snippet is incorrect, thanks, I din't realize that!

I am thinking if this isn't a separate issue, though - the anti-pattern here is IMO assigning a new SvelteMap() to an object which is a SvelteMap instead of using .clear(). And that would also be an issue with the regular Map, wouldn't it? So maybe you found a more general issue here that assigning new X() to a variable of type X is incorrect and can be replaced with X.clear() for X being any of SvelteMap, SvelteSet, Map, Set (I checked MDN and that's all I could find)

Now if that were a separate rule it would nicely dovetail with this one even if this rule ignored all re-assigned variables, the other rule would replace the re-assignment with .clear() and this rule would then remove the $state.

I feel I am rambling a little bit - do I make any sense to you? :D

[baseballyama]

In any case, for those who don’t want this rule enabled, they can simply disable it. In most cases, $state(new SvelteMap()) doesn’t really make sense, so I think having it enabled by default isn’t a bad idea.

[baseballyama]

I added allowReassign option.

PR Overview

This PR introduces a new ESLint rule, svelte/no-unnecessary-state-wrap, to disallow redundant $state wrapping for reactive classes in Svelte. Key changes include:

• Implementation of the new rule in the plugin with appropriate configurations.
• Comprehensive test fixtures for basic errors, additional custom classes, reassign scenarios, and import alias handling.
• Updates to documentation, configuration files, and the rule types file to integrate the new rule.

Reviewed Changes

File	Description
docs/rules/no-unnecessary-state-wrap.md	Adds documentation for the new rule.
packages/eslint-plugin-svelte/src/rules/no-unnecessary-state-wrap.ts	Implements the rule logic with suggestions for removal of unnecessary wrapping.
packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/*	Provides test fixtures covering various error scenarios.
.changeset/popular-needles-remain.md	Updates the changeset with the feature.
packages/eslint-plugin-svelte/src/rule-types.ts	Extends the rule types for the new rule.
packages/eslint-plugin-svelte/src/configs/flat/recommended.ts, README.md, docs/rules.md, packages/eslint-plugin-svelte/src/utils/rules.ts	Updates configuration and documentation to include the new rule.

Copilot reviewed 26 out of 26 changed files in this pull request and generated no comments.

[ota-meshi]

LGTM! Thanks!