Skip to content

Commit 024d4bd

Browse files
ota-meshiota-meshi
committed
feat: add svelte/no-loss-of-prop-reactivity rule
1 parent afc47e4 commit 024d4bd

File tree

10 files changed

+260
-0
lines changed

10 files changed

+260
-0
lines changed

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,7 @@ These rules relate to better ways of doing things to help you avoid problems:
341341
| [svelte/button-has-type](https://sveltejs.github.io/eslint-plugin-svelte/rules/button-has-type/) | disallow usage of button without an explicit type attribute | |
342342
| [svelte/no-at-debug-tags](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-at-debug-tags/) | disallow the use of `{@debug}` | :star: |
343343
| [svelte/no-immutable-reactive-statements](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-immutable-reactive-statements/) | disallow reactive statements that don't reference reactive values. | |
344+
| [svelte/no-loss-of-prop-reactivity](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-loss-of-prop-reactivity/) | disallow the use of props that potentially lose reactivity | |
344345
| [svelte/no-reactive-functions](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-functions/) | it's not necessary to define functions in reactive statements | :bulb: |
345346
| [svelte/no-reactive-literals](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-literals/) | don't assign literal values in reactive statements | :bulb: |
346347
| [svelte/no-unused-svelte-ignore](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: |

‎docs/rules.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ These rules relate to better ways of doing things to help you avoid problems:
5454
| [svelte/button-has-type](./rules/button-has-type.md) | disallow usage of button without an explicit type attribute | |
5555
| [svelte/no-at-debug-tags](./rules/no-at-debug-tags.md) | disallow the use of `{@debug}` | :star: |
5656
| [svelte/no-immutable-reactive-statements](./rules/no-immutable-reactive-statements.md) | disallow reactive statements that don't reference reactive values. | |
57+
| [svelte/no-loss-of-prop-reactivity](./rules/no-loss-of-prop-reactivity.md) | disallow the use of props that potentially lose reactivity | |
5758
| [svelte/no-reactive-functions](./rules/no-reactive-functions.md) | it's not necessary to define functions in reactive statements | :bulb: |
5859
| [svelte/no-reactive-literals](./rules/no-reactive-literals.md) | don't assign literal values in reactive statements | :bulb: |
5960
| [svelte/no-unused-svelte-ignore](./rules/no-unused-svelte-ignore.md) | disallow unused svelte-ignore comments | :star: |

‎docs/rules/no-loss-of-prop-reactivity.md‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
pageClass: "rule-details"
3+
sidebarDepth: 0
4+
title: "svelte/no-loss-of-prop-reactivity"
5+
description: "disallow the use of props that potentially lose reactivity"
6+
---
7+
8+
# svelte/no-loss-of-prop-reactivity
9+
10+
> disallow the use of props that potentially lose reactivity
11+
12+
- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>
13+
14+
## :book: Rule Details
15+
16+
This rule reports the use of props that potentially lose reactivity.
17+
18+
<ESLintCodeBlock>
19+
20+
<!--eslint-skip-->
21+
22+
```svelte
23+
<script>
24+
/* eslint svelte/no-loss-of-prop-reactivity: "error" */
25+
export let prop = 42
26+
/* ✓ GOOD */
27+
$: double1 = prop * 2
28+
/* ✗ BAD */
29+
let double1 = prop * 2
30+
</script>
31+
```
32+
33+
</ESLintCodeBlock>
34+
35+
## :wrench: Options
36+
37+
Nothing.
38+
39+
## :mag: Implementation
40+
41+
- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/src/rules/no-loss-of-prop-reactivity.ts)
42+
- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/tests/src/rules/no-loss-of-prop-reactivity.ts)

‎src/rules/no-loss-of-prop-reactivity.ts‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
import type { AST } from "svelte-eslint-parser"
2+
import { createRule } from "../utils"
3+
import type { TSESTree } from "@typescript-eslint/types"
4+
import { findAttribute, findVariable } from "../utils/ast-utils"
5+
import { getStaticAttributeValue } from "../utils/ast-utils"
6+
7+
export default createRule("no-loss-of-prop-reactivity", {
8+
meta: {
9+
docs: {
10+
description: "disallow the use of props that potentially lose reactivity",
11+
category: "Best Practices",
12+
recommended: false,
13+
},
14+
schema: [],
15+
messages: {
16+
potentiallyLoseReactivity:
17+
"Referencing props that potentially lose reactivity should be avoided.",
18+
},
19+
type: "suggestion",
20+
},
21+
create(context) {
22+
if (!context.parserServices.isSvelte) {
23+
return {}
24+
}
25+
let contextModule: AST.SvelteScriptElement | null = null
26+
let scriptNode: AST.SvelteScriptElement | null = null
27+
const reactiveStatements: AST.SvelteReactiveStatement[] = []
28+
const functions: TSESTree.FunctionLike[] = []
29+
const props = new Set<TSESTree.Identifier>()
30+
return {
31+
SvelteScriptElement(node: AST.SvelteScriptElement) {
32+
const contextAttr = findAttribute(node, "context")
33+
if (contextAttr && getStaticAttributeValue(contextAttr) === "module") {
34+
contextModule = node
35+
return
36+
}
37+
38+
scriptNode = node
39+
},
40+
SvelteReactiveStatement(node: AST.SvelteReactiveStatement) {
41+
reactiveStatements.push(node)
42+
},
43+
":function"(node: TSESTree.FunctionLike) {
44+
functions.push(node)
45+
},
46+
ExportNamedDeclaration(node: TSESTree.ExportNamedDeclaration) {
47+
if (
48+
contextModule &&
49+
contextModule.range[0] < node.range[0] &&
50+
node.range[1] < contextModule.range[1]
51+
) {
52+
return
53+
}
54+
if (
55+
node.declaration?.type === "VariableDeclaration" &&
56+
(node.declaration.kind === "let" || node.declaration.kind === "var")
57+
) {
58+
for (const decl of node.declaration.declarations) {
59+
if (decl.id.type === "Identifier") {
60+
props.add(decl.id)
61+
}
62+
}
63+
}
64+
for (const spec of node.specifiers) {
65+
if (spec.exportKind === "type") {
66+
continue
67+
}
68+
if (isMutableProp(spec.local)) {
69+
props.add(spec.exported)
70+
}
71+
}
72+
},
73+
"Program:exit"() {
74+
for (const prop of props) {
75+
const variable = findVariable(context, prop)
76+
if (
77+
!variable ||
78+
// ignore multiple definitions
79+
variable.defs.length > 1
80+
) {
81+
return
82+
}
83+
for (const reference of variable.references) {
84+
if (reference.isWrite()) continue
85+
const id = reference.identifier as TSESTree.Identifier
86+
if (
87+
variable.defs.some(
88+
(def) =>
89+
def.node.range[0] <= id.range[0] &&
90+
id.range[1] <= def.node.range[1],
91+
)
92+
) {
93+
// The reference is in the variable definition.
94+
continue
95+
}
96+
if (isInReactivityScope(id)) continue
97+
98+
context.report({
99+
node: id,
100+
messageId: "potentiallyLoseReactivity",
101+
})
102+
}
103+
}
104+
},
105+
}
106+
107+
/** Checks whether given prop id is mutable variable or not */
108+
function isMutableProp(id: TSESTree.Identifier) {
109+
const variable = findVariable(context, id)
110+
if (!variable || variable.defs.length === 0) {
111+
return false
112+
}
113+
return variable.defs.every((def) => {
114+
if (def.type !== "Variable") {
115+
return false
116+
}
117+
return def.parent.kind === "let" || def.parent.kind === "var"
118+
})
119+
}
120+
121+
/** Checks whether given id is in potentially reactive scope or not */
122+
function isInReactivityScope(id: TSESTree.Identifier) {
123+
if (
124+
!scriptNode ||
125+
id.range[1] <= scriptNode.range[0] ||
126+
scriptNode.range[1] <= id.range[0]
127+
) {
128+
// The reference is in the template.
129+
return true
130+
}
131+
if (
132+
reactiveStatements.some(
133+
(node) =>
134+
node.range[0] <= id.range[0] && id.range[1] <= node.range[1],
135+
)
136+
) {
137+
// The reference is in the reactive statement.
138+
return true
139+
}
140+
if (
141+
functions.some(
142+
(node) =>
143+
node.range[0] <= id.range[0] && id.range[1] <= node.range[1],
144+
)
145+
) {
146+
// The reference is in the function.
147+
return true
148+
}
149+
return false
150+
}
151+
},
152+
})

‎src/utils/rules.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import noExportLoadInSvelteModuleInKitPages from "../rules/no-export-load-in-sve
2929
import noExtraReactiveCurlies from "../rules/no-extra-reactive-curlies"
3030
import noImmutableReactiveStatements from "../rules/no-immutable-reactive-statements"
3131
import noInnerDeclarations from "../rules/no-inner-declarations"
32+
import noLossOfPropReactivity from "../rules/no-loss-of-prop-reactivity"
3233
import noNotFunctionHandler from "../rules/no-not-function-handler"
3334
import noObjectInTextMustaches from "../rules/no-object-in-text-mustaches"
3435
import noReactiveFunctions from "../rules/no-reactive-functions"
@@ -88,6 +89,7 @@ export const rules = [
8889
noExtraReactiveCurlies,
8990
noImmutableReactiveStatements,
9091
noInnerDeclarations,
92+
noLossOfPropReactivity,
9193
noNotFunctionHandler,
9294
noObjectInTextMustaches,
9395
noReactiveFunctions,

‎tests/fixtures/rules/no-loss-of-prop-reactivity/invalid/prop01-errors.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
- message: Referencing props that potentially lose reactivity should be avoided.
2+
line: 4
3+
column: 17
4+
suggestions: null

‎tests/fixtures/rules/no-loss-of-prop-reactivity/invalid/prop01-input.svelte‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<script>
2+
export let prop = 42
3+
/* ✗ BAD */
4+
let double1 = prop * 2
5+
</script>

‎tests/fixtures/rules/no-loss-of-prop-reactivity/valid/keypad01-input.svelte‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<script>
2+
import { createEventDispatcher } from "svelte"
3+
4+
export let value = ""
5+
6+
const dispatch = createEventDispatcher()
7+
8+
const select = (num) => () => (value += num)
9+
const clear = () => (value = "")
10+
const submit = () => dispatch("submit")
11+
</script>
12+
13+
<div class="keypad">
14+
<button on:click={select(1)}>1</button>
15+
16+
<button disabled={!value} on:click={clear}>clear</button>
17+
<button on:click={select(0)}>0</button>
18+
<button disabled={!value} on:click={submit}>submit</button>
19+
</div>
20+
21+
<style>
22+
.keypad {
23+
display: grid;
24+
grid-template-columns: repeat(3, 5em);
25+
grid-template-rows: repeat(4, 3em);
26+
grid-gap: 0.5em;
27+
}
28+
29+
button {
30+
margin: 0;
31+
}
32+
</style>

‎tests/fixtures/rules/no-loss-of-prop-reactivity/valid/prop01-input.svelte‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<script>
2+
export let prop = 42
3+
/* ✓ GOOD */
4+
$: double1 = prop * 2
5+
</script>

‎tests/src/rules/no-loss-of-prop-reactivity.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { RuleTester } from "eslint"
2+
import rule from "../../../src/rules/no-loss-of-prop-reactivity"
3+
import { loadTestCases } from "../../utils/utils"
4+
5+
const tester = new RuleTester({
6+
parserOptions: {
7+
ecmaVersion: 2020,
8+
sourceType: "module",
9+
},
10+
})
11+
12+
tester.run(
13+
"no-loss-of-prop-reactivity",
14+
rule as any,
15+
loadTestCases("no-loss-of-prop-reactivity"),
16+
)

0 commit comments

Comments
 (0)