Skip to content

Commit

Permalink
Add support for props destructure to `vue/no-setup-props-reactivity-l…
Browse files Browse the repository at this point in the history
…oss` rule (#2550)
  • Loading branch information
ota-meshi authored Sep 18, 2024
1 parent e8a09e9 commit 2dc606c
Show file tree
Hide file tree
Showing 3 changed files with 155 additions and 81 deletions.
200 changes: 136 additions & 64 deletions lib/rules/no-setup-props-reactivity-loss.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<PropId>}
*/
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',
Expand All @@ -31,7 +85,9 @@ module.exports = {
create(context) {
/**
* @typedef {object} ScopePropsReferences
* @property {Set<Identifier>} refs
* @property {object} refs
* @property {Set<Identifier>} refs.props A set of references to container objects with multiple props.
* @property {Set<Identifier>} refs.prop A set of references a variable with one property.
* @property {string} scopeName
*/
/** @type {Map<FunctionDeclaration | FunctionExpression | ArrowFunctionExpression | Program, ScopePropsReferences>} */
Expand Down Expand Up @@ -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
}

/**
Expand All @@ -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' ||
Expand All @@ -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) {
Expand All @@ -189,11 +255,8 @@ module.exports = {

propsReferenceIds.add(reference.identifier)
}
setupScopePropsReferenceIds.set(scopeNode, {
refs: propsReferenceIds,
scopeName
})
}

return utils.compositingVisitors(
{
/**
Expand Down Expand Up @@ -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,
'<script setup>'
)
for (const propId of iteratePropIds(id)) {
processPropId(
propId,
context.getSourceCode().ast,
currentScope,
'<script setup>'
)
}
}
}),
utils.defineVueVisitor(context, {
onSetupFunctionEnter(node) {
const currentScope = utils.getScope(context, node)
const propsParam = utils.skipDefaultParamValue(node.params[0])
processPattern(propsParam, node, currentScope, 'setup()')
if (!propsParam) return
processPropId(
{ node: propsParam, kind: 'props' },
node,
currentScope,
'setup()'
)
}
})
)
Expand Down
15 changes: 0 additions & 15 deletions tests/lib/rules/no-setup-props-destructure.js
Original file line number Diff line number Diff line change
Expand Up @@ -504,21 +504,6 @@ tester.run('no-setup-props-destructure', rule, {
}
]
},
{
filename: 'test.vue',
code: `
<script setup>
const {count} = defineProps({count:Number})
</script>
`,
errors: [
{
message:
'Destructuring the `props` will cause the value to lose reactivity.',
line: 3
}
]
},
{
filename: 'test.vue',
code: `
Expand Down
21 changes: 19 additions & 2 deletions tests/lib/rules/no-setup-props-reactivity-loss.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,22 @@ tester.run('no-setup-props-reactivity-loss', rule, {
const count = computed(() => props.count)
</script>
`
},
{
filename: 'test.vue',
code: `
<script setup>
const {count} = defineProps({count:Number})
</script>
`
},
{
filename: 'test.vue',
code: `
<script setup>
const { foo = 1, bar = 'ok' } = defineProps({ foo: Number, bar: String })
</script>
`
}
],
invalid: [
Expand Down Expand Up @@ -532,13 +548,14 @@ tester.run('no-setup-props-reactivity-loss', rule, {
code: `
<script setup>
const {count} = defineProps({count:Number})
const foo = count
</script>
`,
errors: [
{
message:
'Destructuring the `props` will cause the value to lose reactivity.',
line: 3
'Getting a value from the `props` in root scope of `<script setup>` will cause the value to lose reactivity.',
line: 4
}
]
},
Expand Down

0 comments on commit 2dc606c

Please sign in to comment.