Skip to content

Commit bc23974

Browse files
tivactivac
authored
add no-reactive-functions rule (#206)
* feat: add no-reactive-functions rule * wip: feedback Added a description, and now handling the possibility of no space after the reactive statement label. * chore: yarn updated content
1 parent 1ce4e94 commit bc23974

File tree

9 files changed

+187
-0
lines changed

9 files changed

+187
-0
lines changed

‎README.md

+1
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,7 @@ These rules relate to better ways of doing things to help you avoid problems:
282282
|:--------|:------------|:---|
283283
| [svelte/button-has-type](https://ota-meshi.github.io/eslint-plugin-svelte/rules/button-has-type/) | disallow usage of button without an explicit type attribute | |
284284
| [svelte/no-at-debug-tags](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-at-debug-tags/) | disallow the use of `{@debug}` | :star: |
285+
| [svelte/no-reactive-functions](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-reactive-functions/) | It's not necessary to define functions in reactive statements | :bulb: |
285286
| [svelte/no-reactive-literals](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-reactive-literals/) | Don't assign literal values in reactive statements | :bulb: |
286287
| [svelte/no-unused-svelte-ignore](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: |
287288
| [svelte/no-useless-mustaches](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-useless-mustaches/) | disallow unnecessary mustache interpolations | :wrench: |

‎docs/rules.md

+1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ These rules relate to better ways of doing things to help you avoid problems:
4242
|:--------|:------------|:---|
4343
| [svelte/button-has-type](./rules/button-has-type.md) | disallow usage of button without an explicit type attribute | |
4444
| [svelte/no-at-debug-tags](./rules/no-at-debug-tags.md) | disallow the use of `{@debug}` | :star: |
45+
| [svelte/no-reactive-functions](./rules/no-reactive-functions.md) | It's not necessary to define functions in reactive statements | :bulb: |
4546
| [svelte/no-reactive-literals](./rules/no-reactive-literals.md) | Don't assign literal values in reactive statements | :bulb: |
4647
| [svelte/no-unused-svelte-ignore](./rules/no-unused-svelte-ignore.md) | disallow unused svelte-ignore comments | :star: |
4748
| [svelte/no-useless-mustaches](./rules/no-useless-mustaches.md) | disallow unnecessary mustache interpolations | :wrench: |

‎docs/rules/no-reactive-functions.md

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
---
2+
pageClass: "rule-details"
3+
sidebarDepth: 0
4+
title: "svelte/no-reactive-functions"
5+
description: "It's not necessary to define functions in reactive statements"
6+
---
7+
8+
# svelte/no-reactive-functions
9+
10+
> It's not necessary to define functions in reactive statements
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+
- :bulb: Some problems reported by this rule are manually fixable by editor [suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).
14+
15+
## :book: Rule Details
16+
17+
This rule reports whenever a function is defined in a reactive statement. This isn't necessary, as each time the function is executed it'll already have access to the latest values necessary. Redefining the function in the reactive statement is just a waste of CPU cycles.
18+
19+
<ESLintCodeBlock>
20+
21+
<!--eslint-skip-->
22+
23+
```svelte
24+
<script>
25+
/* eslint svelte/no-reactive-functions: "error" */
26+
27+
/* ✓ GOOD */
28+
const arrowFn = () => { /* ... */ }
29+
const func = function() { /* ... */ }
30+
31+
/* ✗ BAD */
32+
$: arrowFn = () => { /* ... */ }
33+
$: func = function() { /* ... */ }
34+
</script>
35+
```
36+
37+
</ESLintCodeBlock>
38+
39+
## :wrench: Options
40+
41+
Nothing
42+
43+
## :heart: Compatibility
44+
45+
This rule was taken from [@tivac/eslint-plugin-svelte].
46+
This rule is compatible with `@tivac/svelte/reactive-functions` rule.
47+
48+
[@tivac/eslint-plugin-svelte]: https://github.com/tivac/eslint-plugin-svelte/
49+
## :mag: Implementation
50+
51+
- [Rule source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/src/rules/no-reactive-functions.ts)
52+
- [Test source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/tests/src/rules/no-reactive-functions.ts)

‎src/rules/no-reactive-functions.ts

+66
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import type { TSESTree } from "@typescript-eslint/types"
2+
import type { AST } from "svelte-eslint-parser"
3+
import { createRule } from "../utils"
4+
5+
export default createRule("no-reactive-functions", {
6+
meta: {
7+
docs: {
8+
description:
9+
"It's not necessary to define functions in reactive statements",
10+
category: "Best Practices",
11+
recommended: false,
12+
},
13+
hasSuggestions: true,
14+
schema: [],
15+
messages: {
16+
noReactiveFns: `Do not create functions inside reactive statements unless absolutely necessary.`,
17+
fixReactiveFns: `Move the function out of the reactive statement`,
18+
},
19+
type: "suggestion", // "problem", or "layout",
20+
},
21+
create(context) {
22+
return {
23+
// $: foo = () => { ... }
24+
[`SvelteReactiveStatement > ExpressionStatement > AssignmentExpression > :function`](
25+
node: TSESTree.ArrowFunctionExpression,
26+
) {
27+
// Move upwards to include the entire label
28+
const parent = node.parent?.parent?.parent
29+
30+
if (!parent) {
31+
return false
32+
}
33+
34+
const source = context.getSourceCode()
35+
36+
return context.report({
37+
node: parent,
38+
loc: parent.loc,
39+
messageId: "noReactiveFns",
40+
suggest: [
41+
{
42+
messageId: "fixReactiveFns",
43+
fix(fixer) {
44+
const tokens = source.getFirstTokens(parent, {
45+
includeComments: false,
46+
count: 3,
47+
})
48+
49+
const noExtraSpace = source.isSpaceBetweenTokens(
50+
tokens[1] as AST.Token,
51+
tokens[2] as AST.Token,
52+
)
53+
54+
// Replace the entire reactive label with "const"
55+
return fixer.replaceTextRange(
56+
[tokens[0].range[0], tokens[1].range[1]],
57+
noExtraSpace ? "const" : "const ",
58+
)
59+
},
60+
},
61+
],
62+
})
63+
},
64+
}
65+
},
66+
})

‎src/utils/rules.ts

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import noExtraReactiveCurlies from "../rules/no-extra-reactive-curlies"
1616
import noInnerDeclarations from "../rules/no-inner-declarations"
1717
import noNotFunctionHandler from "../rules/no-not-function-handler"
1818
import noObjectInTextMustaches from "../rules/no-object-in-text-mustaches"
19+
import noReactiveFunctions from "../rules/no-reactive-functions"
1920
import noReactiveLiterals from "../rules/no-reactive-literals"
2021
import noShorthandStylePropertyOverrides from "../rules/no-shorthand-style-property-overrides"
2122
import noSpacesAroundEqualSignsInAttribute from "../rules/no-spaces-around-equal-signs-in-attribute"
@@ -51,6 +52,7 @@ export const rules = [
5152
noInnerDeclarations,
5253
noNotFunctionHandler,
5354
noObjectInTextMustaches,
55+
noReactiveFunctions,
5456
noReactiveLiterals,
5557
noShorthandStylePropertyOverrides,
5658
noSpacesAroundEqualSignsInAttribute,

‎tests/fixtures/rules/no-reactive-functions/invalid/test01-errors.json

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
[
2+
{
3+
"message": "Do not create functions inside reactive statements unless absolutely necessary.",
4+
"line": 3,
5+
"column": 5,
6+
"suggestions": [
7+
{
8+
"desc": "Move the function out of the reactive statement",
9+
"messageId": "fixReactiveFns",
10+
"output": "<!-- prettier-ignore -->\n<script>\n const arrow = () => {}\n $: fn = function() {}\n $:nospace = () => {}\n</script>\n"
11+
}
12+
]
13+
},
14+
{
15+
"message": "Do not create functions inside reactive statements unless absolutely necessary.",
16+
"line": 4,
17+
"column": 5,
18+
"suggestions": [
19+
{
20+
"desc": "Move the function out of the reactive statement",
21+
"messageId": "fixReactiveFns",
22+
"output": "<!-- prettier-ignore -->\n<script>\n $: arrow = () => {}\n const fn = function() {}\n $:nospace = () => {}\n</script>\n"
23+
}
24+
]
25+
},
26+
{
27+
"message": "Do not create functions inside reactive statements unless absolutely necessary.",
28+
"line": 5,
29+
"column": 5,
30+
"suggestions": [
31+
{
32+
"desc": "Move the function out of the reactive statement",
33+
"messageId": "fixReactiveFns",
34+
"output": "<!-- prettier-ignore -->\n<script>\n $: arrow = () => {}\n $: fn = function() {}\n const nospace = () => {}\n</script>\n"
35+
}
36+
]
37+
}
38+
]

‎tests/fixtures/rules/no-reactive-functions/invalid/test01-input.svelte

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<!-- prettier-ignore -->
2+
<script>
3+
$: arrow = () => {}
4+
$: fn = function() {}
5+
$:nospace = () => {}
6+
</script>

‎tests/fixtures/rules/no-reactive-functions/valid/test01-input.svelte

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<!-- prettier-ignore -->
2+
<script>
3+
const arrow = () => {}
4+
const fn = function() { }
5+
</script>

‎tests/src/rules/no-reactive-functions.ts

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { RuleTester } from "eslint"
2+
import rule from "../../../src/rules/no-reactive-functions"
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-reactive-functions",
14+
rule as any,
15+
loadTestCases("no-reactive-functions"),
16+
)

0 commit comments

Comments
 (0)