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

Moving v-bind.sync to the v-model API #8

Closed
chrisvfritz opened this issue Oct 11, 2018 · 16 comments
Closed

Moving v-bind.sync to the v-model API #8

chrisvfritz opened this issue Oct 11, 2018 · 16 comments

Comments

@chrisvfritz
Copy link
Contributor

chrisvfritz commented Oct 11, 2018

@yyx990803 I've seen v-bind.sync cause quite a bit of confusion in Vue 2, as users expect it to be able to use expressions like with v-bind (despite whatever we put in the docs). The explanation I've had the best success with is:

Thinking about :title.sync="title" like a normal binding with extra behavior is really the wrong way to think about it, because two-way bindings are fundamentally different. The .sync modifier works essentially like v-model, which is Vue's other syntax sugar for creating a two-way binding. The main difference is that it expands to a slightly different pattern that allows you to have multiple two-way bindings on a single component, rather than being limited to just one.

Which brings me to the question: if it helps to tell users not to think of v-bind.sync like v-bind, but rather to think about it like v-model, should it be part of the v-model API instead? For example, instead of:

<MyComponent v-bind:title.sync="title" />

Perhaps a more intuitive syntax would be:

<MyComponent v-model:title="title" />

As a bonus, a change like this would be very easy to flag with the migration helper and fix with find-and-replace.

Thoughts?

@posva
Copy link
Member

posva commented Oct 12, 2018

Should we consider renaming the input event to update:value when no argument is provided? It would be another breaking change

@chrisvfritz
Copy link
Contributor Author

chrisvfritz commented Oct 13, 2018

@posva I like that idea a lot! 🙂 It'd make component events more consistent and would actually solve a problem we currently have with transparent wrapper components. I just created a new issue to explore this further.

@Jinjiang
Copy link
Member

May I have a question about v-model and .sync here? 😅

IMO, both .sync and v-model are a little limited when use case becomes complicated. Because it often require you to do some validation or conversion during the two ways.
When the value is a primitive type, that's OK. But when it's an array like multiple checkbox or select in https://vuejs.org/v2/guide/forms.html , the data change is actually happened immediately without chance to validate, though you would receive an update event next. It looks like the same way to the primitive value but works different.

So I'd like to make sure in v3.0 whether the value pass to the v-model or .sync is immutable? immutable by default? optional immutable? or still mutable (I see the immutable/reactive observer in source code).

Thanks.

@Justineo
Copy link
Member

Justineo commented Oct 30, 2018

vuejs/vue#4946

This is where we decided to bring .sync back for 2.x and the original feature request was v-model for multiple values.

Here are some potential problems I can think of:

With the current syntax being v-model="val", the actual prop in the binding is encapsulated, especially for native input elements like select which we have quite a lot special logic. If we support provide directive arguments like v-model:title, we have two choices for the default binding:

  1. Always enforce the argument, like v-model:value. While most of the components don't provide multiple two-way bindings this seems a little too verbose to me.
  2. Keep v-model for the old magic and only provide arguments for non-value bindings. This will make our syntax inconsistent: v-bind:title stands for binding single prop while v-bind stands for binding all passed props. So is v-on. If we have both v-model and v-model:title it'll seem confusing with the experience of other directives.

@chrisvfritz
Copy link
Contributor Author

chrisvfritz commented Nov 2, 2018

Keep v-model for the old magic and only provide arguments for non-value bindings. This will make our syntax inconsistent: v-bind:title stands for binding single prop while v-bind stands for binding all passed props. So is v-on. If we have both v-model and v-model:title it'll seem confusing with the experience of other directives.

@Justineo That's a fair point. I think everywhere else that we have a directive with an optional argument, the lack of arguments is used to spread an object. Though I don't think this will actually cause much confusion in practice, since v-model is introduced so early and is used so frequently. It's also different from v-bind and v-on in that its primary use case is argument-less (and the version with an argument is only introduced later in the guide), whereas v-bind and v-on are the opposite.

So I'm still in favor of option 2. What do you think?

@Justineo
Copy link
Member

Justineo commented Nov 3, 2018

So I'm still in favor of option 2. What do you think?

Not quite sure about it…it also disables usage like v-bind.sync="obj".

@chrisvfritz
Copy link
Contributor Author

Good point. Though the use case is relatively rare, I agree we should handle it. Maybe a modifier like v-model.spread="object" could address that use case. Thoughts?

@Jinjiang
Copy link
Member

Should we consider renaming the input event to update:value when no argument is provided? It would be another breaking change

So how could we bind an event named update:value in template without v-model? Is <MyComponent @update:value="xxx" /> or <MyComponent v-on:update:value="xxx" /> (two colons?) a valid syntax?

@Jinjiang
Copy link
Member

I have another idea. Not quite sure it's good enough, but just an idea. 😅

class Foo extends Vue {
  props,
  data,
  model() {
    return {
      value, ...others
    }
  },
  ...
}

In another word, because the update event must be defined in the component to make sense. So maybe it's good to define model/models internal, not external as a syntax sugar only.

When we return a member in model(), it is not reactive but would emit an event when updated.

Thanks.

@chrisvfritz
Copy link
Contributor Author

So how could we bind an event named update:value in template without v-model? Is <MyComponent @update:value="xxx" /> or (two colons?) a valid syntax?

I don't see why we couldn't make it valid, if it isn't already. 🙂 I don't think we currently allow for multiples arguments on a directive, do we?

In another word, because the update event must be defined in the component to make sense. So maybe it's good to define model/models internal, not external as a syntax sugar only.

When we return a member in model(), it is not reactive but would emit an event when updated.

I don't think I fully understand what you're suggesting. Would you mind providing a more complete example to demonstrate the use case?

@Jinjiang
Copy link
Member

Jinjiang commented Nov 30, 2018

I don't see why we couldn't make it valid if it isn't already. 🙂 I don't think we currently allow for multiples arguments on a directive, do we?

Update: Found that syntax like @update:value="xxx" already supported well in v2. No more asked. 🙂

In v2 doc link, <input v-model="searchText"> does the same thing as <input v-bind:value="searchText" v-on:input="searchText = $event.target.value"> (similar to v-model in custom components). So v-model is just a sugar syntax imo which means you can choose both v-model and v-bind+v-on ways to do that. If we include a colon in a event name. That means we must use v-model and it's not a sugar syntax anymore.

For example, think about we have a custom component <InputDate /> which supports v-model and would init the value to new Date() if nothing passed in. For this, it's necessary to support only having an event handler but without value passer at the same time. It works in v2 as <InputDate @input="date = $event" />. But I am not sure update:value is a good event name for that in the future.

I don't think I fully understand what you're suggesting. Would you mind providing a more complete example to demonstrate the use case?

Well, I have realized that's not a good idea. But give an explanation here.

I have written many custom components which support v-model like:

<template>
  <input :value="localValue" @input="handle" />
</template>
<script>
export default {
  props: ['value'],
  data() {
    return { localValue: this.value }
  },
  watch: {
    value(val) { this.localValue = val }
  },
  methods: {
    handle(event) {
      let val = $event.target.value
      // do some validations/transformations
      this.setValue(val)
    },
    otherActions() {
      // do sth.
      this.setValue(...)
    },
    setValue(val) {
      this.localValue = val
      this.$emit('input', val)
    }
  }
}
</script>

So I have a thought to simplifiy that maybe like:

<template>
  <input :value="localValue" @input="handle" />
</template>
<script>
export default {
  // or `model: { value: { type: String, default: xxx }},`
  // or `props: { value: { ..., model: true }}`
  model: ['value'],
  methods: {
    handle(event) {
      let val = $event.target.value
      // do some validations/transformations
      this.setValue(val)
    },
    otherActions() {
      // do sth.
      this.setValue(...)
    },
  }
}
</script>

It supplies a data named loaclXxx and a method setXxx(v) automatically. And when you call setXxx(v), an event named input or update:xxx would be emitted if v is different with localXxx.

and actually the code below also works the same:

<template>
  <input :value="localValue" @input="handle" />
</template>
<script>
const mixinModel = {
  data() {
    return { localValue: this.value }
  },
  watch: {
    value(val) { this.localValue = val }
  },
  methods: {
    setValue(val) {
      this.localValue = val
      this.$emit('input', val)
    }
  }
}
export default {
  mixins: [mixinModel],
  props: ['value'],
  methods: {
    handle(event) {
      let val = $event.target.value
      // do some validations/transformations
      this.setValue(val)
    },
    otherActions() {
      // do sth.
      this.setValue(...)
    },
  }
}
</script>

@yyx990803
Copy link
Member

Just linking this comment here for reference (implementation details) #9 (comment)

@yyx990803
Copy link
Member

@chrisvfritz I'm starting to prepare for moving things we are discussing here to public RFCs. Do you want to champion this one?

@chrisvfritz
Copy link
Contributor Author

@yyx990803 Happy to. 🙂 Filling out the RFC now.

@chrisvfritz
Copy link
Contributor Author

This is now a public RFC, so further discussion should happen there. @Justineo Your thoughts on the best compromise for spreading an object would be especially welcome. 🙂

@yyx990803
Copy link
Member

Closing in favor of the public RFC.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants