Skip to content

Commit 2fd72ee

Browse files
committed
wip: feedback
1 parent 48187dd commit 2fd72ee

File tree

5 files changed

+86
-39
lines changed

5 files changed

+86
-39
lines changed

src/rules/prefer-destructured-store-props.ts

+49-33
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { TSESTree } from "@typescript-eslint/types"
33
import type { AST } from "svelte-eslint-parser"
44
import { createRule } from "../utils"
55
import { getStringIfConstant } from "../utils/ast-utils"
6+
import { returnStatement } from "@babel/types"
67

78
type NodeRecord = { property: string; node: TSESTree.MemberExpression }
89

@@ -17,20 +18,33 @@ export default createRule("prefer-destructured-store-props", {
1718
hasSuggestions: true,
1819
schema: [],
1920
messages: {
20-
useDestructuring: `Destructure {{property}} from store {{store}} for better change tracking & fewer redraws`,
21+
useDestructuring: `Destructure {{property}} from {{store}} for better change tracking & fewer redraws`,
2122
fixUseDestructuring: `Using destructuring like $: ({ {{property}} } = {{store}}); will run faster`,
2223
},
2324
type: "suggestion",
2425
},
2526
create(context) {
2627
let script: AST.SvelteScriptElement
28+
29+
// Store off instances of probably-destructurable statements
2730
const reports: NodeRecord[] = []
2831

32+
// Store off defined variable names so we can avoid offering a suggestion in those cases
33+
const defined: Set<string> = new Set()
34+
2935
return {
3036
[`SvelteScriptElement`](node: AST.SvelteScriptElement) {
3137
script = node
3238
},
3339

40+
[`VariableDeclarator[id.type="Identifier"]`](
41+
node: TSESTree.VariableDeclarator,
42+
) {
43+
const { name } = node.id as TSESTree.Identifier
44+
45+
defined.add(name)
46+
},
47+
3448
// {$foo.bar}
3549
// should be
3650
// $: ({ bar } = $foo);
@@ -54,13 +68,6 @@ export default createRule("prefer-destructured-store-props", {
5468
[`Program:exit`]() {
5569
reports.forEach(({ property, node }) => {
5670
const store = (node.object as TSESTree.Identifier).name
57-
// let prop: string | null = null
58-
59-
// if (node.property.type === "Literal") {
60-
// prop = node.property.value as string
61-
// } else if (node.property.type === "Identifier") {
62-
// prop = node.property.name
63-
// }
6471

6572
context.report({
6673
node,
@@ -70,33 +77,42 @@ export default createRule("prefer-destructured-store-props", {
7077
property,
7178
},
7279

73-
suggest: [
74-
{
75-
messageId: "fixUseDestructuring",
76-
data: {
77-
store,
78-
property,
79-
},
80+
// Avoid suggestions for:
81+
// dynamic accesses like {$foo[bar]}
82+
// no <script> tag
83+
// no <script> ending
84+
// variable name already defined
85+
suggest:
86+
node.computed ||
87+
!script ||
88+
!script.endTag ||
89+
defined.has(property)
90+
? []
91+
: [
92+
{
93+
messageId: "fixUseDestructuring",
94+
data: {
95+
store,
96+
property,
97+
},
8098

81-
fix(fixer) {
82-
// Avoid autofix suggestions for:
83-
// dynamic accesses like {$foo[bar]}
84-
// no <script> tag
85-
// no <script> ending
86-
if (node.computed || !script || !script.endTag) {
87-
return []
88-
}
99+
fix(fixer) {
100+
// This is only necessary to make TS shut up about script.endTag maybe being null
101+
// but since we already checked it above that warning is just wrong
102+
if (!script.endTag) {
103+
return []
104+
}
89105

90-
return [
91-
fixer.insertTextAfterRange(
92-
[script.endTag.range[0], script.endTag.range[0]],
93-
`$: ({ ${property} } = ${store});\n`,
94-
),
95-
fixer.replaceText(node, property),
96-
]
97-
},
98-
},
99-
],
106+
return [
107+
fixer.insertTextAfterRange(
108+
[script.endTag.range[0], script.endTag.range[0]],
109+
`$: ({ ${property} } = ${store});\n`,
110+
),
111+
fixer.replaceText(node, property),
112+
]
113+
},
114+
},
115+
],
100116
})
101117
})
102118
},

tests/fixtures/rules/prefer-destructured-store-props/invalid/test01-errors.json

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[
22
{
3-
"message": "Destructure bar from store $foo for better change tracking & fewer redraws",
3+
"message": "Destructure bar from $foo for better change tracking & fewer redraws",
44
"line": 6,
55
"column": 2,
66
"suggestions": [
@@ -12,13 +12,13 @@
1212
]
1313
},
1414
{
15-
"message": "Destructure baz from store $foo for better change tracking & fewer redraws",
15+
"message": "Destructure baz from $foo for better change tracking & fewer redraws",
1616
"line": 8,
1717
"column": 2,
1818
"suggestions": null
1919
},
2020
{
21-
"message": "Destructure qux from store $foo for better change tracking & fewer redraws",
21+
"message": "Destructure qux from $foo for better change tracking & fewer redraws",
2222
"line": 11,
2323
"column": 2,
2424
"suggestions": null

tests/fixtures/rules/prefer-destructured-store-props/invalid/test02-errors.json

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
[
22
{
3-
"message": "Destructure bar from store $foo for better change tracking & fewer redraws",
3+
"message": "Destructure bar from $foo for better change tracking & fewer redraws",
44
"line": 2,
55
"column": 2,
66
"suggestions": null
77
},
88
{
9-
"message": "Destructure bar from store $foo for better change tracking & fewer redraws",
9+
"message": "Destructure bar from $foo for better change tracking & fewer redraws",
1010
"line": 4,
1111
"column": 2,
1212
"suggestions": null
1313
},
1414
{
15-
"message": "Destructure bar from store $foo for better change tracking & fewer redraws",
15+
"message": "Destructure bar from $foo for better change tracking & fewer redraws",
1616
"line": 7,
1717
"column": 2,
1818
"suggestions": null
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
[
2+
{
3+
"message": "Destructure foo from $store for better change tracking & fewer redraws",
4+
"line": 7,
5+
"column": 11,
6+
"suggestions": null
7+
},
8+
{
9+
"message": "Destructure bar from $store for better change tracking & fewer redraws",
10+
"line": 8,
11+
"column": 11,
12+
"suggestions": null
13+
}
14+
]
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<!-- prettier-ignore -->
2+
<script>
3+
import store from "./store.js";
4+
5+
let foo, bar
6+
$: {
7+
foo = $store.foo;
8+
bar = $store.bar;
9+
}
10+
</script>
11+
12+
<div>
13+
foo: {foo + " " + Date.now()}
14+
</div>
15+
<div>
16+
bar: {bar + " " + Date.now()}
17+
</div>

0 commit comments

Comments
 (0)