Skip to content

Commit e15152c

Browse files
ota-meshiota-meshi
committed
fix: update prefer-destructured-store-props rule
1 parent 6d53ff2 commit e15152c

19 files changed

+425
-145
lines changed

‎README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ These rules relate to better ways of doing things to help you avoid problems:
294294
| [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: |
295295
| [svelte/no-unused-svelte-ignore](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: |
296296
| [svelte/no-useless-mustaches](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-useless-mustaches/) | disallow unnecessary mustache interpolations | :wrench: |
297-
| [svelte/prefer-destructured-store-props](https://ota-meshi.github.io/eslint-plugin-svelte/rules/prefer-destructured-store-props/) | (no description) | |
297+
| [svelte/prefer-destructured-store-props](https://ota-meshi.github.io/eslint-plugin-svelte/rules/prefer-destructured-store-props/) | destructure values from object stores for better change tracking & fewer redraws | :bulb: |
298298
| [svelte/require-optimized-style-attribute](https://ota-meshi.github.io/eslint-plugin-svelte/rules/require-optimized-style-attribute/) | require style attributes that can be optimized | |
299299
| [svelte/require-stores-init](https://ota-meshi.github.io/eslint-plugin-svelte/rules/require-stores-init/) | require initial value in store | |
300300

‎docs/rules.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ These rules relate to better ways of doing things to help you avoid problems:
4747
| [svelte/no-reactive-literals](./rules/no-reactive-literals.md) | don't assign literal values in reactive statements | :bulb: |
4848
| [svelte/no-unused-svelte-ignore](./rules/no-unused-svelte-ignore.md) | disallow unused svelte-ignore comments | :star: |
4949
| [svelte/no-useless-mustaches](./rules/no-useless-mustaches.md) | disallow unnecessary mustache interpolations | :wrench: |
50-
| [svelte/prefer-destructured-store-props](./rules/prefer-destructured-store-props.md) | (no description) | |
50+
| [svelte/prefer-destructured-store-props](./rules/prefer-destructured-store-props.md) | destructure values from object stores for better change tracking & fewer redraws | :bulb: |
5151
| [svelte/require-optimized-style-attribute](./rules/require-optimized-style-attribute.md) | require style attributes that can be optimized | |
5252
| [svelte/require-stores-init](./rules/require-stores-init.md) | require initial value in store | |
5353

‎docs/rules/prefer-destructured-store-props.md

+4-3
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,19 @@
22
pageClass: "rule-details"
33
sidebarDepth: 0
44
title: "svelte/prefer-destructured-store-props"
5-
description: "Destructure store props for more efficient redraws"
5+
description: "destructure values from object stores for better change tracking & fewer redraws"
66
---
77

88
# svelte/prefer-destructured-store-props
99

10-
> Destructure store props for more efficient redraws
10+
> destructure values from object stores for better change tracking & fewer redraws
1111
1212
- :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).
1314

1415
## :book: Rule Details
1516

16-
This rule reports on directly accessing properties of a store containing an object. These usages can instead be written as a reactive statement using destructuring to allow for more granular change-tracking and reduced redraws in the component.
17+
This rule reports on directly accessing properties of a store containing an object in templates. These usages can instead be written as a reactive statement using destructuring to allow for more granular change-tracking and reduced redraws in the component.
1718

1819
An example of the improvements can be see in this [REPL](https://svelte.dev/repl/7de86fea94ff40c48abb82da534dfb89)
1920

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

+167-58
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,83 @@
1-
import type * as ESTree from "estree"
21
import type { TSESTree } from "@typescript-eslint/types"
2+
import { findVariable } from "eslint-utils"
33
import type { AST } from "svelte-eslint-parser"
44
import { createRule } from "../utils"
5-
import { getStringIfConstant } from "../utils/ast-utils"
6-
7-
type NodeRecord = { property: string; node: TSESTree.MemberExpression }
8-
5+
import { findAttribute, isExpressionIdentifier } from "../utils/ast-utils"
6+
7+
type StoreMemberExpression = TSESTree.MemberExpression & {
8+
object: TSESTree.Identifier & { name: string }
9+
}
10+
11+
const keywords = new Set([
12+
"abstract",
13+
"arguments",
14+
"boolean",
15+
"break",
16+
"byte",
17+
"case",
18+
"catch",
19+
"char",
20+
"class",
21+
"const",
22+
"continue",
23+
"debugger",
24+
"default",
25+
"delete",
26+
"do",
27+
"double",
28+
"else",
29+
"enum",
30+
"eval",
31+
"export",
32+
"extends",
33+
"false",
34+
"final",
35+
"finally",
36+
"float",
37+
"for",
38+
"function",
39+
"goto",
40+
"if",
41+
"implements",
42+
"import",
43+
"in",
44+
"instanceof",
45+
"int",
46+
"interface",
47+
"let",
48+
"long",
49+
"native",
50+
"new",
51+
"null",
52+
"package",
53+
"private",
54+
"protected",
55+
"public",
56+
"return",
57+
"short",
58+
"static",
59+
"super",
60+
"switch",
61+
"synchronized",
62+
"this",
63+
"throw",
64+
"throws",
65+
"transient",
66+
"true",
67+
"try",
68+
"typeof",
69+
"var",
70+
"void",
71+
"volatile",
72+
"while",
73+
"with",
74+
"yield",
75+
])
976
export default createRule("prefer-destructured-store-props", {
1077
meta: {
1178
docs: {
1279
description:
13-
"Destructure values from object stores for better change tracking & fewer redraws",
80+
"destructure values from object stores for better change tracking & fewer redraws",
1481
category: "Best Practices",
1582
recommended: false,
1683
},
@@ -23,67 +90,101 @@ export default createRule("prefer-destructured-store-props", {
2390
type: "suggestion",
2491
},
2592
create(context) {
26-
let script: AST.SvelteScriptElement
93+
let mainScript: AST.SvelteScriptElement | null = null
2794

2895
// Store off instances of probably-destructurable statements
29-
const reports: NodeRecord[] = []
30-
31-
// Store off defined variable names so we can avoid offering a suggestion in those cases
32-
const defined: Set<string> = new Set()
96+
const reports: StoreMemberExpression[] = []
97+
let inScriptElement = false
98+
99+
const storeMemberAccessStack: {
100+
node: StoreMemberExpression
101+
// A list of Identifiers that make up the member expression.
102+
identifiers: TSESTree.Identifier[]
103+
}[] = []
104+
105+
/** Checks whether the given name is already defined as a variable. */
106+
function hasTopLevelVariable(name: string) {
107+
const scopeManager = context.getSourceCode().scopeManager
108+
if (scopeManager.globalScope?.set.has(name)) {
109+
return true
110+
}
111+
const moduleScope = scopeManager.globalScope?.childScopes.find(
112+
(s) => s.type === "module",
113+
)
114+
return moduleScope?.set.has(name) ?? false
115+
}
33116

34117
return {
35-
[`SvelteScriptElement`](node: AST.SvelteScriptElement) {
36-
script = node
118+
SvelteScriptElement(node) {
119+
inScriptElement = true
120+
const scriptContext = findAttribute(node, "context")
121+
const contextValue =
122+
scriptContext?.value.length === 1 && scriptContext.value[0]
123+
if (
124+
contextValue &&
125+
contextValue.type === "SvelteLiteral" &&
126+
contextValue.value === "module"
127+
) {
128+
// It is <script context="module">
129+
return
130+
}
131+
mainScript = node
37132
},
38-
39-
// Capture import names
40-
[`ImportSpecifier, ImportDefaultSpecifier, ImportNamespaceSpecifier`](
41-
node: TSESTree.ImportSpecifier,
42-
) {
43-
const { name } = node.local
44-
45-
defined.add(name)
46-
},
47-
48-
// Capture variable names
49-
[`VariableDeclarator[id.type="Identifier"]`](
50-
node: TSESTree.VariableDeclarator,
51-
) {
52-
const { name } = node.id as TSESTree.Identifier
53-
54-
defined.add(name)
133+
"SvelteScriptElement:exit"() {
134+
inScriptElement = false
55135
},
56136

57137
// {$foo.bar}
58138
// should be
59139
// $: ({ bar } = $foo);
60140
// {bar}
61141
// Same with {$foo["bar"]}
62-
[`MemberExpression[object.name=/^\\$/]`](
63-
node: TSESTree.MemberExpression,
142+
"MemberExpression[object.type='Identifier'][object.name=/^\\$/]"(
143+
node: StoreMemberExpression,
64144
) {
65-
const property =
66-
node.property.type === "Identifier"
67-
? node.property.name
68-
: getStringIfConstant(node.property as ESTree.Expression)
69-
70-
if (!property) {
71-
return
145+
if (inScriptElement) return // Within a script tag
146+
storeMemberAccessStack.unshift({ node, identifiers: [] })
147+
},
148+
Identifier(node: TSESTree.Identifier) {
149+
storeMemberAccessStack[0]?.identifiers.push(node)
150+
},
151+
"MemberExpression[object.type='Identifier'][object.name=/^\\$/]:exit"(
152+
node: StoreMemberExpression,
153+
) {
154+
if (storeMemberAccessStack[0]?.node !== node) return
155+
const { identifiers } = storeMemberAccessStack.shift()!
156+
157+
for (const id of identifiers) {
158+
if (!isExpressionIdentifier(id)) continue
159+
const variable = findVariable(context.getScope(), id)
160+
const isTopLevel =
161+
!variable ||
162+
variable.scope.type === "module" ||
163+
variable.scope.type === "global"
164+
if (!isTopLevel) {
165+
// Member expressions may use variables defined with {#each} etc.
166+
return
167+
}
72168
}
73-
74-
reports.push({ property, node })
169+
reports.push(node)
75170
},
76171

77-
[`Program:exit`]() {
78-
reports.forEach(({ property, node }) => {
79-
const store = (node.object as TSESTree.Identifier).name
172+
"Program:exit"() {
173+
const scriptEndTag = mainScript && mainScript.endTag
174+
for (const node of reports) {
175+
const store = node.object.name
80176

81177
context.report({
82178
node,
83179
messageId: "useDestructuring",
84180
data: {
85181
store,
86-
property,
182+
property: !node.computed
183+
? node.property.name
184+
: context
185+
.getSourceCode()
186+
.getText(node.property)
187+
.replace(/\s+/g, " "),
87188
},
88189

89190
// Avoid suggestions for:
@@ -92,38 +193,46 @@ export default createRule("prefer-destructured-store-props", {
92193
// no <script> ending
93194
// variable name already defined
94195
suggest:
95-
node.computed ||
96-
!script ||
97-
!script.endTag ||
98-
defined.has(property)
196+
node.computed || !scriptEndTag
99197
? []
100198
: [
101199
{
102200
messageId: "fixUseDestructuring",
103201
data: {
104202
store,
105-
property,
203+
property: node.property.name,
106204
},
107205

108206
fix(fixer) {
109-
// This is only necessary to make TS shut up about script.endTag maybe being null
110-
// but since we already checked it above that warning is just wrong
111-
if (!script.endTag) {
112-
return []
207+
const propName = node.property.name
208+
209+
let varName = propName
210+
if (varName.startsWith("$")) {
211+
varName = varName.slice(1)
212+
}
213+
const baseName = varName
214+
let suffix = 0
215+
if (keywords.has(varName)) {
216+
varName = `${baseName}${++suffix}`
217+
}
218+
while (hasTopLevelVariable(varName)) {
219+
varName = `${baseName}${++suffix}`
113220
}
114221

115222
return [
116223
fixer.insertTextAfterRange(
117-
[script.endTag.range[0], script.endTag.range[0]],
118-
`$: ({ ${property} } = ${store});\n`,
224+
[scriptEndTag.range[0], scriptEndTag.range[0]],
225+
`$: ({ ${propName}${
226+
propName !== varName ? `: ${varName}` : ""
227+
} } = ${store});\n`,
119228
),
120-
fixer.replaceText(node, property),
229+
fixer.replaceText(node, varName),
121230
]
122231
},
123232
},
124233
],
125234
})
126-
})
235+
}
127236
},
128237
}
129238
},

0 commit comments

Comments
 (0)