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

Add "emits" component option #16

Merged
merged 5 commits into from
Apr 15, 2020
Merged

Conversation

niko278
Copy link
Contributor

@niko278 niko278 commented Feb 27, 2019

@posva

This comment has been minimized.

@HerringtonDarkholme
Copy link
Member

Thanks for the proposal! Another concern is that this API isn't useful for production code: we probably don't want to check emitted events as we don't for props. So checking API might add runtime overhead.

@634750802
Copy link

@niko278
Vue do have validator for props, maybe validate the events is also useful for debugging?

@yyx990803
Copy link
Member

I like this. I've referred to this in #26 - specifically, this option can serve as an indication that a certain event listener should be consumed by this specific component and should not be passed down in $attrs.

Some other feedback:

  • Naming. We've had an option named events in the past with completely different behavior (it was used to register event listeners). It was deprecated and removed later but may still show up in search results. Maybe we can rename the new option to something like emits.

  • Ideally, the option and its type validation should be as close to props as possible to lower learning costs.

    • For this reason I am not so sure about the object validation format as for props an object means something quite different.

    • Suppose we also support arrays:

      emits: {
        foo: [String, Number]
      }

      With implicit shape validation, this can be confusing: is it denoting two arguments, a single argument of Array type, or a single argument of string | number type? Should we also support complex shapes like [{ foo: String }]? It seems to be a rabbit hole where Vue would need to include a full schema validation implementation. I don't think Vue should go that far into runtime type validation.

      Maybe we should support only simple types and then allow the same custom validation escape hatch like props:

      emits: {
        foo: {
          validator: (...args) => {
            // perform custom validation logic
          }
        }
      }

    Similar to props, events validation should be dev-only so related logic can be removed in production.

@Akryum
Copy link
Member

Akryum commented Apr 8, 2019

I'm all for emits 👍

@niko278
Copy link
Contributor Author

niko278 commented Apr 8, 2019

@yyx990803
You are right, validation should be as close to props as possible. If a developer would like to validate nested objects, it can be done using validator function. Including a full schema validation may be unnecessary then. I also like the emits option name.

@yyx990803
Copy link
Member

@niko278 great - could you update the PR to reflect the suggested changes?

@Akryum Akryum changed the title Add "events" component option Add "emits" component option Apr 8, 2019
@tmorehouse
Copy link

For emits, do you really need validation? it is the component that defines the values emitted, and not the consumer of the events (unlike props where the consumer is passing a value to the component)

Maybe the definition of an emitter should be like:

emits {
  'foo-event': {
    args: [
      // Argument 1
      [Number],
      // Argument 2
      [Number, String],
      // Argument 3
      [Event]
    ]
  }
}

Where args is an array of argument positions and expected types for that position. If an event emits no arguments, then one would specify an empty array (or perhaps null or false)

@ycmjason
Copy link

ycmjason commented Dec 2, 2019

This can potentially help typing the emit function too right? 😍

@tmorehouse
Copy link

When combining with #137, and one has a custom event input, and a consumer also wants to listen to the native input event, how would this work? Would the consumer be blocked from also listening to native events?

Perhaps all custom events should be prefixed (camelCase, PascalCase, kebab-case) so there cannot be any conflicts with native events? This would be considered a breaking change, but might be the only way around this now that the .native modifier is to be removed. The one issue with prefixing (requiring custom events to be hypenated) is browser templates, where attributes are all lower cased. Maybe on-customevent is better than onCustomevent?

@cawa-93
Copy link

cawa-93 commented Mar 2, 2020

It is interesting whether it is possible to somehow combine this with the TypeScript syntax. To specify a more specific type of data that is transmitted with the event. And so that the component component listener can tell what is coming.

@sirlancelot
Copy link

@tmorehouse Based what I'm reading/understanding, if you omit input from the emits option, in addition to the event falling through to the Node, you will still have access to it as $attrs.onInput in code and, presumably you could still $emit() it. I believe this RFC simply defines a way to disable implicit fallthrough for events.

@tmorehouse
Copy link

tmorehouse commented Mar 27, 2020

@sirlancelot how would the following work with the removal of .native modifiers, where input is also the name of a custom emitted event of <custom-input>?

<custom-input @input.native="nativeHandler" @input="customHandler" :value="value"></custom-input>

@sirlancelot
Copy link

This is my opinion, but I wouldn't expect that scenario to be supported. To me it looks rather confusing. I have to imagine there's a better way to implement that component as it would seem like it's doing more than one thing.

@tmorehouse
Copy link

tmorehouse commented Mar 27, 2020

Imagine a custom input that debounces the v-model as the user type, and emits a custom input event after the debouncing timeout. But someone wants to alter the text as the user is typing (ie. transform to upper case), of which they need access to the native event object (the custom input event emits the value of the input, while the native input event emits the native event object)

The majority of users expect custom input components to emit the input event as the v-model

Here is the same example from above written another way for clarity:

<custom-input @input.native="nativeHandler" v-model="value"></custom-input>

Where <custom-input> emits a custom input event for updating the v-model (as most people would expect). And nativeHandler is a consumer supplied handler that transforms the input value as they type.

@jods4
Copy link

jods4 commented Mar 27, 2020

@tmorehouse v-model events changed in Vue 3.
https://github.com/vuejs/rfcs/blob/master/active-rfcs/0011-v-model-api-change.md

If a component supports v-model it should emit update:modelValue, not input

@tmorehouse
Copy link

@jods4 input for v-model was just one example... but the same issue applies for other custom events emitted that may have the same name as a native event.

@jods4
Copy link

jods4 commented Mar 27, 2020

Sure, the point kind of was that v-model forces the event names.
I think it's not a great idea to have components that emit events with the same name as native ones that are inside them.

I'd rather user different event names to start with: if it's not the same thing, don't give it the same name. It avoids a lot of confusion and problems in the long run.

@tmorehouse
Copy link

In some cases that is not easy... when the name change would be more meaningful to the user, than using custom-change.

Unless Vue decides to dictate that all custom events need to be kebab-case/prefixed.... but then that just breaks when using onCustomChange (in browser templates, as capitalization is lost)

@sirlancelot
Copy link

Architecting an app is never easy, but I have to say that in the 5+ years I've been using Vue.js to write professionally and personally, I have never encountered the problem you're describing. That's not to imply it's not a valid scenario, but to point out that it's possible to avoid while still maintaining a manageable codebase.

But I digress, we've gone a little off-topic...

@yyx990803
Copy link
Member

yyx990803 commented Apr 9, 2020

@jods4 it would be best to keep the API consistent with v2 to avoid unnecessary differences.

@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 added the final comments This RFC is in final comments period label Apr 9, 2020
@jods4
Copy link

jods4 commented Apr 9, 2020

The proposed syntax allows strong typing of parameters but not of return values, right?
Because the proposed declaration uses the returned value as a boolean to validate parameters, there's no way to tell what the event actual return value is.

In practice many events are void, but Vue returns an array of all values returned by registered handlers.
Use case:

// Asks parent if he vetoes the update of some data
let result = emit("canSave", entity); // should be boolean[] but any[] in current proposal
if (result.some(x => x === true)) return;

Is custom parameter validation actually useful?

Props are meant to be set by consumers of the component. It makes sense to be able to apply validation to their inputs, i.e. that the api is used correctly.

Emits are sent by developpers of the component. It's an output and we validate that api behaves correctly with unit tests.

I'd rather have no validation and strong typing, maybe like this:

{
  emits: {
    // never called but defines Typescript types for the emit call and returned values
    canSave(entity: object) { return true }, // emit("canSave", object) => boolean[]
    getUuid() { return "" }, // emit("getUuid") => string[]
    untyped: null,  // emit("untyped", any) => any[]

    // alternatively, works the same
    canSave: null as (entity: object) => boolean,
    getUuid: null as () => string,
  }
}

EDIT: as a bonus, the first syntax could actually define a default listener/return value when there's no listener. This can be handy if you build component with a lot of default overridable behavior (e.g. how Hibernate works on the back-end). Your form component could have emits: { load, save, delete } and provide default implementation for all of those, but the consumer could provide alternative implementation for any/all of those.

EDIT2: I focused on the emitting side, but having a return type would also enable tooling like Vetur validate that the listeners are of correct type, including the value they return. Validating consumers usage is IMHO more important than validating internal implementation.

@yyx990803
Copy link
Member

yyx990803 commented Apr 9, 2020

@jods note that we shouldn't design an API that is solely geared for typing. Your proposed usage, while potentially provides a bit more type coverage, introduces an alternative that serves nothing but types, while blocking another runtime feature. Users not using TypeScript will most likely be confused why the function syntax is used for typing (which to them equals nothing) but not for runtime validation. You would certainly prefer the typing because you are a heavy TS user, but considering the overall user demographics of Vue I don't think that trade-off makes sense.

@jods4
Copy link

jods4 commented Apr 10, 2020

@yyx990803 That was only a proposal. Maybe someone can come up with a better alternative.

The key point here is that the proposal doesn't provide strong typing for event results for neither the emitter nor the listener (assuming one day Vetur may typecheck event).

One message is that Vue 3 is designed for TS and as a heavy TS user (you're right), seeing any creep into my code is not something I fancy. It is 100% gonna mean casts in my code.

BTW It's not the key point here but I'm gonna reiterate that I'm not sold on the "other runtime feature", to me the idea just feels weird. Input validation is a given, output not so much.

Typing aside, I personally would have more use for a "default" event listener feature rather than output validation.

So far since alpha.11 is out, all my events look like name: (something: Type) => true.
Maybe that's just me.

@jods4
Copy link

jods4 commented Apr 10, 2020

Here's another idea then: reuse the same shape as props?
It is (1) consistent and familiar; (2) extensible with many more facets.

emits: {
  // parameter is a number, implicitely no return value (void)
  click: Number,  
  // couple is [payload type, return type]
  canSave: [Object, Boolean], 
  // more complex scenarios can use an object
  complex: {  
    type: Date,  // payload type
    return: Number,  // return type
    validator: (date) => (+date) > +(new Date),  // optional validation of payload
    required: true, // indicates that consumers are supposed to always provide a listener
    default: (date) => 0, // optional listener that will be called if consumer didn't provide at least one
  }
}

@tmorehouse
Copy link

Events can emit multiple values though:

this.$emit('foo-event', 'string', 123, { foo: 'bar', bar: 'baz' })

@jods4
Copy link

jods4 commented Apr 10, 2020

@tmorehouse interesting, I wasn't aware. I assume you can only handle them by passing a function that takes multiple args? As inline expression only has access to a single $event parameter?

That doesn't change much, though does it? I would be fine with using the object form when declaring a return value (one-way is the most common case here). So:

emits: {
  // Single parameter ($event)
  click: Number,  
  // Multiple parameters
  canSave: [Object, Boolean, Array], 
  // more complex scenarios can use an object
  complex: {  
    // same as shorthand: could be a single type or an array
    parameters: Date,  
    // optional return type
    return: Number,  
    // optional validation of parameters
    validator: (date, offset) => (+date) > offset+(new Date),  
    // indicates that consumers are supposed to always provide a listener
    required: true, 
    // optional listener that will be called if consumer didn't provide at least one
    default: (date, offset) => 0, 
  }
}

@tmorehouse
Copy link

tmorehouse commented Apr 10, 2020

Yeah, you need a method handler when accepting multiple args. we use it quite a bit.

The first argument is available inline as $event

I've never used the return value from this.$emits though, as there can be multiple listeners for a single custom event... Think of event buses... they would likely have multiple subscribers to a single event name. And which listener would be the winner for returning the value?

@jods4
Copy link

jods4 commented Apr 10, 2020

I've never used the return value from this.$emits though, as their can be multiple listeners for a single custom event... Think of event buses... they would likely have multiple subscribers to a single event.

Vue always returns an array with all results from every listener. What you do with them is up to you.
There are plenty of good use cases for that (albeit the event bus is not one).

Some other frameworks differentiate the two concepts. Take Aurelia for example, it has one syntax for "events" (as you use them) and a different syntax for "delegates", which are just a single function that the component can call back.

Vue doesn't make a difference but since it provides the values returned by events, it can be used in both situations.

@backbone87
Copy link

"events" (as you use them) and a different syntax for "delegates", which are just a single function that the component can call back.

delegates in vue are a prop which takes a function to be called. i use this pattern basically in all my components that want to delegate behavior (like forms).

Vue always returns an array with all results from every listener. What you do with them is up to you. There are plenty of good use cases for that (albeit the event bus is not one).

the event pattern is inherently fire and forget. feedback from listeners to the event source should always be optional and can occur at any point in time after the event was fired. if you need a feedback channel, you can add methods to the event, which the listener can use (which may throw or do nothing depending on the state of the event source). i would consider using the return values of listeners an anti pattern, which may work well in very specific use cases, but a method on an event works just as well and communicates the control flow much better.

@jods4
Copy link

jods4 commented Apr 10, 2020

delegates in vue are a prop which takes a function to be called. i use this pattern basically in all my components that want to delegate behavior (like forms).

There's a slight difference in that you can only bind a method name and not use a function expression like you can using v-on / @; and the fact that writing a call in v-on "feels" like code whereas writing a value in v-bind "feels" like data, so something like canSave or validate "feels" more like an event than a property.

But ok, that works well enough.

the event pattern is inherently fire and forget. [...] i would consider using the return values of listeners an anti pattern

Then let's revisit this design choice: let's drop event return values from Vue 3.
My point is that if it's a Vue feature, then it should have TS support one way or another.

Or at least mark it obsolete, tell everyone it's only there to help backcompat of Vue 2 users and shouldn't be used and that's why it's not as well supported as other features.

@yyx990803
Copy link
Member

yyx990803 commented Apr 10, 2020

@jods4 in case you didn't know... when you declare a props named onFoo, you can indeed pass a listener to it in the parent as v-on:foo, because it compiles down to onFoo (v3 only). So if you want to strongly type the return value, use a prop declaration instead of emits, and just call the function instead of using emit.

@jacekkarczmarczyk
Copy link

does it make sense to return anything from emit in this case?

@jods4
Copy link

jods4 commented Apr 10, 2020

@yyx990803 Right, that's a slick trick. I like that new approach in v3! I knew all pieces that make it work but didn't glue them together! Well... maybe except I didn't know that properties onXxx take precedence over events.

Still, what do you think of those return values? Are they obsolete, are they supported?
I agree the pattern @xxx to bind a onXXX prop that is a function is much better, but then what are the event return values for?

If they stay I still feel they should support Typescript (even though I care much less now).

@yyx990803
Copy link
Member

yyx990803 commented Apr 10, 2020

@jods4 @jacekkarczmarczyk yeah, I think it makes more sense to remove emit return values - if you need return values, you should use a callback prop instead.

The feature actually did not exist in v2. I implemented it in v3 due to some existing requests, but now it seems indeed redundant.

yyx990803 added a commit to vuejs/core that referenced this pull request Apr 10, 2020
BREAKING CHANGE: this.$emit() and setupContext.emit() no longer
return values. For logic that relies on return value of listeners,
the listener should be declared as an `onXXX` prop and be called
directly. This still allows the parent component to pass in
a handler using `v-on`, since `v-on:foo` internally compiles
to `onFoo`.

    ref: vuejs/rfcs#16
@backbone87
Copy link

@yyx990803

I implemented it in v3 due to some existing requests

i think this is a documentation problem. when i started to use vue and read the official guide and a lot of blog posts about how to get stuff done in vue. on the topic of "function props" it was recommended to avoid them and use events/slots instead without going into detail about valid use-cases for props that receive functions.

when you declare a props named onFoo, you can indeed pass a listener to it in the parent as v-on:foo, because it compiles down to onFoo (v3 only). So if you want to strongly type the return value, use a prop declaration instead of emits, and just call the function instead of using emit.

how does this interact with listener merging/multiple listeners? (if it interacts)

@jods4

There's a slight difference in that you can only bind a method name and not use a function expression like you can using v-on / @

you can do :calculate="(a, b) => a + b" which is much more clear in contrast to @calculate="arguments[0] + arguments[1]" imho.

@backbone87
Copy link

picking up on my last part of my last comment:

you can do :calculate="(a, b) => a + b" which is much more clear in contrast to @calculate="arguments[0] + arguments[1]" imho.

what is missing here, is my conclusion to also use either function values or inline functions in v-on directive values: @calculate="(a, b) => a + b" or @calculate="calculate". I would go as far and deprecate "deferred" expressions in v-on values. e.g. @calculate="makeCalculator('+')": does this mean first making the calculator and then pass this calculator as a listener, or will the listener create a calculator on each calculate` event, but then do nothing with it?

"but what about consistency with HTML inline listeners?", the main difference between HTML inline listeners and vue inline listeners is that in HTML you can ONLY pass statements acting as the listener code, while in vue you can also pass values (functions), but in JS a value or variable symbol is also a statement.

@beeplin
Copy link

beeplin commented Apr 11, 2020

Maybe I miss something but I wonder why we need both props and emits. Why not just have props and use function props to send data and trigger actions from child to parent?

In vue 2 each component is an full functional event emitter, with the classic emit/on/off interface which can pass events not only from child to parent but also across the whole compoent tree. I think that is why in vue 2 events cannot turn into function props. But in vue 3 there is no more emit/on/off interface, and events are now limited from child to parent. So maybe it is better to unify the concepts of props and emits, making @click=... identical to :onClick=...?

By doing this, typings of emits, including types of the parameters and the return value, can be easily defined just like normal props.

@yyx990803
Copy link
Member

@beeplin it is a viable style, but we also need to preserve the backwards compatibility for the event-based usage. Event system is also more fire-and-forget which is somewhat a different mental model. It would be too drastic a change to remove emit and force everyone to use a callback props style. The best we can do is to support both styles and find a way for them to be bridged together.

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

Successfully merging this pull request may close these issues.

None yet