From 2dc606ca589ff5cdcd197c3c64ace7e575a6347d Mon Sep 17 00:00:00 2001 From: Yosuke Ota Date: Wed, 18 Sep 2024 19:36:15 +0900 Subject: [PATCH] Add support for props destructure to `vue/no-setup-props-reactivity-loss` rule (#2550) --- lib/rules/no-setup-props-reactivity-loss.js | 200 ++++++++++++------ tests/lib/rules/no-setup-props-destructure.js | 15 -- .../rules/no-setup-props-reactivity-loss.js | 21 +- 3 files changed, 155 insertions(+), 81 deletions(-) diff --git a/lib/rules/no-setup-props-reactivity-loss.js b/lib/rules/no-setup-props-reactivity-loss.js index 4c3be8b90..04517f90a 100644 --- a/lib/rules/no-setup-props-reactivity-loss.js +++ b/lib/rules/no-setup-props-reactivity-loss.js @@ -6,6 +6,60 @@ const { findVariable } = require('@eslint-community/eslint-utils') const utils = require('../utils') +/** + * @typedef {'props'|'prop'} PropIdKind + * - `'props'`: A node is a container object that has props. + * - `'prop'`: A node is a variable with one prop. + */ +/** + * @typedef {object} PropId + * @property {Pattern} node + * @property {PropIdKind} kind + */ +/** + * Iterates over Prop identifiers by parsing the given pattern + * in the left operand of defineProps(). + * @param {Pattern} node + * @returns {IterableIterator} + */ +function* iteratePropIds(node) { + switch (node.type) { + case 'ObjectPattern': { + for (const prop of node.properties) { + yield prop.type === 'Property' + ? { + // e.g. `const { prop } = defineProps()` + node: unwrapAssignment(prop.value), + kind: 'prop' + } + : { + // RestElement + // e.g. `const { x, ...prop } = defineProps()` + node: unwrapAssignment(prop.argument), + kind: 'props' + } + } + break + } + default: { + // e.g. `const props = defineProps()` + yield { node: unwrapAssignment(node), kind: 'props' } + } + } +} + +/** + * @template {Pattern} T + * @param {T} node + * @returns {Pattern} + */ +function unwrapAssignment(node) { + if (node.type === 'AssignmentPattern') { + return node.left + } + return node +} + module.exports = { meta: { type: 'suggestion', @@ -31,7 +85,9 @@ module.exports = { create(context) { /** * @typedef {object} ScopePropsReferences - * @property {Set} refs + * @property {object} refs + * @property {Set} refs.props A set of references to container objects with multiple props. + * @property {Set} refs.prop A set of references a variable with one property. * @property {string} scopeName */ /** @type {Map} */ @@ -72,70 +128,72 @@ module.exports = { wrapperExpressionTypes.has(rightNode.type) && isPropsMemberAccessed(rightNode, propsReferences) ) { - return report(rightNode, 'getProperty', propsReferences.scopeName) - } - - if ( - left.type !== 'ArrayPattern' && - left.type !== 'ObjectPattern' && - rightNode.type !== 'MemberExpression' && - rightNode.type !== 'ConditionalExpression' && - rightNode.type !== 'TemplateLiteral' - ) { + // e.g. `const foo = { x: props.x }` + report(rightNode, 'getProperty', propsReferences.scopeName) return } - if (rightNode.type === 'TemplateLiteral') { - rightNode.expressions.some((expression) => - checkMemberAccess(expression, propsReferences, left, right) - ) - } else { - checkMemberAccess(rightNode, propsReferences, left, right) + // Get the expression that provides the value. + /** @type {Expression | Super} */ + let expression = rightNode + while (expression.type === 'MemberExpression') { + expression = utils.skipChainExpression(expression.object) } - } + /** A list of expression nodes to verify */ + const expressions = + expression.type === 'TemplateLiteral' + ? expression.expressions + : expression.type === 'ConditionalExpression' + ? [expression.test, expression.consequent, expression.alternate] + : expression.type === 'Identifier' + ? [expression] + : [] - /** - * @param {Expression | Super} rightId - * @param {ScopePropsReferences} propsReferences - * @param {Pattern} left - * @param {Expression} right - * @return {boolean} - */ - function checkMemberAccess(rightId, propsReferences, left, right) { - while (rightId.type === 'MemberExpression') { - rightId = utils.skipChainExpression(rightId.object) - } - if (rightId.type === 'Identifier' && propsReferences.refs.has(rightId)) { - report(left, 'getProperty', propsReferences.scopeName) - return true - } if ( - rightId.type === 'ConditionalExpression' && - (isPropsMemberAccessed(rightId.test, propsReferences) || - isPropsMemberAccessed(rightId.consequent, propsReferences) || - isPropsMemberAccessed(rightId.alternate, propsReferences)) + (left.type === 'ArrayPattern' || left.type === 'ObjectPattern') && + expressions.some( + (expr) => + expr.type === 'Identifier' && propsReferences.refs.props.has(expr) + ) ) { - report(right, 'getProperty', propsReferences.scopeName) - return true + // e.g. `const {foo} = props` + report(left, 'getProperty', propsReferences.scopeName) + return + } + + const reportNode = expressions.find((expr) => + isPropsMemberAccessed(expr, propsReferences) + ) + if (reportNode) { + report(reportNode, 'getProperty', propsReferences.scopeName) } - return false } /** - * @param {Expression} node + * @param {Expression | Super} node * @param {ScopePropsReferences} propsReferences */ function isPropsMemberAccessed(node, propsReferences) { - const propRefs = [...propsReferences.refs.values()] - - return propRefs.some((props) => { + for (const props of propsReferences.refs.props) { const isPropsInExpressionRange = utils.inRange(node.range, props) const isPropsMemberExpression = props.parent.type === 'MemberExpression' && props.parent.object === props - return isPropsInExpressionRange && isPropsMemberExpression - }) + if (isPropsInExpressionRange && isPropsMemberExpression) { + return true + } + } + + // Checks for actual member access using prop destructuring. + for (const prop of propsReferences.refs.prop) { + const isPropsInExpressionRange = utils.inRange(node.range, prop) + if (isPropsInExpressionRange) { + return true + } + } + + return false } /** @@ -149,16 +207,12 @@ module.exports = { let scopeStack = null /** - * @param {Pattern | null} node + * @param {PropId} propId * @param {FunctionDeclaration | FunctionExpression | ArrowFunctionExpression | Program} scopeNode * @param {import('eslint').Scope.Scope} currentScope * @param {string} scopeName */ - function processPattern(node, scopeNode, currentScope, scopeName) { - if (!node) { - // no arguments - return - } + function processPropId({ node, kind }, scopeNode, currentScope, scopeName) { if ( node.type === 'RestElement' || node.type === 'AssignmentPattern' || @@ -176,7 +230,19 @@ module.exports = { if (!variable) { return } - const propsReferenceIds = new Set() + + let scopePropsReferences = setupScopePropsReferenceIds.get(scopeNode) + if (!scopePropsReferences) { + scopePropsReferences = { + refs: { + props: new Set(), + prop: new Set() + }, + scopeName + } + setupScopePropsReferenceIds.set(scopeNode, scopePropsReferences) + } + const propsReferenceIds = scopePropsReferences.refs[kind] for (const reference of variable.references) { // If reference is in another scope, we can't check it. if (reference.from !== currentScope) { @@ -189,11 +255,8 @@ module.exports = { propsReferenceIds.add(reference.identifier) } - setupScopePropsReferenceIds.set(scopeNode, { - refs: propsReferenceIds, - scopeName - }) } + return utils.compositingVisitors( { /** @@ -287,20 +350,29 @@ module.exports = { } else if (target.parent.type === 'AssignmentExpression') { id = target.parent.right === target ? target.parent.left : null } + if (!id) return const currentScope = utils.getScope(context, node) - processPattern( - id, - context.getSourceCode().ast, - currentScope, - ' - `, - errors: [ - { - message: - 'Destructuring the `props` will cause the value to lose reactivity.', - line: 3 - } - ] - }, { filename: 'test.vue', code: ` diff --git a/tests/lib/rules/no-setup-props-reactivity-loss.js b/tests/lib/rules/no-setup-props-reactivity-loss.js index edf721d8d..fce921bfe 100644 --- a/tests/lib/rules/no-setup-props-reactivity-loss.js +++ b/tests/lib/rules/no-setup-props-reactivity-loss.js @@ -251,6 +251,22 @@ tester.run('no-setup-props-reactivity-loss', rule, { const count = computed(() => props.count) ` + }, + { + filename: 'test.vue', + code: ` + + ` + }, + { + filename: 'test.vue', + code: ` + + ` } ], invalid: [ @@ -532,13 +548,14 @@ tester.run('no-setup-props-reactivity-loss', rule, { code: ` `, errors: [ { message: - 'Destructuring the `props` will cause the value to lose reactivity.', - line: 3 + 'Getting a value from the `props` in root scope of `