-
-
Notifications
You must be signed in to change notification settings - Fork 45
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
feat: add excludedRunes
option to the prefer-const
rule
#1064
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: e4f255e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Try the Instant Preview in Online PlaygroundInstall the Instant Preview to Your Local
Published Instant Preview Packages:
|
15e0038
to
c3d5bd7
Compare
I still don't understand why you want to ignore I know that some users want to use |
We might want to enable In other words, semantically, since (Personally, I always use const consistently, though.😅) |
In this case, there is no actual reassignment, so export const Time3 = () => {
// If `$state` has object value,
// Time3 user cn reassign `value.value` even without reassign logic in this composable.
// Therefore it can be declare as both `const` / `let`
const value = $state( { value: new Date().toString() });
return {
get value() {
return value;
}
}
} |
I understand that when a user uses |
Therefore, I am considering creating a separate rule, |
@ota-meshi So, what you’re saying is that extending |
No, I think Since we don't ignore If a primitive value of |
I still don’t fully understand this part. Since |
If in that example, |
The area I have an issue with is the consistency of the rule. |
I fully understand your perspective.👍 As stated in the documentation, Svelte runes recommend using In your example, since user does not use runes, I understood that standard JavaScript rules apply (without being overridden by Svelte rules). Therefore, I believed that the behavior of this PR was consistent. Additionally, based on your reasoning, shouldn’t |
I think the reason why
Hmm... I don't think it explicitly says "recommended". I think it's still a matter of user preference 🤔 |
I see. Would adding a config be a good middle ground? Like |
If there are users requesting it, I think it makes sense to add the option. |
Got it. In that case, since there are no breaking changes, I will try implementing this option after the v3 release. |
$state
in the prefer-const
ruleexcludedRunes
option to the prefer-const
rule
I added |
close part of #818
As explained here,
$state
also needs to be excluded from theprefer-const
rule.#985 (comment)