Skip to content

Commit 8828bbb

Browse files
Add vue/no-ref-object-destructure rule (#1965)
* Add `vue/no-ref-object-destructure` rule * add test cases * fix test case * Apply suggestions from code review Co-authored-by: Flo Edelmann <[email protected]> * remove comment in testcases * add rfc link to doc Co-authored-by: Flo Edelmann <[email protected]>
1 parent 9b55f3c commit 8828bbb

39 files changed

+1804
-81
lines changed

docs/rules/README.md

+1
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ For example:
227227
| [vue/no-empty-component-block](./no-empty-component-block.md) | disallow the `<template>` `<script>` `<style>` block to be empty | | :hammer: |
228228
| [vue/no-multiple-objects-in-class](./no-multiple-objects-in-class.md) | disallow to pass multiple objects into array to class | | :hammer: |
229229
| [vue/no-potential-component-option-typo](./no-potential-component-option-typo.md) | disallow a potential typo in your component property | :bulb: | :hammer: |
230+
| [vue/no-ref-object-destructure](./no-ref-object-destructure.md) | disallow destructuring of ref objects that can lead to loss of reactivity | | :warning: |
230231
| [vue/no-restricted-block](./no-restricted-block.md) | disallow specific block | | :hammer: |
231232
| [vue/no-restricted-call-after-await](./no-restricted-call-after-await.md) | disallow asynchronously called restricted methods | | :hammer: |
232233
| [vue/no-restricted-class](./no-restricted-class.md) | disallow specific classes in Vue components | | :warning: |
+55
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
pageClass: rule-details
3+
sidebarDepth: 0
4+
title: vue/no-ref-object-destructure
5+
description: disallow destructuring of ref objects that can lead to loss of reactivity
6+
---
7+
# vue/no-ref-object-destructure
8+
9+
> disallow destructuring of ref objects that can lead to loss of reactivity
10+
11+
- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> ***This rule has not been released yet.*** </badge>
12+
13+
## :book: Rule Details
14+
15+
This rule reports the destructuring of ref objects causing the value to lose reactivity.
16+
17+
<eslint-code-block :rules="{'vue/no-ref-object-destructure': ['error']}" language="javascript" filename="example.js" >
18+
19+
```js
20+
import { ref } from 'vue'
21+
const count = ref(0)
22+
const v1 = count.value /* ✗ BAD */
23+
const { value: v2 } = count /* ✗ BAD */
24+
const v3 = computed(() => count.value /* ✓ GOOD */)
25+
const v4 = fn(count.value) /* ✗ BAD */
26+
const v5 = fn(count) /* ✓ GOOD */
27+
const v6 = computed(() => fn(count.value) /* ✓ GOOD */)
28+
```
29+
30+
</eslint-code-block>
31+
32+
This rule also supports Reactivity Transform, but Reactivity Transform is an experimental feature and may have false positives due to future Vue changes.
33+
See the [RFC](https://github.com/vuejs/rfcs/pull/420) for more information on Reactivity Transform.
34+
35+
<eslint-code-block :rules="{'vue/no-ref-object-destructure': ['error']}" language="javascript" filename="example.js" >
36+
37+
```js
38+
const count = $ref(0)
39+
const v1 = count /* ✗ BAD */
40+
const v2 = $computed(() => count /* ✓ GOOD */)
41+
const v3 = fn(count) /* ✗ BAD */
42+
const v4 = fn($$(count)) /* ✓ GOOD */
43+
const v5 = $computed(() => fn(count) /* ✓ GOOD */)
44+
```
45+
46+
</eslint-code-block>
47+
48+
## :wrench: Options
49+
50+
Nothing.
51+
52+
## :mag: Implementation
53+
54+
- [Rule source](https://github.com/vuejs/eslint-plugin-vue/blob/master/lib/rules/no-ref-object-destructure.js)
55+
- [Test source](https://github.com/vuejs/eslint-plugin-vue/blob/master/tests/lib/rules/no-ref-object-destructure.js)

lib/index.js

+1
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ module.exports = {
104104
'no-parsing-error': require('./rules/no-parsing-error'),
105105
'no-potential-component-option-typo': require('./rules/no-potential-component-option-typo'),
106106
'no-ref-as-operand': require('./rules/no-ref-as-operand'),
107+
'no-ref-object-destructure': require('./rules/no-ref-object-destructure'),
107108
'no-reserved-component-names': require('./rules/no-reserved-component-names'),
108109
'no-reserved-keys': require('./rules/no-reserved-keys'),
109110
'no-reserved-props': require('./rules/no-reserved-props'),

lib/rules/no-ref-as-operand.js

+12-67
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,14 @@
33
* See LICENSE file in root directory for full license.
44
*/
55
'use strict'
6-
const { ReferenceTracker, findVariable } = require('eslint-utils')
6+
7+
const { extractRefObjectReferences } = require('../utils/ref-object-references')
78
const utils = require('../utils')
89

10+
/**
11+
* @typedef {import('../utils/ref-object-references').RefObjectReferences} RefObjectReferences
12+
*/
13+
914
module.exports = {
1015
meta: {
1116
type: 'suggestion',
@@ -24,20 +29,14 @@ module.exports = {
2429
},
2530
/** @param {RuleContext} context */
2631
create(context) {
27-
/**
28-
* @typedef {object} ReferenceData
29-
* @property {VariableDeclarator} variableDeclarator
30-
* @property {VariableDeclaration | null} variableDeclaration
31-
* @property {string} method
32-
*/
33-
/** @type {Map<Identifier, ReferenceData>} */
34-
const refReferenceIds = new Map()
32+
/** @type {RefObjectReferences} */
33+
let refReferences
3534

3635
/**
3736
* @param {Identifier} node
3837
*/
3938
function reportIfRefWrapped(node) {
40-
const data = refReferenceIds.get(node)
39+
const data = refReferences.get(node)
4140
if (!data) {
4241
return
4342
}
@@ -54,59 +53,7 @@ module.exports = {
5453
}
5554
return {
5655
Program() {
57-
const tracker = new ReferenceTracker(context.getScope())
58-
const traceMap = utils.createCompositionApiTraceMap({
59-
[ReferenceTracker.ESM]: true,
60-
ref: {
61-
[ReferenceTracker.CALL]: true
62-
},
63-
computed: {
64-
[ReferenceTracker.CALL]: true
65-
},
66-
toRef: {
67-
[ReferenceTracker.CALL]: true
68-
},
69-
customRef: {
70-
[ReferenceTracker.CALL]: true
71-
},
72-
shallowRef: {
73-
[ReferenceTracker.CALL]: true
74-
}
75-
})
76-
77-
for (const { node, path } of tracker.iterateEsmReferences(traceMap)) {
78-
const variableDeclarator = node.parent
79-
if (
80-
!variableDeclarator ||
81-
variableDeclarator.type !== 'VariableDeclarator' ||
82-
variableDeclarator.id.type !== 'Identifier'
83-
) {
84-
continue
85-
}
86-
const variable = findVariable(
87-
context.getScope(),
88-
variableDeclarator.id
89-
)
90-
if (!variable) {
91-
continue
92-
}
93-
const variableDeclaration =
94-
(variableDeclarator.parent &&
95-
variableDeclarator.parent.type === 'VariableDeclaration' &&
96-
variableDeclarator.parent) ||
97-
null
98-
for (const reference of variable.references) {
99-
if (!reference.isRead()) {
100-
continue
101-
}
102-
103-
refReferenceIds.set(reference.identifier, {
104-
variableDeclarator,
105-
variableDeclaration,
106-
method: path[1]
107-
})
108-
}
109-
}
56+
refReferences = extractRefObjectReferences(context)
11057
},
11158
// if (refValue)
11259
/** @param {Identifier} node */
@@ -148,11 +95,9 @@ module.exports = {
14895
return
14996
}
15097
// Report only constants.
151-
const data = refReferenceIds.get(node)
152-
if (!data) {
153-
return
154-
}
98+
const data = refReferences.get(node)
15599
if (
100+
!data ||
156101
!data.variableDeclaration ||
157102
data.variableDeclaration.kind !== 'const'
158103
) {
+183
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
/**
2+
* @author Yosuke Ota <https://github.com/ota-meshi>
3+
* See LICENSE file in root directory for full license.
4+
*/
5+
'use strict'
6+
7+
// ------------------------------------------------------------------------------
8+
// Requirements
9+
// ------------------------------------------------------------------------------
10+
11+
const utils = require('../utils')
12+
const {
13+
extractRefObjectReferences,
14+
extractReactiveVariableReferences
15+
} = require('../utils/ref-object-references')
16+
17+
// ------------------------------------------------------------------------------
18+
// Helpers
19+
// ------------------------------------------------------------------------------
20+
21+
/**
22+
* @typedef {import('../utils/ref-object-references').RefObjectReferences} RefObjectReferences
23+
* @typedef {import('../utils/ref-object-references').RefObjectReference} RefObjectReference
24+
*/
25+
26+
/**
27+
* Checks whether writing assigns a value to the given pattern.
28+
* @param {Pattern | AssignmentProperty | Property} node
29+
* @returns {boolean}
30+
*/
31+
function isUpdate(node) {
32+
const parent = node.parent
33+
if (parent.type === 'UpdateExpression' && parent.argument === node) {
34+
// e.g. `pattern++`
35+
return true
36+
}
37+
if (parent.type === 'AssignmentExpression' && parent.left === node) {
38+
// e.g. `pattern = 42`
39+
return true
40+
}
41+
if (
42+
(parent.type === 'Property' && parent.value === node) ||
43+
parent.type === 'ArrayPattern' ||
44+
(parent.type === 'ObjectPattern' &&
45+
parent.properties.includes(/** @type {any} */ (node))) ||
46+
(parent.type === 'AssignmentPattern' && parent.left === node) ||
47+
parent.type === 'RestElement' ||
48+
(parent.type === 'MemberExpression' && parent.object === node)
49+
) {
50+
return isUpdate(parent)
51+
}
52+
return false
53+
}
54+
55+
// ------------------------------------------------------------------------------
56+
// Rule Definition
57+
// ------------------------------------------------------------------------------
58+
59+
module.exports = {
60+
meta: {
61+
type: 'problem',
62+
docs: {
63+
description:
64+
'disallow destructuring of ref objects that can lead to loss of reactivity',
65+
categories: undefined,
66+
url: 'https://eslint.vuejs.org/rules/no-ref-object-destructure.html'
67+
},
68+
fixable: null,
69+
schema: [],
70+
messages: {
71+
getValueInSameScope:
72+
'Getting a value from the ref object in the same scope will cause the value to lose reactivity.',
73+
getReactiveVariableInSameScope:
74+
'Getting a reactive variable in the same scope will cause the value to lose reactivity.'
75+
}
76+
},
77+
/**
78+
* @param {RuleContext} context
79+
* @returns {RuleListener}
80+
*/
81+
create(context) {
82+
/**
83+
* @typedef {object} ScopeStack
84+
* @property {ScopeStack | null} upper
85+
* @property {Program | FunctionExpression | FunctionDeclaration | ArrowFunctionExpression} node
86+
*/
87+
/** @type {ScopeStack} */
88+
let scopeStack = { upper: null, node: context.getSourceCode().ast }
89+
/** @type {Map<CallExpression, ScopeStack>} */
90+
const scopes = new Map()
91+
92+
const refObjectReferences = extractRefObjectReferences(context)
93+
const reactiveVariableReferences =
94+
extractReactiveVariableReferences(context)
95+
96+
/**
97+
* Verify the given ref object value. `refObj = ref(); refObj.value;`
98+
* @param {Expression | Super | ObjectPattern} node
99+
*/
100+
function verifyRefObjectValue(node) {
101+
const ref = refObjectReferences.get(node)
102+
if (!ref) {
103+
return
104+
}
105+
if (scopes.get(ref.define) !== scopeStack) {
106+
// Not in the same scope
107+
return
108+
}
109+
110+
context.report({
111+
node,
112+
messageId: 'getValueInSameScope'
113+
})
114+
}
115+
116+
/**
117+
* Verify the given reactive variable. `refVal = $ref(); refVal;`
118+
* @param {Identifier} node
119+
*/
120+
function verifyReactiveVariable(node) {
121+
const ref = reactiveVariableReferences.get(node)
122+
if (!ref || ref.escape) {
123+
return
124+
}
125+
if (scopes.get(ref.define) !== scopeStack) {
126+
// Not in the same scope
127+
return
128+
}
129+
130+
context.report({
131+
node,
132+
messageId: 'getReactiveVariableInSameScope'
133+
})
134+
}
135+
136+
return {
137+
':function'(node) {
138+
scopeStack = { upper: scopeStack, node }
139+
},
140+
':function:exit'() {
141+
scopeStack = scopeStack.upper || scopeStack
142+
},
143+
CallExpression(node) {
144+
scopes.set(node, scopeStack)
145+
},
146+
/**
147+
* Check for `refObj.value`.
148+
*/
149+
'MemberExpression:exit'(node) {
150+
if (isUpdate(node)) {
151+
// e.g. `refObj.value = 42`, `refObj.value++`
152+
return
153+
}
154+
const name = utils.getStaticPropertyName(node)
155+
if (name !== 'value') {
156+
return
157+
}
158+
verifyRefObjectValue(node.object)
159+
},
160+
/**
161+
* Check for `{value} = refObj`.
162+
*/
163+
'ObjectPattern:exit'(node) {
164+
const prop = utils.findAssignmentProperty(node, 'value')
165+
if (!prop) {
166+
return
167+
}
168+
verifyRefObjectValue(node)
169+
},
170+
/**
171+
* Check for reactive variable`.
172+
* @param {Identifier} node
173+
*/
174+
'Identifier:exit'(node) {
175+
if (isUpdate(node)) {
176+
// e.g. `reactiveVariable = 42`, `reactiveVariable++`
177+
return
178+
}
179+
verifyReactiveVariable(node)
180+
}
181+
}
182+
}
183+
}

0 commit comments

Comments
 (0)