feat(no-unnecessary-state-wrap): support string array in allowReassign, default to true and improve messages

[baseballyama]

close: #1154
close: #1180

[changeset-bot]

⚠️ No Changeset found

Latest commit: 9070e12

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a 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@9070e12

Published Instant Preview Packages:

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

View Commit

[benmccann]

I think set = new SvelteSet([1, 2, 3]) should be allowed as it's cumbersome to write another way, but set = new SvelteSet() should not be because set.clear(); is easier to write

[baseballyama]

I think set = new SvelteSet([1, 2, 3]) should be allowed as it's cumbersome to write another way, but set = new SvelteSet() should not be because set.clear(); is easier to write

Since the rule would become more complex, would it be better to implement it as a separate rule, like prefer-svelteset-new?

[marekdedic]

Why should SvelteSet be allowed by default? I don't see why it's different from e.g. SvelteMap, am I missing something?

[benmccann]

Since the rule would become more complex, would it be better to implement it as a separate rule, like prefer-svelteset-new?

I think that would be fine. The name seems backwards to me though. Perhaps something like prefer-clear-over-reassignment. I also think you don't necessarily need svelteset in the name because it could apply to SvelteMap as well

Why should SvelteSet be allowed by default? I don't see why it's different from e.g. SvelteMap, am I missing something?

I agree. I think we ended up deciding all reassignments should be allowed by default though, so probably not an issue once that change is made. The prefer-clear-over-reassignment rule can handle the SvelteSet and SvelteMap cases

[marekdedic]

Thanks 🙏

I'm a bit lost and want to understand this: Am I correct that what this PR essentially does is by default turn off the rule for any SvelteSet or SvelteMap that is re-assigned anywhere?

And then the suggestion is to add a new rule that basically reverts this change?

[baseballyama]

I'm a bit lost and want to understand this: Am I correct that what this PR essentially does is by default turn off the rule for any SvelteSet or SvelteMap that is re-assigned anywhere?

The purpose of this PR is to change the default value of allowReassign to true, and to exclude any reassigned reactive classes including SvelteSet and SvelteMap from being reported.

[baseballyama]

I’ve updated the PR title, and I’ll update the PR this weekend.