Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add shallowOnly option to vue/no-mutating-props #2135

Merged
merged 6 commits into from May 10, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
72 changes: 71 additions & 1 deletion docs/rules/no-mutating-props.md
Expand Up @@ -22,6 +22,8 @@ This rule reports mutation of component props.
<template>
<div>
<input v-model="value" @click="openModal">
<button @click="pushItem">Push Item</button>
<button @click="changeId">Change ID</button>
</div>
</template>
<script>
Expand All @@ -30,11 +32,25 @@ This rule reports mutation of component props.
value: {
type: String,
required: true
},
list: {
type: Array,
required: true
},
user: {
type: Object,
required: true
}
},
methods: {
openModal() {
this.value = 'test'
},
pushItem() {
this.list.push(0)
},
changeId() {
this.user.id = 1
}
}
}
Expand All @@ -50,6 +66,8 @@ This rule reports mutation of component props.
<template>
<div>
<input :value="value" @input="$emit('input', $event.target.value)" @click="openModal">
<button @click="pushItem">Push Item</button>
<button @click="changeId">Change ID</button>
</div>
</template>
<script>
Expand All @@ -58,11 +76,25 @@ This rule reports mutation of component props.
value: {
type: String,
required: true
},
list: {
type: Array,
required: true
},
user: {
type: Object,
required: true
}
},
methods: {
openModal() {
this.$emit('input', 'test')
},
pushItem() {
this.$emit('push', 0)
},
changeId() {
this.$emit('change-id', 1)
}
}
}
Expand All @@ -88,7 +120,45 @@ This rule reports mutation of component props.

## :wrench: Options

Nothing.
```json
{
"vue/no-mutating-props": ["error", {
"shallowOnly": false
}]
}
```

- "shallowOnly" (`boolean`) Enables mutating the value of a prop but leaving the reference the same. Default is `true`.
FloEdelmann marked this conversation as resolved.
Show resolved Hide resolved

### "shallowOnly": true

<eslint-code-block :rules="{'vue/no-mutating-props': ['error', {shallowOnly: true}]}">

```vue
<!-- ✓ GOOD -->
<template>
<div>
<input v-model="value.id" @click="openModal">
</div>
</template>
<script>
export default {
props: {
value: {
type: Object,
required: true
}
},
methods: {
openModal() {
this.value.visible = true
}
}
}
</script>
```

</eslint-code-block>

## :books: Further Reading

Expand Down
146 changes: 103 additions & 43 deletions lib/rules/no-mutating-props.js
Expand Up @@ -4,6 +4,10 @@
*/
'use strict'

/**
* @typedef {{name?: string, set: Set<string>}} PropsInfo
*/

const utils = require('../utils')
const { findVariable } = require('@eslint-community/eslint-utils')

Expand Down Expand Up @@ -84,6 +88,19 @@ function isVmReference(node) {
return false
}

/**
* @param { object } options
* @param { boolean } options.shallowOnly Enables mutating the value of a prop but leaving the reference the same
*/
function parseOptions(options) {
return Object.assign(
{
shallowOnly: false
},
options
)
}

module.exports = {
meta: {
type: 'suggestion',
Expand All @@ -94,12 +111,21 @@ module.exports = {
},
fixable: null, // or "code" or "whitespace"
schema: [
// fill in your schema
{
type: 'object',
properties: {
shallowOnly: {
type: 'boolean'
}
},
additionalProperties: false
}
]
},
/** @param {RuleContext} context */
create(context) {
/** @type {Map<ObjectExpression|CallExpression, Set<string>>} */
const { shallowOnly } = parseOptions(context.options[0])
/** @type {Map<ObjectExpression|CallExpression, PropsInfo>} */
const propsMap = new Map()
/** @type { { type: 'export' | 'mark' | 'definition', object: ObjectExpression } | { type: 'setup', object: CallExpression } | null } */
let vueObjectData = null
Expand Down Expand Up @@ -138,10 +164,11 @@ module.exports = {
/**
* @param {MemberExpression|Identifier} props
* @param {string} name
* @param {boolean} isRootProps
*/
function verifyMutating(props, name) {
function verifyMutating(props, name, isRootProps = false) {
const invalid = utils.findMutating(props)
if (invalid) {
if (invalid && isShallowOnlyInvalid(invalid, isRootProps)) {
report(invalid.node, name)
}
}
Expand Down Expand Up @@ -210,6 +237,9 @@ module.exports = {
continue
}
let name
if (!isShallowOnlyInvalid(invalid, path.length === 0)) {
continue
}
if (path.length === 0) {
if (invalid.pathNodes.length === 0) {
continue
Expand Down Expand Up @@ -246,26 +276,43 @@ module.exports = {
}
}

/**
* Is shallowOnly false or the prop reassigned
* @param {Exclude<ReturnType<typeof utils.findMutating>, null>} invalid
* @param {boolean} isRootProps
* @return {boolean}
*/
function isShallowOnlyInvalid(invalid, isRootProps) {
return (
!shallowOnly ||
(invalid.pathNodes.length === (isRootProps ? 1 : 0) &&
['assignment', 'update'].includes(invalid.kind))
)
}

return utils.compositingVisitors(
{},
utils.defineScriptSetupVisitor(context, {
onDefinePropsEnter(node, props) {
const defineVariableNames = new Set(extractDefineVariableNames())

const propsSet = new Set(
props
.map((p) => p.propName)
.filter(
/**
* @returns {propName is string}
*/
(propName) =>
utils.isDef(propName) &&
!GLOBALS_WHITE_LISTED.has(propName) &&
!defineVariableNames.has(propName)
)
)
propsMap.set(node, propsSet)
const propsInfo = {
name: '',
set: new Set(
props
.map((p) => p.propName)
.filter(
/**
* @returns {propName is string}
*/
(propName) =>
utils.isDef(propName) &&
!GLOBALS_WHITE_LISTED.has(propName) &&
!defineVariableNames.has(propName)
)
)
}
propsMap.set(node, propsInfo)
vueObjectData = {
type: 'setup',
object: node
Expand Down Expand Up @@ -294,22 +341,25 @@ module.exports = {
target.parent.id,
[]
)) {
if (path.length === 0) {
propsInfo.name = prop.name
} else {
propsInfo.set.add(prop.name)
}
verifyPropVariable(prop, path)
propsSet.add(prop.name)
}
}
}),
utils.defineVueVisitor(context, {
onVueObjectEnter(node) {
propsMap.set(
node,
new Set(
propsMap.set(node, {
set: new Set(
utils
.getComponentPropsFromOptions(node)
.map((p) => p.propName)
.filter(utils.isDef)
)
)
})
},
onVueObjectExit(node, { type }) {
if (
Expand Down Expand Up @@ -359,7 +409,7 @@ module.exports = {
const name = utils.getStaticPropertyName(mem)
if (
name &&
/** @type {Set<string>} */ (propsMap.get(vueNode)).has(name)
/** @type {PropsInfo} */ (propsMap.get(vueNode)).set.has(name)
) {
verifyMutating(mem, name)
}
Expand All @@ -378,9 +428,9 @@ module.exports = {
const name = utils.getStaticPropertyName(mem)
if (
name &&
/** @type {Set<string>} */ (propsMap.get(vueObjectData.object)).has(
name
)
/** @type {PropsInfo} */ (
propsMap.get(vueObjectData.object)
).set.has(name)
) {
verifyMutating(mem, name)
}
Expand All @@ -393,14 +443,19 @@ module.exports = {
if (!isVmReference(node)) {
return
}
const name = node.name
if (
name &&
/** @type {Set<string>} */ (propsMap.get(vueObjectData.object)).has(
name
)
) {
verifyMutating(node, name)
const propsInfo = /** @type {PropsInfo} */ (
propsMap.get(vueObjectData.object)
)
const isRootProps = !!node.name && propsInfo.name === node.name
const parent = node.parent
const parentProperty =
parent.type === 'MemberExpression' ? parent.property : null
const name =
isRootProps && parentProperty?.type === 'Identifier'
? parentProperty.name
: node.name
FlareZh marked this conversation as resolved.
Show resolved Hide resolved
if (name && (propsInfo.set.has(name) || isRootProps)) {
verifyMutating(node, name, isRootProps)
}
},
/** @param {ESNode} node */
Expand All @@ -423,12 +478,22 @@ module.exports = {
return
}

const propsInfo = /** @type {PropsInfo} */ (
propsMap.get(vueObjectData.object)
)

const nodes = utils.getMemberChaining(node)
const first = nodes[0]
let name
if (isVmReference(first)) {
if (isVmReference(first) && first.name !== propsInfo.name) {
ota-meshi marked this conversation as resolved.
Show resolved Hide resolved
if (shallowOnly && nodes.length > 1) {
return
}
name = first.name
} else if (first.type === 'ThisExpression') {
} else if (first.type === 'ThisExpression' || isVmReference(first)) {
if (shallowOnly && nodes.length > 2) {
return
}
const mem = nodes[1]
if (!mem) {
return
Expand All @@ -437,12 +502,7 @@ module.exports = {
} else {
return
}
if (
name &&
/** @type {Set<string>} */ (propsMap.get(vueObjectData.object)).has(
name
)
) {
if (name && propsInfo.set.has(name)) {
report(node, name)
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/utils/index.js
Expand Up @@ -1778,7 +1778,7 @@ module.exports = {

/**
* @param {MemberExpression|Identifier} props
* @returns { { kind: 'assignment' | 'update' | 'call' , node: ESNode, pathNodes: MemberExpression[] } | null }
* @returns { { kind: 'assignment' | 'update' | 'call' | 'unary', node: ESNode, pathNodes: MemberExpression[] } | null }
*/
findMutating(props) {
/** @type {MemberExpression[]} */
Expand Down Expand Up @@ -1810,7 +1810,7 @@ module.exports = {
case 'UnaryExpression': {
if (target.operator === 'delete') {
return {
kind: 'update',
kind: 'unary',
node: target,
pathNodes
}
Expand Down