Is mutating object props bad practice?

Vuex doesnt solve this problem we’re talking about here, especially if we are dealing with simple dumb components, where its the parent componetn that should be commiting the results to Vuex in the end.

You generally yes, would have to clone your object, use the cloned object as local state to that component, emit your changes upwards.

There are other techniques you could employ that Linus mentions but he may have to explain a little bit more on the topic.

In my opinion this answers your question. Your code is simple and it does exactly what you want.

I am doing exactly the same thing in the app I am actually working on. Otherwise my code would be much more complex, worse readable/maintainable etc.

Perhaps this is wrong for bigger applications, but my opinion is: It keeps my app simple, so I do it like this.

I read this several times. But I still don’t understand why this should be the case. To me it sounds very straightforward to have one source of data and to manipulate it where needed. Probably I don’t see the problems since I’m quite new to Vue and webdevelopment and hence the apps I were working on until now are not too big of course.

2 Likes

I’ve been thinking about the question again, because I had the same question while watching a tutorial and I don’t want to get used to bad practices :slight_smile:

In the case of the app I am/was actually working on, there is some data that has to be displayed in different ways/views. One of those views can change the data, the other views only have very limited access of changing data. In this case I would still say that it’s absolutely fine to have one object, that is passed from parent to child as prop and thereafter modified in the child.

Nevertheless I would now agree that it can be bad practice, e.g. if:

  • this would result in a “god-object” that holds all the data, especially if the data doesn’t logically belong to one object.
  • this causes bad encapsulation of data
  • the different components have fairly independent data, hence the components should be decoupled as much as possible

There are other things that could be mentioned for sure. But for me this is enough to come to the conclusion:

It’s fine (for me) to pass objects as props and to change them in the child in simple use cases. In more complex uses cases I should indeed think of using events or callbacks.

2 Likes

Thanks for the thoughtful reply, Tomka. I think I’ll stick with what I have for the moment, then, unless an easy and easily-maintained alternative comes to mind. I looked at the documentation again and noted this, talking about one-way data flow: “This prevents child components from accidentally mutating the parent’s state, which can make your app’s data flow harder to reason about.” The key point for me is that I very deliberately want to mutate the parent’s state, there’s nothing accidental about it. I have a shared object, changes to which I want reflected wherever it’s used.

I’m still tempted to go with cloning the prop and emitting a changed event, simply because it’s the ‘correct’ way, but I don’t want to make this more complex than it need be.

You can use the sync modifier to do fairly easy and easily-maintained editor components.

This has the added benefit that your component will have a more explicit interface via props declarations.

1 Like

That’s very interesting. The sync modifier has been (re)introduced since I first did the code I’m talking about, but it certainly looks relevant. I have to say that I don’t really understand how your example works, though. You are passing ‘customer’ as a prop to the Editor component, but in the Editor component itself you specify ‘name’ and ‘color’ as the props. Does the use of the ‘sync’ modifier mean that the prop is known to be an object by the child component, and thus that the props are fields of that object?

No, I am binding an object of attributes.

v-bind="customer"

vs

v-bind:customer="customer"

The sync modifier is separate:

<editor v-bind="customer"></editor>
<!-- is the same as -->
<editor v-bind:name="customer.name" v-bind:color="customer.color"></editor

and

<editor v-bind.sync="customer"></editor>
<!-- is the same as -->
<editor v-bind:name.sync="customer.name" v-bind:color.sync="customer.color"></editor
1 Like

Sorry, yes, that should have been obvious!

I have this problem now too. Maybe is there the use of a global event bus an elegant solution ?

I’ve run into some trouble with this approach.
I have an editor-like component for an entity and an object in data for all the entity properties.
Then there is a ‘save’ button to finish editing and pass the data to the parent component and reset an editor to the blank state. So I use a method to emit an event with entity object and in the next lines I’m changing this object’s properties. This results that parent component receives the object with changed properties.
What would be the best practice to prevent this?

I think cloning the object before emitting it would work. Something like this should do it: JSON.parse(JSON.stringify(obj))

Note: This post was created on 20-01-2019. The code excerpts refer to a jsfiddle as it was posted on 11-01-2019. However, a new revision of the jsfiddle has been posted on 20-02-2019, with significant changes in the addChild method. This might indicate an effort to discourage the discussed practice.

Judging by the official Vue example of a Tree View component, this practice is not entirely discouraged. See the code at https://jsfiddle.net/chrisvfritz/pnqzspoe/12174 or the code at github. In particular:

props: {
  model: Object
},

and

methods: {
    /* ... */
    addChild: function () {
      this.model.children.push({
        name: 'new stuff'
      })
    }
},

That is, there is a single property (a “prop”) named model whose type is Object. The prop is directly “manipulated” by the addChild method which changes the value of model.children.length.

2 Likes

I deal with the cloning data like you:grinning:

this practice is not entirely discouraged

I think so, too. Without the ability to mutate object props, it would be hard to do something like
grafik
i.e. arbitrary nested dynamic forms.

Source-Code: https://github.com/drahkrub/props-as-data
CodeSandbox: https://codesandbox.io/s/0y7jw79ljn
And: Arbitrary nested dynamic forms (still unclear: mutating object props - bad or good/ok)

Any Feedback is welcome!

It is interesting, how vuex can recognise its object mutations, while vue can’t recognise mutations of a nested value of an props object.

Oh it can certainly detect the mutation. The thing it can’t do rigjht now is detect that this object was mutated in the context of being received as a prop.

While technicallypossible the necessary tracking just for the warning wouldn’t be worth the implementation cost.

Let us say I have a component with a large form with 20 input fields.

I can refactor this into 4 subcomponents with 5 fields each and pass them each the same model object so they can all modify separate keys in the same object passed in as a prop.

And I don’t need an emit methid.

In what situation can this go wrong?

1 Like

I do something like this without mutation:

In parent component:
html:

  <Location
    v-model="location"
  />

js:

  data () {
    return {
      location: {
        address: '',
        latitude: '',
        longitude: ''
      }
    }
  }

In children component:
html:

    <input
      :value="value.address"
      @input="handleInput('address', $event)"
    />
    <input
      :value="value.latitude"
      @input="handleInput('latitude', $event)"
    />
    <input
      :value="value.longitude"
      @input="handleInput('longitude', $event)"
    />

js:

  props: {
    value: {
      type: Object,
      required: true
    }
  },

  methods: {
    handleInput (key, value) {
      const payload = Object.assign({}, this.value)
      payload[key] = value
      this.$emit('input', payload)
    }
  }

I used to manipulate prop object properties directly too, but I found quite painless and clean methods to adhere to the Vue dogma’s of props down events up. Basically, you can create a computed property with a get/setter, of which the getter just returns the prop value, and the setter emits the update event, e.g.:

setup (props, { emit } ) {
  const localDescription = computed({
    get () {
       return props.description
    },
    set(val) {
       emit('update:description', val)
    }
  })
  return { localDescription }
}

(I am using composition API here, but the options API supports the same idea). This way, if you manipulate localDescription (e.g. by passing at as a v-model argument), it just emits the update event if it is changed, and also nicely changes if the underlying prop is changed. You can even extract this function to a generic solution:

// propsync.ts
import { computed } from '@vue/composition-api'
type EmitFunc = (event: string, ...args: any[]) => void

export function useSync<P extends {}> (props: P, emit: EmitFunc) {
  return <K extends keyof P> (field: K) => computed({
    get () {
      return props[field]
    },
    set (val: P[K]): void {
      emit(`update:${field}`, val)
    }
  })
}

and then use it in the component as:

import useSync from './propsync'
export default defineComponent({
  props: {
    description: {
      type: String,
      default: ''
    }
  },
  setup (props, { emit }) {
    const propSync = useSync(props, emit)
    const localDescription = sync('description')
    return { localDescription }
  })
})

This also places nice with the v-bind.sync="objectToSync" solutions provided above. It only doesn’t play nice if the synced properties are non-primitives (e.g. objects/arrays), because they are passed by reference, and thus still the props are changed directly if their properties are touched. Also, this method only detects reassignments of the variable and not changes to the properties of the reactive object. So something like

someArray.value.splice(0, 1, 'new1')

is out of the question, and needs a more ‘immutable’ approach:

 const replaceItems = <T extends any, A extends T[]>(arr: Ref<A>, pos: number, ...items: T[]): void => {
    const arrCopy = cloneDeep(arr.value)
    arrCopy.splice(pos, items.length, ...items)
    arr.value = arrCopy
}
replaceItems(someArray, 0, 'newItem1', 'newItem2')

I don’t know if this is a performance killer, and I yet have yet to find a more elegant solution to this.

Old topic but I will revive it.

I’m working with vue for 3-4 years, but still “fighting” against this practice. Especially when creating some calculators where data is nested and very related. It’s just impossible to make root/parent component responsible for every change. I could use vuex, but when data is related only with single component three and nowhere else - why should I? Instead, I could build my own data object, and let it to be modified through methods (like vuex mutators, to avoid warnings) from anywhere. But this is somehow considered as a bad practice. :smiley: