Skip to content

Commit

Permalink
Add vue/no-ref-object-destructure rule (#1965)
Browse files Browse the repository at this point in the history
* Add `vue/no-ref-object-destructure` rule

* add test cases

* fix test case

* Apply suggestions from code review

Co-authored-by: Flo Edelmann <florian-edelmann@online.de>

* remove comment in testcases

* add rfc link to doc

Co-authored-by: Flo Edelmann <florian-edelmann@online.de>
  • Loading branch information
ota-meshi and FloEdelmann committed Sep 10, 2022
1 parent 9b55f3c commit 8828bbb
Show file tree
Hide file tree
Showing 39 changed files with 1,804 additions and 81 deletions.
1 change: 1 addition & 0 deletions docs/rules/README.md
Expand Up @@ -227,6 +227,7 @@ For example:
| [vue/no-empty-component-block](./no-empty-component-block.md) | disallow the `<template>` `<script>` `<style>` block to be empty | | :hammer: |
| [vue/no-multiple-objects-in-class](./no-multiple-objects-in-class.md) | disallow to pass multiple objects into array to class | | :hammer: |
| [vue/no-potential-component-option-typo](./no-potential-component-option-typo.md) | disallow a potential typo in your component property | :bulb: | :hammer: |
| [vue/no-ref-object-destructure](./no-ref-object-destructure.md) | disallow destructuring of ref objects that can lead to loss of reactivity | | :warning: |
| [vue/no-restricted-block](./no-restricted-block.md) | disallow specific block | | :hammer: |
| [vue/no-restricted-call-after-await](./no-restricted-call-after-await.md) | disallow asynchronously called restricted methods | | :hammer: |
| [vue/no-restricted-class](./no-restricted-class.md) | disallow specific classes in Vue components | | :warning: |
Expand Down
55 changes: 55 additions & 0 deletions docs/rules/no-ref-object-destructure.md
@@ -0,0 +1,55 @@
---
pageClass: rule-details
sidebarDepth: 0
title: vue/no-ref-object-destructure
description: disallow destructuring of ref objects that can lead to loss of reactivity
---
# vue/no-ref-object-destructure

> disallow destructuring of ref objects that can lead to loss of reactivity
- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> ***This rule has not been released yet.*** </badge>

## :book: Rule Details

This rule reports the destructuring of ref objects causing the value to lose reactivity.

<eslint-code-block :rules="{'vue/no-ref-object-destructure': ['error']}" language="javascript" filename="example.js" >

```js
import { ref } from 'vue'
const count = ref(0)
const v1 = count.value /* ✗ BAD */
const { value: v2 } = count /* ✗ BAD */
const v3 = computed(() => count.value /* ✓ GOOD */)
const v4 = fn(count.value) /* ✗ BAD */
const v5 = fn(count) /* ✓ GOOD */
const v6 = computed(() => fn(count.value) /* ✓ GOOD */)
```

</eslint-code-block>

This rule also supports Reactivity Transform, but Reactivity Transform is an experimental feature and may have false positives due to future Vue changes.
See the [RFC](https://github.com/vuejs/rfcs/pull/420) for more information on Reactivity Transform.

<eslint-code-block :rules="{'vue/no-ref-object-destructure': ['error']}" language="javascript" filename="example.js" >

```js
const count = $ref(0)
const v1 = count /* ✗ BAD */
const v2 = $computed(() => count /* ✓ GOOD */)
const v3 = fn(count) /* ✗ BAD */
const v4 = fn($$(count)) /* ✓ GOOD */
const v5 = $computed(() => fn(count) /* ✓ GOOD */)
```

</eslint-code-block>

## :wrench: Options

Nothing.

## :mag: Implementation

- [Rule source](https://github.com/vuejs/eslint-plugin-vue/blob/master/lib/rules/no-ref-object-destructure.js)
- [Test source](https://github.com/vuejs/eslint-plugin-vue/blob/master/tests/lib/rules/no-ref-object-destructure.js)
1 change: 1 addition & 0 deletions lib/index.js
Expand Up @@ -104,6 +104,7 @@ module.exports = {
'no-parsing-error': require('./rules/no-parsing-error'),
'no-potential-component-option-typo': require('./rules/no-potential-component-option-typo'),
'no-ref-as-operand': require('./rules/no-ref-as-operand'),
'no-ref-object-destructure': require('./rules/no-ref-object-destructure'),
'no-reserved-component-names': require('./rules/no-reserved-component-names'),
'no-reserved-keys': require('./rules/no-reserved-keys'),
'no-reserved-props': require('./rules/no-reserved-props'),
Expand Down
79 changes: 12 additions & 67 deletions lib/rules/no-ref-as-operand.js
Expand Up @@ -3,9 +3,14 @@
* See LICENSE file in root directory for full license.
*/
'use strict'
const { ReferenceTracker, findVariable } = require('eslint-utils')

const { extractRefObjectReferences } = require('../utils/ref-object-references')
const utils = require('../utils')

/**
* @typedef {import('../utils/ref-object-references').RefObjectReferences} RefObjectReferences
*/

module.exports = {
meta: {
type: 'suggestion',
Expand All @@ -24,20 +29,14 @@ module.exports = {
},
/** @param {RuleContext} context */
create(context) {
/**
* @typedef {object} ReferenceData
* @property {VariableDeclarator} variableDeclarator
* @property {VariableDeclaration | null} variableDeclaration
* @property {string} method
*/
/** @type {Map<Identifier, ReferenceData>} */
const refReferenceIds = new Map()
/** @type {RefObjectReferences} */
let refReferences

/**
* @param {Identifier} node
*/
function reportIfRefWrapped(node) {
const data = refReferenceIds.get(node)
const data = refReferences.get(node)
if (!data) {
return
}
Expand All @@ -54,59 +53,7 @@ module.exports = {
}
return {
Program() {
const tracker = new ReferenceTracker(context.getScope())
const traceMap = utils.createCompositionApiTraceMap({
[ReferenceTracker.ESM]: true,
ref: {
[ReferenceTracker.CALL]: true
},
computed: {
[ReferenceTracker.CALL]: true
},
toRef: {
[ReferenceTracker.CALL]: true
},
customRef: {
[ReferenceTracker.CALL]: true
},
shallowRef: {
[ReferenceTracker.CALL]: true
}
})

for (const { node, path } of tracker.iterateEsmReferences(traceMap)) {
const variableDeclarator = node.parent
if (
!variableDeclarator ||
variableDeclarator.type !== 'VariableDeclarator' ||
variableDeclarator.id.type !== 'Identifier'
) {
continue
}
const variable = findVariable(
context.getScope(),
variableDeclarator.id
)
if (!variable) {
continue
}
const variableDeclaration =
(variableDeclarator.parent &&
variableDeclarator.parent.type === 'VariableDeclaration' &&
variableDeclarator.parent) ||
null
for (const reference of variable.references) {
if (!reference.isRead()) {
continue
}

refReferenceIds.set(reference.identifier, {
variableDeclarator,
variableDeclaration,
method: path[1]
})
}
}
refReferences = extractRefObjectReferences(context)
},
// if (refValue)
/** @param {Identifier} node */
Expand Down Expand Up @@ -148,11 +95,9 @@ module.exports = {
return
}
// Report only constants.
const data = refReferenceIds.get(node)
if (!data) {
return
}
const data = refReferences.get(node)
if (
!data ||
!data.variableDeclaration ||
data.variableDeclaration.kind !== 'const'
) {
Expand Down
183 changes: 183 additions & 0 deletions lib/rules/no-ref-object-destructure.js
@@ -0,0 +1,183 @@
/**
* @author Yosuke Ota <https://github.com/ota-meshi>
* See LICENSE file in root directory for full license.
*/
'use strict'

// ------------------------------------------------------------------------------
// Requirements
// ------------------------------------------------------------------------------

const utils = require('../utils')
const {
extractRefObjectReferences,
extractReactiveVariableReferences
} = require('../utils/ref-object-references')

// ------------------------------------------------------------------------------
// Helpers
// ------------------------------------------------------------------------------

/**
* @typedef {import('../utils/ref-object-references').RefObjectReferences} RefObjectReferences
* @typedef {import('../utils/ref-object-references').RefObjectReference} RefObjectReference
*/

/**
* Checks whether writing assigns a value to the given pattern.
* @param {Pattern | AssignmentProperty | Property} node
* @returns {boolean}
*/
function isUpdate(node) {
const parent = node.parent
if (parent.type === 'UpdateExpression' && parent.argument === node) {
// e.g. `pattern++`
return true
}
if (parent.type === 'AssignmentExpression' && parent.left === node) {
// e.g. `pattern = 42`
return true
}
if (
(parent.type === 'Property' && parent.value === node) ||
parent.type === 'ArrayPattern' ||
(parent.type === 'ObjectPattern' &&
parent.properties.includes(/** @type {any} */ (node))) ||
(parent.type === 'AssignmentPattern' && parent.left === node) ||
parent.type === 'RestElement' ||
(parent.type === 'MemberExpression' && parent.object === node)
) {
return isUpdate(parent)
}
return false
}

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = {
meta: {
type: 'problem',
docs: {
description:
'disallow destructuring of ref objects that can lead to loss of reactivity',
categories: undefined,
url: 'https://eslint.vuejs.org/rules/no-ref-object-destructure.html'
},
fixable: null,
schema: [],
messages: {
getValueInSameScope:
'Getting a value from the ref object in the same scope will cause the value to lose reactivity.',
getReactiveVariableInSameScope:
'Getting a reactive variable in the same scope will cause the value to lose reactivity.'
}
},
/**
* @param {RuleContext} context
* @returns {RuleListener}
*/
create(context) {
/**
* @typedef {object} ScopeStack
* @property {ScopeStack | null} upper
* @property {Program | FunctionExpression | FunctionDeclaration | ArrowFunctionExpression} node
*/
/** @type {ScopeStack} */
let scopeStack = { upper: null, node: context.getSourceCode().ast }
/** @type {Map<CallExpression, ScopeStack>} */
const scopes = new Map()

const refObjectReferences = extractRefObjectReferences(context)
const reactiveVariableReferences =
extractReactiveVariableReferences(context)

/**
* Verify the given ref object value. `refObj = ref(); refObj.value;`
* @param {Expression | Super | ObjectPattern} node
*/
function verifyRefObjectValue(node) {
const ref = refObjectReferences.get(node)
if (!ref) {
return
}
if (scopes.get(ref.define) !== scopeStack) {
// Not in the same scope
return
}

context.report({
node,
messageId: 'getValueInSameScope'
})
}

/**
* Verify the given reactive variable. `refVal = $ref(); refVal;`
* @param {Identifier} node
*/
function verifyReactiveVariable(node) {
const ref = reactiveVariableReferences.get(node)
if (!ref || ref.escape) {
return
}
if (scopes.get(ref.define) !== scopeStack) {
// Not in the same scope
return
}

context.report({
node,
messageId: 'getReactiveVariableInSameScope'
})
}

return {
':function'(node) {
scopeStack = { upper: scopeStack, node }
},
':function:exit'() {
scopeStack = scopeStack.upper || scopeStack
},
CallExpression(node) {
scopes.set(node, scopeStack)
},
/**
* Check for `refObj.value`.
*/
'MemberExpression:exit'(node) {
if (isUpdate(node)) {
// e.g. `refObj.value = 42`, `refObj.value++`
return
}
const name = utils.getStaticPropertyName(node)
if (name !== 'value') {
return
}
verifyRefObjectValue(node.object)
},
/**
* Check for `{value} = refObj`.
*/
'ObjectPattern:exit'(node) {
const prop = utils.findAssignmentProperty(node, 'value')
if (!prop) {
return
}
verifyRefObjectValue(node)
},
/**
* Check for reactive variable`.
* @param {Identifier} node
*/
'Identifier:exit'(node) {
if (isUpdate(node)) {
// e.g. `reactiveVariable = 42`, `reactiveVariable++`
return
}
verifyReactiveVariable(node)
}
}
}
}

0 comments on commit 8828bbb

Please sign in to comment.