8000 Rule proposal: `no-mutating-props` · Issue #256 · vuejs/eslint-plugin-vue · GitHub
[go: up one dir, main page]
Skip to content

Rule proposal: no-mutating-props #256

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.

Sign up for GitHub

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

Jump to bottom
Closed
michalsnik opened this issue Nov 26, 2017 · 4 comments · Fixed by #1148
Closed

Rule proposal: no-mutating-props #256

michalsnik opened this issue Nov 26, 2017 · 4 comments · Fixed by #1148
Assignees
@armano2
Labels
new rule proposition Present in Style Guide work in progress

Comments

@michalsnik
Copy link
Member

Style guide:
https://vuejs.org/v2/style-guide/#Implicit-parent-child-communication-use-with-caution

Description:
This rule would disallow mutating props. It's yet to be defined as there are many cases to handle, and I'm not yet sure it's 100% doable. Of course we should forbid both template-related mutations (as v-model) as well as direct mutations in script (as this.someProp.filter(...)).
@michalsnik michalsnik mentioned this issue Nov 26, 2017
Closed
@chrisvfritz
Copy link
Contributor

This sounds great. 🙂 @michalsnik Anything I can provide to make developing this easier?

@michalsnik
8000 Copy link
Member Author
michalsnik commented Aug 4, 2018 •
edited
Loading

Alright, so these are the scenarios I think we might cover:

1. v-model

Bad:

<template>
  <div>
    <input v-model="todo.text">
  </div>
</template>
<script>
  export default {
      props: {
        todo: {
          type: Object,
          required: true
        }
    }
  }
</script>

Good:

<template>
  <div>
    <input
      :value="todo.text"
      @input="$emit('input', $event.target.value)"
    >
  </div>
</template>
<script>
  export default {
      props: {
        todo: {
          type: Object,
          required: true
        }
    }
  }
</script>

2. mutations in JS

Bad:

  export default {
      props: {
        todo: {
          type: Object,
          required: true
        },
        items: {
          type: Array,
          default: []
        }
    },
    methods: {
      openModal() {
        this.todo.type = 'completed' // mutation
        this.items.push('something') // mutation
      }
    }
  }

Good:

  export default {
      props: {
        todo: {
          type: Object,
          required: true
        },
        items: {
          type: Array,
          default: []
        }
    },
    methods: {
      openModal() {
        this.$emit('someEvent', this.todo)
        const a = this.items.slice(0).push('something') // no mutation because of `slice(0)`
      }
    }
  }

Here we could use similar logic that we've already implemented in no-side-effects-in-computed-properties to catch possible mutations.

3. Mutations inside VExpressionContainer in <template>

Same as with script we might detect the following cases:

  • VExpressionContainer > AssignmentExpression
<div v-if="someProp = [1, 2] && someProp"></div>
  • VExpressionContainer > UpdateExpression
<div v-if="someProp++ && someProp < 10"></div>
  • VExpressionContainer > CallExpression
<!-- Bad -->
<div v-text="items.shift()"></div>

<!-- Good -->
<div v-text="items.slice(0).shift()"></div>

What do you think @chrisvfritz ? Do you see any other cases we might want or not want to cover?

@chrisvfritz
Copy link
Contributor

Nothing more is coming to me at the moment. 🙂

@armano2
Copy link
Contributor
armano2 commented Oct 19, 2018

@michalsnik i can take this one

@armano2 armano2 mentioned this issue Nov 7, 2018
Closed
armano2 added a commit to armano2/eslint-plugin-vue that referenced this issue Nov 11, 2018
@armano2
⭐️New: Add component-definition-name-casing rule. vuejs#256
5ed9a17 
this rule replaces `name-property-casing`
fix issue: vuejs#251
@armano2 armano2 self-assigned this Nov 28, 2018
@ota-meshi ota-meshi mentioned this issue May 20, 2020
Merged
@mgibas mgibas mentioned this issue Feb 15, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@armano2 armano2

Labels
new rule proposition Present in Style Guide work in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add vue/no-mutating-props rule (from #633) vuejs/eslint-plugin-vue
3 participants
@armano2 @chrisvfritz @michalsnik
0