Is mutating object props bad practice?

I’ve been working with Vue for a little while now but there are still some basic things I don’t think I’ve fully sorted out in my head yet. One is to do with the issue of two-way data communication between parent and child components.

Where I’m getting confused over the correct approach is in a CRUD application, with listing and detail screens. I have a list of customers. When the user double clicks on one, the list is hidden and a data entry form is displayed in its place, with the details of the customer selected. So for this, I have a parent CustomerManager component, with ‘customer’ as one of its data fields, and two child components, CustomerList and CustomerDetail. Double-clicking on a row in CustomerList sets the values of the ‘customer’ object in CustomerManager. This is used as a prop in CustomerDetail, thus:

<customer-detail v-if="editMode==true" :customer="customer"></customer-detail>

Now, I’m aware that I shouldn’t directly mutate props in child objects, and if the customer prop for CustomerDetail were e.g., a string, I would get a warning when I attempted to do so. Because it’s an object in this case, though, I can mutate it without warning in the CustomerDetail component, and I can see the changes taking effect in the parent CustomerManager.

Is this bad practice? It achieves exactly what I want to achieve in as simple a way as I can. The customer object is effectively shared between components, and I do want it to be updated throughout when I make a change to it in one of the components.

The alternative, as I understand it, is cloning the prop as a data field in CustomerDetail (simply copying it to a data field from a prop doesn’t do anything as both prop and data versions would be references to the same object), using the cloned prop in the child component and when it is changed, doing an emit with the new value which is then picked up by a handler in the CustomerManager object, which duly updates the parent object. This seems to me to be rather cumbersome and longwinded and doesn’t offer me any compensating advantages.

What would others do in this situation?

2 Likes

Since you’re not modifying a prop) directly, but infact, a nested value in an object, Vue doesnt detect that thus you do not see the warning message. But the principle still applies.

Generally you shouldnt do that. This can / could cause unexpected bugs in your code. Generally you should make a copy of your object, modify that, then emit upwards to the parent (as you have mentioned in your last paragraph I believe). This is the method I take and is the general consensus of top down, emit up approach.

1 Like

While I agree with bolerodan generally, note that cloning the object is not really necessary. You could alps emu an event with property name and new value to do The mutation in the parent without ant cloning involved.

That being said, migrating a prop object like this is not advised, but doesn’t have any real imminent risk in simple components.

We just learned from experience that its usually best to have all mutations happen in the parent helps with concisteny and maintainablility in the longrun.

1 Like

But my point is that, because the prop is an object, if I don’t clone it and just do something like this instead in data:

data () {
  return {
    _customer: customer
 }
}

and work on _customer locally, any changes I make to it are still going to directly mutate the object passed in as prop, as _customer and customer point to the same object. So I have to actually clone it and then work on the cloned object if I want to avoid mutating the parent object. Maybe I’m not understanding what you’re saying?

In some ways the answer I should be looking at for this is actually using a Vuex store, I suppose, where I mutate and retrieve a ‘customer’ in the store. It seems overkill, though.

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.