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

Transparent v-model #21

Closed
wants to merge 1 commit into from

Conversation

backbone87
Copy link

@backbone87 backbone87 commented Mar 22, 2019

Rendered

const child = {
  template: `<input type="text" v-model="value">`,
  props: {
    value: String,
  },
};
const parent = {
  components: { child },
  template: `<child v-model="value">`,
  data() {
    return { value: '' };
  },
};

@backbone87 backbone87 changed the title transparent-v-model: first draft Transparent v-model Mar 22, 2019
@posva
Copy link
Member

posva commented Mar 22, 2019

I don't think this is a good idea, mostly because of the implicit behaviour and there would be no idea to opt out unless it was a modifier or option in the component. But even if a modifier is added, there is still the problem of detecting what event to emit and if it has to be emitted by looking at the component's parent.~

I would rather see something on the prop itself than v-model, so that if the prop is assigned to a new value, it emits an event. This also intersects with #10 which is in part also addressing the same issue and could be solved by adding a plugin that reads an option from props like emitOnAssign, creating an internal prop with a setter that emits an event

@backbone87

This comment has been minimized.

@posva

This comment has been minimized.

@backbone87
Copy link
Author

I don't think this is a good idea, mostly because of the implicit behaviour.

What do you mean by implicit behavior? Wrapping components is a common use-case and so is wrapping model-exposing components. The boilerplate to wrap model-exposing components is even used in the vue guide https://vuejs.org/v2/guide/components-custom-events.html#Customizing-Component-v-model. And then you only have the basic handling, which does not respect keyed models or dynamic components.

@posva
Copy link
Member

posva commented Mar 22, 2019

implicit as being something that happens without user consent, passing a prop to v-model could be a mistake on the dev's end while they meant to use :value=" instead

@backbone87
Copy link
Author

backbone87 commented Mar 22, 2019

Thats the tradeoff for expressiveness, isnt it?

We could make it explicit:

const child = {
  model: {
    // ...
    transparent: true,
  },
  // ...
};

edit:
Imho, the way from v-model="value" to :value="value" is quite long, so i cant follow that mixup. Maybe when we have a shorthand for v-model in the future, then &value="value" can be mixed up with :value="value".

I rather see the mixup in v-model="someProp" insteadof v-model="someData".

@backbone87
Copy link
Author

To still have an indication for data/prop mixups, we could provide a dev runtime notice, in case there is no listener for the update event in the parent (and therefore the bound model would never update anywhere).

@agronick
Copy link

agronick commented Jun 19, 2019

To build on this how about an option for forwarding events? Something like

forwards: ['change', 'input', 'hover']

in the js instead of

@change="$emit('change', $event)
@input="$emit('input', $event)
@hover="$emit('hover', $event)

in the template. Sometimes you end up writing code like that 3 components deep just to get events where they need to be.

@gustojs
Copy link
Member

gustojs commented Sep 9, 2019

Forwarding events can be dealt with by the developer with a reusable function, doesn't have to be solved on the API level.

@yyx990803
Copy link
Member

After reviewing the comments so far and a round of voting among core team members, we believe this is not something we will be implementing. Thanks for submitting your idea though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants