-
-
Notifications
You must be signed in to change notification settings - Fork 48
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 svelte/no-immutable-reactive-statements
rule
#439
Conversation
if ( | ||
pp && | ||
pp.type === "ExportNamedDeclaration" && | ||
pp.declaration === parent | ||
) { | ||
// Props | ||
return true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if prop is const
, class
or function
, it should return false
I think.
https://svelte.dev/docs#component-format-script-1-export-creates-a-component-prop
If you export a const, class or function, it is readonly from outside the component. Functions are valid prop values, however, as shown below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your comment!
if prop is const, class or function, it should return false I think.
Effectively they are already checked. But I think I should add test cases 👍. I will add it.
if (def.type === "Variable") {
in L66 enters a branch only for var
, let
, or const
declarations.
if (parent.kind === "const") {
in L68 enters a branch only for const
declarations. and it returns false
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The below code throws an ESLint error, but I think it shouldn't right?
<script>
/* eslint svelte/no-immutable-reactive-statements: "error" */
export const bar = "readonly";
$: console.log(bar)
</script>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the statement is only processed the first time, so I think the correct behavior to be reported by the rule.
Do you think I'm misunderstanding how svelte works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sorry. I was wrong.
Yes. this is correct.🙏
This PR adds
svelte/no-immutable-reactive-statements
rule that reports if all variables referenced in reactive statements are immutable.close #382