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

custom components, v-model and values that are object. #4373

Closed
teleclimber opened this Issue Dec 2, 2016 · 17 comments

Comments

Projects
None yet
@teleclimber

teleclimber commented Dec 2, 2016

I am migrating from Vue 1.x to 2.1.x so I am going through my components and painstakingly replacing all my occurrences of :some_data.sync="a" with v-model="a", and adapting the components to accept a value prop, etc...

My application has lots of complex little input types, so it's very convenient to create a bunch of custom components that each can handle a specific type and nest these all as needed. I was very happy with how Vue helped me do this in 1.x.

The change from .sync to v-model makes sense and I like the idea overall. However most of my custom input components' value is an object, and that does not seem to be well supported by the new model.

In the example in the docs the value is a string, but if you use the same pattern with an object, the behavior can change considerably!

I've lost sight of the long thread where this switch to v-model was discussed but it's clear one of the main reasons to not use .sync is you don't want to change your parent's data in a child component.

But if my value a is an object v-model="a" passes a reference to a to the child, and any changes the child makes to it affect the parent immediately. This defeats the intended abstraction and makes this.$emit( 'input', ... ) redundant window dressing.

So the solution it seems is for the child to make a deep clone of the value prop into a local_val data attribute whenever it changes. Unfortunately this has to be done when component is mounted too, which adds boilerplate. (edit: just found out you can clone this.value right in the data function, so that is a bit cleaner.)

Now when we emit the input event, we attach our cloned local_val object which then becomes the value of value, which then triggers the watch on value and causes our local_val to be replaced by a deep clone of itself. So far that's inefficient but not inherently problematic.

The real problem is now we can't use a watch expression on local_val to know if it's been changed (like say in a sub-component) to trigger this.$emit because you get an infinite loop!

In my case I really wanted to use watch on local_val because it's so much less work than attaching listeners to each of my sub-components (what's the point of v-model if I also have to attach listeners everywhere?) So to prevent infinite loops I have to deep-compare this.value and this.local_val before emitting an input. More boilerplate, more inefficiencies.

So in the end my input custom components each have the following boilerplate:

module.exports = {
	props: ['value'],
	data: function() {
		return {
			local_val: clone( this.value );
		}
	},
	watch: {
		value: {
			handler: function() {
				this.local_val = clone( this.value );
			},
			deep: true
		},
		local_val: {
			handler: function() {
				if( !deepEqual(this.local_val, this.value, {strict:true}) ) {
					this.$emit( 'input', this.local_val );
				}
			},
			deep: true
		}
	},
//...

So my question is: is that the way v-model is intended to work with custom input components that have a value that is an object? Or did I miss something? If I am doing things completely wrong feel free to point me in the right direction.

If so, it seems Vue could do more to help reduce boilerplate and enforce the pattern intended by v-model. Perhaps it could deep clone data sent to child components via v-model, and it could do a deep-comparison of data returned on input event before applying it as a change.

Maybe v-model.deep="a" could trigger these behaviors?

Thanks for reading!

@fnlctrl

This comment has been minimized.

Show comment
Hide comment
@fnlctrl

fnlctrl Dec 3, 2016

Member

Hi, thanks for filling this issue.

But if my value a is an object v-model="a" passes a reference to a to the child, and any changes the child
makes to it affect the parent immediately.

Actually, if you don't want to make it affect parent immediately, you should not be mutating the prop object directly in child. Any change to the prop should be done by $emit('input', <mutated clone>), which is how v-model was designed to be used with objects. - Yes, the parent state gets replaced by a new fresh object.

In your case, using a local cloned value is the right approach. It serves as a temporary state of the child, and it should be flushed by parent state changes triggered by $emit('input', ...)

Therefore, simply removing the $emit inside local_val watcher should do it. Only $emit elsewhere, when you want to update parent state and flush the local state. This way you can still watch local_val and react to changes.

Also, I really think that maybe you should consider breaking the component down if it's value is a deep object - multiple components modifying primitive values / simple objects like {a:1} makes it easier to maintain and reason about.

Member

fnlctrl commented Dec 3, 2016

Hi, thanks for filling this issue.

But if my value a is an object v-model="a" passes a reference to a to the child, and any changes the child
makes to it affect the parent immediately.

Actually, if you don't want to make it affect parent immediately, you should not be mutating the prop object directly in child. Any change to the prop should be done by $emit('input', <mutated clone>), which is how v-model was designed to be used with objects. - Yes, the parent state gets replaced by a new fresh object.

In your case, using a local cloned value is the right approach. It serves as a temporary state of the child, and it should be flushed by parent state changes triggered by $emit('input', ...)

Therefore, simply removing the $emit inside local_val watcher should do it. Only $emit elsewhere, when you want to update parent state and flush the local state. This way you can still watch local_val and react to changes.

Also, I really think that maybe you should consider breaking the component down if it's value is a deep object - multiple components modifying primitive values / simple objects like {a:1} makes it easier to maintain and reason about.

@teleclimber

This comment has been minimized.

Show comment
Hide comment
@teleclimber

teleclimber Dec 4, 2016

I really think that maybe you should consider breaking the component down if it's value is a deep object - multiple components modifying primitive values / simple objects like {a:1} makes it easier to maintain and reason about.

Yes, this is what I do. But I consider that component that holds the primitive values to be input / v-model components as well. And that's how they end up with an object prop that gets mutated.

There are many examples of simple inputs where the value is an object:

  • a distance + units { length:12, unit:'px' }
  • time: { hours:3, minutes:12: seconds: 58 }
  • position: { x:2, y:4 }
  • any picker that can select multiple values (like <select> for that matter)
  • car {make:'volvo', model:'..', year:2015 }
  • and on and on....

I seems I should be able to encapsulate such inputs into a reusable component that I can use as simply as this:
<distance-input v-model="css.padding_top">. Simple and effective and easy to reason about.

The problem here, the issue that I am trying to raise is that Vue js 2 tried to fix an anti-pattern and instead gave us an extremely easy way to do a different anti-pattern.

Here is why:

  1. v-model exists to create input components, right? So the prop passed in v-model is intended to be modified, right? That's the only reason to pass things via v-model.
  2. Because Vue js doesn't use immutable data, and it passes everything by reference, modifying the prop passed is an anti-pattern. ("This means you should not attempt to mutate a prop inside a child component." )
  3. So if I modify the value prop that I got with the intention of modifying it, I'm doing the anti pattern. How does that make sense?

I guess I don't understand why v-model exists without deep-copying the passed value since there is no way to do what is intended to do without deep copying first.

teleclimber commented Dec 4, 2016

I really think that maybe you should consider breaking the component down if it's value is a deep object - multiple components modifying primitive values / simple objects like {a:1} makes it easier to maintain and reason about.

Yes, this is what I do. But I consider that component that holds the primitive values to be input / v-model components as well. And that's how they end up with an object prop that gets mutated.

There are many examples of simple inputs where the value is an object:

  • a distance + units { length:12, unit:'px' }
  • time: { hours:3, minutes:12: seconds: 58 }
  • position: { x:2, y:4 }
  • any picker that can select multiple values (like <select> for that matter)
  • car {make:'volvo', model:'..', year:2015 }
  • and on and on....

I seems I should be able to encapsulate such inputs into a reusable component that I can use as simply as this:
<distance-input v-model="css.padding_top">. Simple and effective and easy to reason about.

The problem here, the issue that I am trying to raise is that Vue js 2 tried to fix an anti-pattern and instead gave us an extremely easy way to do a different anti-pattern.

Here is why:

  1. v-model exists to create input components, right? So the prop passed in v-model is intended to be modified, right? That's the only reason to pass things via v-model.
  2. Because Vue js doesn't use immutable data, and it passes everything by reference, modifying the prop passed is an anti-pattern. ("This means you should not attempt to mutate a prop inside a child component." )
  3. So if I modify the value prop that I got with the intention of modifying it, I'm doing the anti pattern. How does that make sense?

I guess I don't understand why v-model exists without deep-copying the passed value since there is no way to do what is intended to do without deep copying first.

@rhyek

This comment has been minimized.

Show comment
Hide comment
@rhyek

rhyek Dec 16, 2016

The way I look at props is I'm passing an argument by value, not by reference. To me, doing this.value = something (where value is a prop) inside a method, for example, doesn't make sense, but this.value.someProp = someValue does.

Thinking about it as someone who's not privy to the internals of Vue, it seems logical/safe to modify a prop object's property since the parent component and any other component that depends on that same object for its state can react accordingly. But changing the object itself in a child component would imply somehow updating all those references magically.

I would asume this is one of the motivations for the data down/actions up paradigm. I myself probably wouldn't worry about deep cloning a prop object and using $emit for every change.

rhyek commented Dec 16, 2016

The way I look at props is I'm passing an argument by value, not by reference. To me, doing this.value = something (where value is a prop) inside a method, for example, doesn't make sense, but this.value.someProp = someValue does.

Thinking about it as someone who's not privy to the internals of Vue, it seems logical/safe to modify a prop object's property since the parent component and any other component that depends on that same object for its state can react accordingly. But changing the object itself in a child component would imply somehow updating all those references magically.

I would asume this is one of the motivations for the data down/actions up paradigm. I myself probably wouldn't worry about deep cloning a prop object and using $emit for every change.

@marcovc

This comment has been minimized.

Show comment
Hide comment
@marcovc

marcovc Dec 29, 2016

I'm with teleclimber, I've read the docs several times and cannot find my way around this issue.

marcovc commented Dec 29, 2016

I'm with teleclimber, I've read the docs several times and cannot find my way around this issue.

@leevigraham

This comment has been minimized.

Show comment
Hide comment
@leevigraham

leevigraham Feb 5, 2017

@teleclimber I'm guessing clone and deepEqual are npm packages?

leevigraham commented Feb 5, 2017

@teleclimber I'm guessing clone and deepEqual are npm packages?

@yyx990803

This comment has been minimized.

Show comment
Hide comment
@yyx990803

yyx990803 Feb 14, 2017

Member

See https://jsfiddle.net/yyx990803/58kxs8tj/ for an example of how v-model is supposed to work with objects.

Member

yyx990803 commented Feb 14, 2017

See https://jsfiddle.net/yyx990803/58kxs8tj/ for an example of how v-model is supposed to work with objects.

@yyx990803 yyx990803 closed this Feb 14, 2017

@rhyek

This comment has been minimized.

Show comment
Hide comment
@rhyek

rhyek Feb 14, 2017

@yyx990803: Correct me if I'm wrong: I believe using v-model is conceptually akin to passing arguments to a function which has a return value which itself is a result of a calculation based off of those same arguments. v-model is your argument and the component is your function; $emit('input', newValue) is essentially returning the new value to your caller (the parent component).

Of course, not all functions need to return a value. Some can operate as a procedure and simply modify your program's state in some way, possibly even the same function's arguments. This could be the distinction between using v-model or just plain v-bind:value.

Since Javascript uses the call-by-sharing evaluation strategy for function arguments, it is possible to modify arguments' properties inside a function, and have those changes visible outside it without returning anything.

E.g., you wouldn't normally do:

function modify(a) {
  const b = deepCopy(a) // possible bottleneck
  b.someProp = 'someValue'
  return b
}

You'd just do:

function modify(a) {
  a.someProp = 'someValue'
}
modify(someVariable)
someVariable.someProp === 'someValue' // true

I, personally, don't find it necessary to clone my prop and $emit every time I change something about it (which can happen in many ways and many times). It introduces more complexity (since you have to eventually use watchers that handle the input emits in a DRY way and then you start having the problems @teleclimber mentioned), possible performance issues, and there's no benefit to it. IMO, there's not even a benefit of correctness.

Now, the true answer here might be that v-model is probably not appropriate for objects, just for primitive values. For objects, just use v-bind:value.

rhyek commented Feb 14, 2017

@yyx990803: Correct me if I'm wrong: I believe using v-model is conceptually akin to passing arguments to a function which has a return value which itself is a result of a calculation based off of those same arguments. v-model is your argument and the component is your function; $emit('input', newValue) is essentially returning the new value to your caller (the parent component).

Of course, not all functions need to return a value. Some can operate as a procedure and simply modify your program's state in some way, possibly even the same function's arguments. This could be the distinction between using v-model or just plain v-bind:value.

Since Javascript uses the call-by-sharing evaluation strategy for function arguments, it is possible to modify arguments' properties inside a function, and have those changes visible outside it without returning anything.

E.g., you wouldn't normally do:

function modify(a) {
  const b = deepCopy(a) // possible bottleneck
  b.someProp = 'someValue'
  return b
}

You'd just do:

function modify(a) {
  a.someProp = 'someValue'
}
modify(someVariable)
someVariable.someProp === 'someValue' // true

I, personally, don't find it necessary to clone my prop and $emit every time I change something about it (which can happen in many ways and many times). It introduces more complexity (since you have to eventually use watchers that handle the input emits in a DRY way and then you start having the problems @teleclimber mentioned), possible performance issues, and there's no benefit to it. IMO, there's not even a benefit of correctness.

Now, the true answer here might be that v-model is probably not appropriate for objects, just for primitive values. For objects, just use v-bind:value.

@yyx990803

This comment has been minimized.

Show comment
Hide comment
@yyx990803

yyx990803 Feb 14, 2017

Member

@rhyek FYI, if I am writing a function that returns some value, I'd avoid mutating the object passed in if possible. A function that mutates its arguments produces side effects when called and is thus always impure. This makes them harder to reason about:

function one (obj) {
  return {
    ...obj,
    someProp: 'someValue'
  }
}

function two (obj) {
  obj.someProp = 'someValue'
}

var newValue = one(oldValue) // no side effects

two(oldValue) // side effect: oldValue is mutated

The point here is calling two on oldValue can cause unexpected behavior if other parts of your code has logic that depends on oldValue and assumes it has not changed.

As for v-model, the benefits of cloning an object is making the values immutable, similar to primitive values like numbers and strings. Sticking to this practice allows you to reason about all v-model behaviors consistently no matter what the the data type is: the value you pass in never gets mutated, when an update happens, a new value gets sent back and replaces the old value. (This also means you don't need to use deep watchers.) This consistency is a clear benefit to me, making things easier to reason about. Cloning objects are not complex by any means with Object.assign or better spread operators, and Arrays provide immutable methods already. As for performance... v-model updates are very, very low frequency operations compared to other parts of the framework, so it's practically negligible. That said, you are free to ignore the advice from me if you insist :)

Member

yyx990803 commented Feb 14, 2017

@rhyek FYI, if I am writing a function that returns some value, I'd avoid mutating the object passed in if possible. A function that mutates its arguments produces side effects when called and is thus always impure. This makes them harder to reason about:

function one (obj) {
  return {
    ...obj,
    someProp: 'someValue'
  }
}

function two (obj) {
  obj.someProp = 'someValue'
}

var newValue = one(oldValue) // no side effects

two(oldValue) // side effect: oldValue is mutated

The point here is calling two on oldValue can cause unexpected behavior if other parts of your code has logic that depends on oldValue and assumes it has not changed.

As for v-model, the benefits of cloning an object is making the values immutable, similar to primitive values like numbers and strings. Sticking to this practice allows you to reason about all v-model behaviors consistently no matter what the the data type is: the value you pass in never gets mutated, when an update happens, a new value gets sent back and replaces the old value. (This also means you don't need to use deep watchers.) This consistency is a clear benefit to me, making things easier to reason about. Cloning objects are not complex by any means with Object.assign or better spread operators, and Arrays provide immutable methods already. As for performance... v-model updates are very, very low frequency operations compared to other parts of the framework, so it's practically negligible. That said, you are free to ignore the advice from me if you insist :)

@rhyek

This comment has been minimized.

Show comment
Hide comment
@rhyek

rhyek Feb 14, 2017

@yyx990803: I understand your reasoning and believe it to be sound. Only:

The point here is calling two on oldValue can cause unexpected behavior if other parts of your code has logic that depends on oldValue and assumes it has not changed.

This is true, but doesn't really happen in VueJS land, since all my code is always using the newest version of my object anyways.

Sticking to this practice allows you to reason about all v-model behaviors consistently no matter what the the data type is

This is the only real reason for cloning prop objects: cognitive consistency.

Anyways, thank you for your comments.

rhyek commented Feb 14, 2017

@yyx990803: I understand your reasoning and believe it to be sound. Only:

The point here is calling two on oldValue can cause unexpected behavior if other parts of your code has logic that depends on oldValue and assumes it has not changed.

This is true, but doesn't really happen in VueJS land, since all my code is always using the newest version of my object anyways.

Sticking to this practice allows you to reason about all v-model behaviors consistently no matter what the the data type is

This is the only real reason for cloning prop objects: cognitive consistency.

Anyways, thank you for your comments.

@rhyek

This comment has been minimized.

Show comment
Hide comment
@rhyek

rhyek Feb 15, 2017

@yyx990803, thoughts on deep vs shallow copy in this context?

rhyek commented Feb 15, 2017

@yyx990803, thoughts on deep vs shallow copy in this context?

@yyx990803

This comment has been minimized.

Show comment
Hide comment
@yyx990803

yyx990803 Feb 15, 2017

Member

@rhyek I'd consider it a misuse if v-model is used with an object more than 1 levels deep. That essentially becomes shared state. I'd just use a proper state management pattern instead.

Member

yyx990803 commented Feb 15, 2017

@rhyek I'd consider it a misuse if v-model is used with an object more than 1 levels deep. That essentially becomes shared state. I'd just use a proper state management pattern instead.

@leevigraham

This comment has been minimized.

Show comment
Hide comment
@leevigraham

leevigraham Feb 16, 2017

I'd consider it a misuse if v-model is used with an object more than 1 levels deep. That essentially becomes shared state. I'd just use a proper state management pattern instead.

@yyx990803 What about in the case of a date / time multiple select box.

image

I this scenario I see two sub-components, one for date and one for time. You could even take this a step further and say each of the select boxes is a <vue-select> custom component.

How would you handle this situation?

leevigraham commented Feb 16, 2017

I'd consider it a misuse if v-model is used with an object more than 1 levels deep. That essentially becomes shared state. I'd just use a proper state management pattern instead.

@yyx990803 What about in the case of a date / time multiple select box.

image

I this scenario I see two sub-components, one for date and one for time. You could even take this a step further and say each of the select boxes is a <vue-select> custom component.

How would you handle this situation?

@fenduru

This comment has been minimized.

Show comment
Hide comment
@fenduru

fenduru Feb 27, 2017

Contributor

@yyx990803 I'm also interested in what the recommended pattern is here. I work on a component library, and it is very common to compose reusable components into other (more complex) reusable components.

Contributor

fenduru commented Feb 27, 2017

@yyx990803 I'm also interested in what the recommended pattern is here. I work on a component library, and it is very common to compose reusable components into other (more complex) reusable components.

@fenduru

This comment has been minimized.

Show comment
Hide comment
@fenduru

fenduru Feb 27, 2017

Contributor

@leevigraham I don't think that is related to components with objects as their value. If I'm reading it correctly it simply allows customizing the property and event name.

Contributor

fenduru commented Feb 27, 2017

@leevigraham I don't think that is related to components with objects as their value. If I'm reading it correctly it simply allows customizing the property and event name.

@PickerU

This comment has been minimized.

Show comment
Hide comment
@PickerU

PickerU Apr 24, 2017

How can I use v-model for two level deep nested component?
e.g.
HTML
here day is an object
<opening-hr-field v-model="day"> </opening-hr-field>

JS template

<template type="text/x-template" id="opening-hr-field-template">
   <div>
     <input type="checkbox" v-model="value.is24Open"> 24 hour
     <time-select v-model = "value.startTime"></time-select>
   </div>
</template>

<template  type="text/x-template" id="time-select-template">
  <select :value="value" 
      v-on:input="$emit('input', $event.target.value)">
      <option v-for="t in getHours()">
        {{ t }}
      </option>
  </select>
</template>

Here, I have two level deep v-model. Can you please check whether it's right way to do? How can I propagate the emit from 2nd template to first template and all way up to the parent? 

PickerU commented Apr 24, 2017

How can I use v-model for two level deep nested component?
e.g.
HTML
here day is an object
<opening-hr-field v-model="day"> </opening-hr-field>

JS template

<template type="text/x-template" id="opening-hr-field-template">
   <div>
     <input type="checkbox" v-model="value.is24Open"> 24 hour
     <time-select v-model = "value.startTime"></time-select>
   </div>
</template>

<template  type="text/x-template" id="time-select-template">
  <select :value="value" 
      v-on:input="$emit('input', $event.target.value)">
      <option v-for="t in getHours()">
        {{ t }}
      </option>
  </select>
</template>

Here, I have two level deep v-model. Can you please check whether it's right way to do? How can I propagate the emit from 2nd template to first template and all way up to the parent? 
@theoephraim

This comment has been minimized.

Show comment
Hide comment
@theoephraim

theoephraim Dec 29, 2017

This seems like a common enough use case to justify some kind of built-in helper?

theoephraim commented Dec 29, 2017

This seems like a common enough use case to justify some kind of built-in helper?

@chriscdn

This comment has been minimized.

Show comment
Hide comment
@chriscdn

chriscdn Apr 23, 2018

Google brought me to this thread while searching for a better understand of the props/emitter pattern. I was particularly unsure about emitting an event using v-model from a deeply nested component to the top. I think this might be the cleanest solution I found:

// MyComponent.vue
export default {
	props: ['value'],
	computed: {
		localValue: {
			get: function() {
				return this.value
			},
			set: function(value) {
				this.$emit('input', value);
			}
		}
	}
});

This can be used from the parent as follows (where whatever could also be a computed property defined in the same manner above):

<MyComponent v-model="whatever"></MyComponent>

Within the component all other setter and getter operations should use the localValue computed property instead of the value prop. This makes updating and emitting a change to the parent easy:

this.localValue = 'new value'

If value is an object then it might be worthwhile creating computed properties for those as well. For example, if value represented a "person" with firstName and lastName properties:

// MyComponent.vue
export default {
	props: ['value'],
	computed: {
		localValue: {
			get: function() {
				return this.value
			},
			set: function(value) {
				this.$emit('input', value);
			}
		},
		firstName: {
			get: function() {
				return this.localValue.firstName;
			},
			set: function(value) {
				this.propertySetter('firstName', value);
			}
		}
	},
	methods: {
		// mixin?
		propertySetter: function(key, value) {
			this.localValue = Object.assign({}, this.localValue, {[key]:value});
		}
	}
});

This allows firstName and lastName to be used with v-model from MyComponent.vue, and have changes propagate all the way up. E.g.,

<input type="text" v-model="firstName" />

I'm still experimenting with this but it seems to be the most consistent approach I could find.

Here's a codepen.

chriscdn commented Apr 23, 2018

Google brought me to this thread while searching for a better understand of the props/emitter pattern. I was particularly unsure about emitting an event using v-model from a deeply nested component to the top. I think this might be the cleanest solution I found:

// MyComponent.vue
export default {
	props: ['value'],
	computed: {
		localValue: {
			get: function() {
				return this.value
			},
			set: function(value) {
				this.$emit('input', value);
			}
		}
	}
});

This can be used from the parent as follows (where whatever could also be a computed property defined in the same manner above):

<MyComponent v-model="whatever"></MyComponent>

Within the component all other setter and getter operations should use the localValue computed property instead of the value prop. This makes updating and emitting a change to the parent easy:

this.localValue = 'new value'

If value is an object then it might be worthwhile creating computed properties for those as well. For example, if value represented a "person" with firstName and lastName properties:

// MyComponent.vue
export default {
	props: ['value'],
	computed: {
		localValue: {
			get: function() {
				return this.value
			},
			set: function(value) {
				this.$emit('input', value);
			}
		},
		firstName: {
			get: function() {
				return this.localValue.firstName;
			},
			set: function(value) {
				this.propertySetter('firstName', value);
			}
		}
	},
	methods: {
		// mixin?
		propertySetter: function(key, value) {
			this.localValue = Object.assign({}, this.localValue, {[key]:value});
		}
	}
});

This allows firstName and lastName to be used with v-model from MyComponent.vue, and have changes propagate all the way up. E.g.,

<input type="text" v-model="firstName" />

I'm still experimenting with this but it seems to be the most consistent approach I could find.

Here's a codepen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment