-
Notifications
You must be signed in to change notification settings - Fork 545
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
Event Forwarding #11
Event Forwarding #11
Conversation
How about including an option to specify what should be emitted? By default it would be Before:<div>
<input
type="checkbox"
@change="$emit('change', $event.target.checked)"
>
</div> After:<div>
<input
type="checkbox"
@change.emit="$event.target.checked"
>
</div> |
@Aferz well, now it becomes more a rearrangement than shortcut... On a similar note, what about forwarding as some different event? Something like: <children-component @click.emit:clack />
...
<parent-component @clack="handleClack" /> Also, in most cases you can just use |
I would actually prefer to have |
May be add some options for
Or we can do it like
|
I think this encourages bad design, there are better patterns to deal with nested components communication. |
Honestly I don't know why are you saying this because the combo emit/prop is the original implementation and the recommended way to communicate between parents and children. This RFC only pretends to shorten the way we emit some of this events.
Could you please be more specific? |
The emit is fine for child-parent communication, but less for child-grandparents communication, which is what this RFC encourages to do. Don't get me wrong, for some cases it's okay, but I don't think it should be encouraged because it gets very hard to follow as it goes deeper. There are other ways to deal with components communication: props, provide/inject, event bus, vuex, portals... Each pattern has different cons/pros so it all depends on the specific situation. |
@rellect i think you miss the point. Event forwarding isnt bad practice. From the grandparents pov the event is emitted by the parent, and that is also exactly what happens, because the parent has explicit control over what is emitted/forwarded. The thing which is considered bad practice is a child emitting to grandparents around the parents control, and this is a violation of hierarchy. |
@rellect I agree with @backbone87 that you may miss the point of my proposal. I don't want to work around grandparent/parent/child communication. In fact, what I proposed is a way to write less to encourage the recommended approach of component communication. The patterns you proposed are, in fact, workarounds to communicate between deep components hierarchies:
|
I don't think event forwarding is bad practice, I used it countless of times, but to a certain level. I understand that this RFC is only to reduce boilerplate for a common pattern, but my concern is it will be considered "official" approach for communication and abused. (Especially by newcomers). It's only my opinion that's all |
On a similar note, how about passing props down and events up in the same syntax? Something I've had to do when creating a custom input Right now: <!--Child Input Component-->
<div>
<input :value="value" @input="$emit('input', $event.target.value)" />
</div> With this proposal: <!--Child Input Component-->
<div>
<input :value="value" @input.emit />
</div> How can do this better? Maybe <!--Child Input Component-->
<div>
<input :value.pass />
</div> In this way See here for a similar proposal. (Caveat with my example above: HTML inputs (and existing custom Vue components designed to be compatible with |
I thought this was removed from Vue 2 (event bubbling existed in Vue 1) specifically because it’s accepted to be a bad practice and strongly couples components needlessly. |
@lloydjatkinson Event bubbling is implicitly propagating an event along an entire path in a tree. This proposal isn’t event bubbling. It is syntactic sugar for event forwarding, which has clear boundaries on which elements receive an event and no such notion as „along an entire path of arbitrary length“ exists. Edit: with event bubbling a node might and most likely isn’t aware of the events it’s descendants emit „through“ it. This causes coupling between parents and children of a node. Using event forwarding requires nodes to be aware of the contract that the forwarded events offer and confirms this contract and ultimately opts in to uphold the contract even if there isn’t a child emitting the event in the future. In this case the Node has to find a way to shim/replace/fulfill its contractual obligation, which it’s explicitly opted into by forwarding the event |
@backbone87 That's correct. I couldn't explained It better. Don't forget this RFC is just syntactic sugar for keeping your code DRY and don't loose readability. Futhermore, is entirely optional. |
In case people don't understand the use case, I think the simplest one is a custom input (and that is indeed explained in the rendered proposal):
Also, you probably want your custom input to support |
@bandleader this use case is even better covered with transparent wrapper pattern with TBH I don't think this feature is worth it, I don't even remember last time I had a use case for it, and in that case, just writing like 5-10 characters is not that big of a problem. |
If we merge this RFC for Vue 3, you'll just need |
Just to clarify, the example of @bandleader is exactly that, an example. This RFC doesn't pretend to handle the creation of transparent wrappers or HOCs. It aims to reduce the amount of boilerplate code you write when you prefer to explicitly define the events the component is gonna handle or simply emit upwards. This: <input
@focus="$emit('focus', $event)"
@click="$emit('click', $event")
@input="$emit('input', $event)" /> Becomes this: <input
@focus.emit
@click.emit
@input.emit /> Sure, I could make use of Anyway, this is completely optional, and you could not use it at all if you don't like it, but I think this feature could improve readability in our own components because of the fact that we explicitly define which events we want to handle instead of relying on Cheers. |
@Aferz 👍 Right, we don't always want to pass up all events. @panstromek Even if we wanted to pass up all events, your solution would still only work for components with a single child. However, most components aren't like that at all. Just as one example, think of a date control: it's a that contains 3 children: a textbox, an 'open calendar' button, and a hidden div with the actual calendar. Maybe |
@bandleader I don't deny that there is a use case for this. I just don't think it's worth implementing in the compiler. Every change like that comes with a maintanence burden for core team so it better be worth it - ie. used by many and saving a lot of work. This is a syntax sugar for very narrow use case, saving just a few keystrokes in a very specific case. Not worth it IMO. |
Another point is that you could implement your own directive for this. Something like <input v-pass:input> or even better with modifiers <input v-pass.input.blur.click > [EDIT] Vue.directive('pass', {
bind: (el, binding, vnode) => {
Object.keys(binding.modifiers).forEach(event => {
el.addEventListener(event, e => {
vnode.context.$emit(event, e)
})
})
}
}) |
@panstromek I like that, but don't forget checking if the node is a Vue component, because then you need to run Also, the scenario I mentioned earlier Vue.directive('bothways', {
bind: (el, binding, vnode) => {
el.addEventListener("update:" + binding.arg, e => {
vnode.context.$emit("update:" + binding.arg, e)
})
// But how do we pass the prop down?
vnode.addVBind(binding.arg, () => binding.value) //??
// No such syntax, obviously!
}
}) (We could re-implement v-bind, i.e. by catching |
@panstromek I actually got a bit further in implementing it as a directive, by not using But it's going to be really hard to re-implement everything that's in v-model and v-bind. That's why built-in directives would be helpful, or else access to some of the v-model and v-bind internals via an API... |
In Vue 3, this will no longer be an issue as all listeners are now part of attrs and will be automatically inherited if the child component returns another component as root (just like |
@yyx990803 it very frequently isn't the root. |
@JosephSilber in that case you can use |
Sorry @yyx990803 but I don't get it. Could you explain with a code example? Because what you said is already available in Vue2 and I don't see how it fixes what this RFC proposes. Thanks! |
More details can be found in the new attrs fallthrough behavior RFC: #92 I assume the use case mostly happens with "wrapper components" where you not only want to forward a specific event, but rather anything that has not been declared as a prop by a component in the hierarchy. Therefore this is should cover a lot of such cases: <!-- Child that wraps a non-root target element: use inheritAttrs: false with explicit v-bind="$attrs" -->
<div>
<input v-bind="$attrs">
</div>
<!-- If the Parent renders Child as root: nothing needed, all attrs implicitly falls through -->
<children-component />
<!-- Grand Parent -->
<div>
<parent-component @click="handleClick" />
</div> The |
Understood. Thanks @yyx990803 ! |
@yyx990803 What about the case where you want to forward specific events? For example: a component that provides a form control with some extra elements (validation feedback, calendar icon, label, etc.) before/after the <div>
<label>
<input :value="value" @input.emit @focus.emit @blur.emit />
</label>
</div> |
@bandleader you can do something like <div v-bind="excludeEvents($attrs)">
<label>
<input :value="value" v-bind="pluckEvents($attrs)" />
</label>
</div> Where you include/exclude the events you want in the method. JavaScript provides full flexibility in these niche cases without the need for introducing additional template syntax. |
@yyx990803 Fair enough. In a similar vein, how would you recommend doing what I mentioned in this comment above -- passing an event up and a prop down in a single syntax? Can it be done similarly: <div>
<input v-bind="twoWayValue" />
</div> computed: {
twoWayValue: function() {
return { value: this.value, oninput: e => this.$emit("input", e.target.value) }
}
} Or via a helper: // Helper function: returns a computed property function that you can v-bind to your element.
function twoWayHelper(propName) {
return function() {
const eventName = propName === "value" ? "input" : propName
const eventHandler = e => this.$emit(eventName, e instanceof InputEvent ? e.target.value : e)
return { [propName]: this[propName], ["on" + eventName]: eventHandler }
}
} <div>
<input v-bind="valueBinding" />
</div> computed: {
valueBinding: twoWayHelper("value")
} Would this work? |
@bandleader yeah that should work. |
@bandleader that looks very clever. Thanks for sharing. |
Fully rendered proposal
Syntactic sugar for emitting events through a deeply nested component tree without repeating too much boilerplate.