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

Replace v-bind's .sync with a v-model argument #8

Merged

Conversation

chrisvfritz
Copy link
Contributor

@chrisvfritz chrisvfritz commented Jan 28, 2019

@yyx990803 yyx990803 added breaking change This RFC contains breaking changes or deprecations of old API. core 3.x This RFC only targets 3.0 and above labels Jan 28, 2019
@zaun
Copy link

zaun commented Jan 29, 2019

I'm just an end user of Vue so my options probably aren't quite as valid as others, but here's my 2 cents.

I didn't know v-bind.sync existed after about a year of using vue. I'm all fo removing it, v-bind as always been one-way for me, v-model has always been two-way. Having a two-way version of bind isn't intuitive for me at all.

As for spread support, I've never used it in v-bind to begin with. To me it makes the code much less readable. I've always used multiple v-binds (shorthand :). I would have no problem not supporting spread on v-model.

Though I have to wonder when multiple two-way bindings would be used on a single component? Any control I can think of only has a single "value" to two-way bind to. And a larger form component would two-way bind to a object, with child components for each control getting their own two-way binder.

@KaelWD
Copy link

KaelWD commented Jan 29, 2019

when multiple two-way bindings would be used

Vuetify's autocomplete component uses v-model for the selected item and :search-input.sync for the search field, just as one example. Data tables also use it for pagination control.

@Justineo
Copy link
Member

Justineo commented Jan 29, 2019

Because v-slot already introduces inconsistency with v-on and v-bind on the behavior of default arg, I think it is no longer a problem for v-model's default arg to work like it does now.

I personally have never used v-bind.sync in my projects, probably because a component usually won't provide so many props with .sync support that requires the spread feature. I would rather remove the spread feature altogether.

@dyllan-to-you
Copy link

I personally have never used v-bind.sync in my projects, probably because a component usually won't provide so many props with .sync support that requires the spread feature. I would rather remove the spread feature altogether.

In a project at work, I currently have a dynamic component that's passed an object using v-bind.sync. The spread feature is pretty convenient; it allows any of the passed components to pull the props that it uses, and makes that action explicit.

When it comes to duplicating the object spread behavior, I'm a fan of option 1. I think that the consistency is valuable, and it would make v-model's underlying behavior more clear (at the cost of it being a little more verbose).

@smolinari
Copy link
Contributor

smolinari commented Jan 30, 2019

I agree. The two way thinking is more in line with v-model.

I got confused by the examples though.

v-model="xxx" means the prop value in the child component will be "synced" with the xxx prop in the parent component. This is standard Vue, isn't it?

Thinking further, couldn't a new syntax be possible for syncing between two different props or set of props between the child and parent?

i.e.

v-model:my-childs-prop="myParentsProp"?

If there are a number of props to sync, use objects.

v-model:my-childs-object-of-props="myParentsObjectofProps"

Of course, just like in v-bind, the child must have the properly defined props available.

You mentioned the current compiler not balking when the child and parent component props don't have the same name. I don't think that should be a requirement, i.e. that both prop names match. For instance, in the docs there is this example:

<text-document v-bind:title.sync="doc.title"></text-document>

The new suggested v-model equivalent of this would be (as I understand it):

<text-document v-model:title="doc.title"></text-document>

Or it could be:

<text-document v-model:some-prop-with-some-other-name="doc.title"></text-document>

Or!!!

When you have just v-model="someProp", AND the prop with the same name someProp is available in the child, sync it to the prop in the parent. If the prop is not available in the child, sync it to value, which has to be there as the last chance for v-model to work. Is this getting too complicated?

Or am I in left field, as is so often the case? 😁

Scott

@panstromek
Copy link

I like the how option 1. for spread is consistent with v-bind and v-on, but it's breaking and It quickly leads to thinking about another shortcut, which I am not sure is desirable.. ::model-value? *model-value? $model-value?

@zaun
Copy link

zaun commented Jan 31, 2019

@pansyromek I was already thinking about :: as a possible shorthand for v-model. I was contemplating writing a rfc before this one popped up.

@panstromek
Copy link

@zaun haha :D, I was joking at first but when I wrote it down, I realized I kinda like that syntax :D


## Summary

Deprecating `v-bind`'s `.sync` modifier and replacing it with an argument on `v-model`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to deprecate or remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove. 🙂 Just fixed!

@chrisvfritz
Copy link
Contributor Author

@smolinari

v-model="xxx" means the prop value in the child component will be "synced" with the xxx prop in the parent component. This is standard Vue, isn't it?

Yes, I was just highlighting that this behavior would remain the same and how it's different from the proposed behavior of v-model with an argument. 🙂

Thinking further, couldn't a new syntax be possible for syncing between two different props or set of props between the child and parent?

If I'm understanding you correctly, isn't that what the proposal already does?

If there are a number of props to sync, use objects.

v-model:my-childs-object-of-props="myParentsObjectofProps"

I'm not sure this syntax makes sense. What are you thinking it would compile to? And what would the shape of each object be?

You mentioned the current compiler not balking when the child and parent component props don't have the same name. I don't think that should be a requirement

As you mentioned, the current proposal is not suggesting it be a requirement. 🙂

When you have just v-model="someProp", AND the prop with the same name someProp is available in the child, sync it to the prop in the parent. If the prop is not available in the child, sync it to value, which has to be there as the last chance for v-model to work. Is this getting too complicated?

Yes, I think so. 😉


@panstromek @zaun I agree with @Justineo that with v-slot already creating an exception to the rule, it's not as important for v-model without an argument to spread. Since the need for a spread will also be quite rare, but does exist, I'm still leaning towards a .spread modifier only documented on the API page.

Despite the consistency of option 1, I'd estimate that v-model without an argument might be hundreds or even thousands of times more common than the use case for v-model with an argument. So I think the advantages would really have to be monumental to justify making the much, much more common use case more verbose and complex.

@smolinari
Copy link
Contributor

smolinari commented Feb 2, 2019

Yes, I was just highlighting that this behavior would remain the same and how it's different from the proposed behavior of v-model with an argument. 🙂

Hmm... so is this correct, if I change model-value to just value?

<input v-model="xxx">

<!-- would be shorthand for: -->

<input
  :value="xxx"
  @update:value="newValue => { xxx = newValue }"
>

Your use of model-value was throwing me, because Vue's docs and the code I've worked on always used value as the name of the child component's prop. 😄

If that's true, the rest all makes sense. Or, I'm definitely in left field. 😁

What are you thinking it would compile to?

I don't know. That is a step more than my knowledge can help with.

And what would the shape of each object be?

They'd have to be equal to work, I think. Ok. Contemplating more on that, that is a problem. Nevermind.

What I was thinking of accomplishing was to avoid having to write multiple v-models in order to sync multiple props.

Scott

@sirberus
Copy link

sirberus commented Feb 4, 2019

I love this proposal and agree completely that this is a more straightforward API. I would like to echo what @panstromek said about considering a shorthand to really cement this. (I know it's a separate RFC, just leaving the comment here.) Since # got used over & for v-slot, I say we yoink & for this. It's a semantically relevant symbol, "and" being a word that connects and ties together. It would flow very cleanly. The tilde (~) might also be acceptable. I would avoid * because of the profusion of *foo patterns in so many languages, experienced programmers would get tripped up. :: might be OK - the doubling-over of the one-way-bind syntax to indicate two-way-binding flows nicely, but I don't know how I feel about multi-character shorthands.

@chrisvfritz
Copy link
Contributor Author

Hm... so is this correct, if I change model-value to just value?

@smolinari No, because v-model's implementation details are likely to change in Vue 3, so that it no longer just compiles to a value prop and input listener, but to something like a modelValue attribute and update:modelValue listener. Since it's not part of directly part of this proposal though, I wouldn't worry too much about it. 🙂

What I was thinking of accomplishing was to avoid having to write multiple v-models in order to sync multiple props.

That's what these proposals are for.


I would like to echo what @panstromek said about considering a shorthand to really cement this.

@sirberus As long as we don't go for option 1, we're not making the most common use case more verbose, so there's no need for a shorthand. v-model with an argument would also not be used nearly enough (as @Justineo mentioned, even many power users never/rarely use it) for a shorthand to ever remove significant syntax noise. That means in this case, the only benefit of a shorthand would be saving keystrokes, and text expansion tools can achieve that just as effectively without the many disadvantages of a shorthand. As you mentioned though, a shorthand would be a separate RFC anyway, so let's leave shorthand discussion out of this RFC. 🙂

@panstromek
Copy link

I agree with @ChrisFritz. Even though I like the :: shortcut, I only like it because it is cute 😄, but I think it's good to have v-model more verbose, because it makes it distinct and reminds you that there is a cost for two-way binding.

@pickfire
Copy link

But the need for a two-way bind is not always the use case. There are some cases where a .sync is purposely removed. Is there any alternative for the plain v-bind after defaulting to v-bind.sync?

@Justineo
Copy link
Member

after defaulting to v-bind.sync

There's no such proposal in this RFC.

@backbone87
Copy link

Maybe it is worth to decouple models from props (just like data is decoupled from props)?

This would result in:

  • Props: One way flow of any reactive value to a bound child components prop or attribute.
  • Provide/Inject: One way flow of any reactive value to a descendant components injection point.
  • Data: Two way flow inside a components reactive data.
  • Model: Two way flow from reactive data or model to a child components model.

The advantage of this would be, that you dont need to configure model definitions in two places inside the options. Also it would be possible to "automatically convert unused models to data": <my-counter v-model="count" /> would still work, if you dont provide a model.

@chrisvfritz
Copy link
Contributor Author

@backbone87 I'm not sure I fully understand what you're suggesting, but I think it may be more related to this RFC, which you may be interested in.

@plehnen
Copy link

plehnen commented Apr 23, 2019

I am voting for option 1, as it feels very natural and consistent:

<!-- parent (i.e. local scope) ppp is passed in MyComp as prop 'ccc' -->
<MyComp v-model:ccc="ppp" />

<!-- parent ppp1 and ppp2 are passed in MyComp as props 'ccc1' and 'ccc2' -->
<MyComp v-model="{ ccc1: ppp1, ccc2: ppp2 }" />

<!-- parent foo is passed as prop foo -->
<MyComp v-model="{ foo }" />

It could automatically fall back to the old behaviour if the prop is not an object:

<!-- if foo is an object, all props are passed to MyComp.
if foo is not an object, it is passed to the default 'model-value' prop inside MyComp -->
<MyComp v-model="foo" />

And I like that idea of using :foo for one-way and ::foo for two way binding shortcut.

@backbone87
Copy link

@chrisvfritz
i am talking about converting models to a equally-leveled concept as props, methods, data and so on:

export default {
  props: ['a', 'b'],
  models: ['c', 'd'],
  data: () => ({ e: 1, f: 2 }),
}

where models functions like a mix of data+props.
currently i cant (easily) have a model-exposing component that works with or without the parent providing a binding. <my-input /> just wont have a usable input field.

@yyx990803 yyx990803 added the final comments This RFC is in final comments period label Nov 6, 2019
@yyx990803
Copy link
Member

This RFC is now in final comments stage. An RFC in final comments stage means that:

  • The core team has reviewed the feedback and reached consensus about the general direction of the RFC and believe that this RFC is a worthwhile addition to the framework.
  • Final comments stage does not mean the RFC's design details are final - we may still tweak the details as we implement it and discover new technical insights or constraints. It may even be further adjusted based on user feedback after it lands in an alpha/beta release.
  • If no major objections with solid supporting arguments have been presented after a week, the RFC will be merged and become an active RFC.

@yyx990803 yyx990803 merged commit 5e289d7 into vuejs:master Nov 12, 2019
@yyx990803 yyx990803 added feat: template syntax Changes related to template syntax feat: directives Changes related to directives and removed final comments This RFC is in final comments period feat: template syntax Changes related to template syntax labels Jul 1, 2020
@bencodezen bencodezen mentioned this pull request Jul 6, 2020
25 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x This RFC only targets 3.0 and above breaking change This RFC contains breaking changes or deprecations of old API. core feat: directives Changes related to directives
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet