Skip to content
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

disallow $state(new SvelteSet) and similar #1052

Open
mustafa0x opened this issue Feb 3, 2025 · 4 comments · May be fixed by #1062
Open

disallow $state(new SvelteSet) and similar #1052

mustafa0x opened this issue Feb 3, 2025 · 4 comments · May be fixed by #1062
Labels
enhancement New feature or request new rule

Comments

@mustafa0x
Copy link

Motivation

SvelteMap, SvelteSet, and SvelteURL don't need to be wrapped in $state. but that's not always obvious

Description

per above

Examples

<!-- ✓ GOOD -->
<script>
const myset = new SvelteSet
</script>

<!-- ✗ BAD -->
<script>
const myset = $state(new SvelteSet)
</script>

Additional comments

No response

@mustafa0x mustafa0x added enhancement New feature or request new rule labels Feb 3, 2025
@marekdedic
Copy link
Contributor

marekdedic commented Feb 3, 2025

Hmm, reacting quickly and not thoroughly enough, but this needs a little bit more nuance:

let myOptionalSet = $state(new SvelteSet());

// Somewhere further down
myOptionalSet = null; // This needs the variable to be wrapped in a $state in order for this to be reactive.

See sveltejs/svelte#14799 for more details.

@mikededo
Copy link
Contributor

mikededo commented Feb 4, 2025

I agree with @marekdedic. The statement that reactive built-ins do not need to be wrapped with $state is not correct.

@marekdedic
Copy link
Contributor

Yes, at the same time, it actually is a mistake like 95% of the time... So I think it's a great candidate for a lint rule, the rule just needs to be clever and actually figure out when the $state is really not needed.

@mustafa0x
Copy link
Author

$state(new Svelte matches 90 files. Most don't seem to reassign. So yes, if the rule was smart, that'd be best.

@baseballyama baseballyama linked a pull request Feb 9, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request new rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants