Skip to content

[lit-element] Declarative event emitters #3277

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

Open
justinfagnani opened this issue Feb 12, 2020 · 17 comments
Open

[lit-element] Declarative event emitters #3277

justinfagnani opened this issue Feb 12, 2020 · 17 comments
Assignees

Comments

@justinfagnani
Copy link
Collaborator

It would be convenient for ergonomics, documentation, React integration, and transpilers to be able to declare what events an element fires.

We could add a decorator (and static block for plain JS, like properties) that allows this by decorating event handler properties. The decorator would create accessors that use addEventListener under the hood.

TypeScript:

class MyElement extends LitElement {
  @event() onFoo!: (e: FooEvent) => any) | null;

  render() {
    html`<button @click=${this._onButtonClick}>Click Me</button>`;
  }

  _onButtonClick(e) {
    this.dispatchEvent(new FooEvent());
  }
}

interface HTMLElementEventMap {
  'foo': FooEvent;
}

JavaScript:

class MyElement extends LitElement {
  static events = {
    onFoo: {},
  };

  render() {
    html`<button @click=${this._onButtonClick}>Click Me</button>`;
  }

  _onButtonClick(e) {
    this.dispatchEvent(new FooEvent());
  }
}

See some similar previous discussion here: lit/lit-element#808

@daKmoR
Copy link
Contributor

daKmoR commented Feb 12, 2020

many questions 😬 😅
how is onFoo connected to _onButtonClick or _onButtonClick?
what would users of the component do? <my-element @onFoo=${}>?

@justinfagnani
Copy link
Collaborator Author

@daKmoR _onButtonClick is just there to should how a component would fire a declared event - ie, highlight that it's the same. There are other event-helper APIs out there that assist in firing events, but I don't think they buy much, especially if you need to pass constructor params or get the return value of dispatchEvent. I think this.dispatchEvent(new FooEvent()) is about as concise as a helper would be anyway.

How this is used depends on the context, but is exactly like built-in element event handler properties:

Plain JS:

const el = document.querySelector('my-element');
el.onFoo = fooHandler;

lit-html, as usual:

html`<my-element @foo=${fooHandler}></my-element>`

lit-html, using the event handler property:

html`<my-element .onFoo=${fooHandler}></my-element>`

React:

import {MyElement as MyCustomElement} from 'my-element';
import {createReactComponent} from 'lit-element/react';
MyElement = createReactComponent(MyElement);

const ParentComponent = (props) =>
  <MyElement onFoo=${props.fooHandler}></MyElement>;

@CaptainCodeman
Copy link

how is onFoo connected to _onButtonClick or _onButtonClick?

That was my first reaction too, but I guess it's like ye-olde standard html event handler properties such as onclick (whatever the official name for them is)

I'm guessing this would help with generating docs about an element (?)

@justinfagnani
Copy link
Collaborator Author

I'm guessing this would help with generating docs about an element

Yep. And help implement the createReactComponent() function, and help with some Google-internal framework integration projects we have going on.

@web-padawan
Copy link
Contributor

At Vaadin we do have a strong need for for typed events. Here are some of our use cases:

Events with generic types

As an example, in vaadin-grid we would like to support the following:

render() {
  return html`
    <vaadin-grid
      .items=${this.contacts}
      @active-item-changed=${(e: GridActiveItemChangedEvent<Contact>) =>
      this.currentContact = e.detail.value}
    ></vaadin-grid>
  `;
}

See also vaadin/vaadin-grid#2071

Events with same name but different payload

We have several components with value property, and the type of that property might vary depending on the component:

  • vaadin-text-field has value property of type string
  • vaadin-checkbox-group has value property of type string[]

So we can't declare value-changed under HTMLElementEventMap in a way that would satisfy all the options.

See also runem/lit-analyzer#143

@ChristianUlbrich
Copy link

ChristianUlbrich commented Nov 26, 2020

How about some prior art? StencilJS has for the same reasons an @Event decorator. Although they just use it for tagging (and then doing their ub3r-cust0m TypeScript transform magic):

borrowing:

export interface EventOptions {
  /**
   * A string custom event name to override the default.
   */
  eventName?: string;
  /**
   * A Boolean indicating whether the event bubbles up through the DOM or not.
   */
  bubbles?: boolean;

  /**
   * A Boolean indicating whether the event is cancelable.
   */
  cancelable?: boolean;

  /**
   * A Boolean value indicating whether or not the event can bubble across the boundary between the shadow DOM and the regular DOM.
   */
  composed?: boolean;
}

To create a decorator @event with its usage something like:

// TODO: utilize TS function overloading stuff, to prevent using eventOptions twice
function event(eventOptionsOrName: EventOptions | string, eventOptions?: EventOptions) {
	return (target, propKey) => {
    // ... generate sth. like this.dispatchEvent() with above set options...
  };
}

@event('selected')
emitSelectedEvent!: (payload?:EventPayload<SelectedEvent>) => EventEmitter<SelectedEvent>

I think the bare minimum you will need for static type analysis and thus some code generation logic depending on the TypeScript AST is the actual eventName on compile time. If you want to actually generate the dispatchEvent()call you need at runtime the eventName and the eventOptions as well. You might have some special needs for Events inside the ShadowDOM, vulgo one might want custom event to either bubble or compose.

@landrito
Copy link

landrito commented Dec 9, 2020

I took a stab at seeing if we could declare the events as a Type parameter to the LitElement class such that:

  1. If you don't parameterize on any Event types, the Class works as it does today.
  2. calling this.dispatchEvent inside a LitElement instance constrains the event parameter to be of the event types specified in the type params.
type TypedEvent<T extends string, E extends Event> = {readonly type: T} & E;

// Union with void and assign to void to make this optional. 
export class LitElement<DispatchableEvents extends TypedEvent<string, Event> | void = void> extends UpdatingElement {

  // If the DispatchableEvents is a TypedEvent use the type, otherwise simply use the event as normal.
  dispatchEvent(e: DispatchableEvents extends TypedEvent<infer T, infer E> ? TypedEvent<T, E> : Event): boolean {
    // Propagate call to EventTarget superclass
    return super.dispatchEvent(e);
  }
  ...
}

There is slight ergonomic issues in this solution which can be seen in how this would be used:

type TestEvent = TypedEvent<'testEvent', CustomEvent<string>>;
type OtherTestEvent = TypedEvent<'otherTestEvent', MouseEvent>;
class TestElement extends LitElement<TestEvent | OtherTestEvent> {
  emit() {
    // This seems a little tedious since the type field is already set to 'testEvent' in the 
    // event constructor.
    this.dispatchEvent({...new CustomEvent<string>('hello'), type: 'testEvent'})

    // Expected Error: Argument of type 'Event' is not assignable to parameter of type 
    // 'TypedEvent<"testEvent", CustomEvent<string>> | TypedEvent<"otherTestEvent", MouseEvent>'.
    this.dispatchEvent(new Event('notDeclared')); 
  }
}

The main hurdle I had to overcome in this was the dom Event type:

interface Event {
    readonly type: string;
}

declare var Event: {
    new(type: string, eventInitDict?: EventInit): Event;
};

Notice how the param type in the constructor is of type string. Ideally this type would pass through a string literal type if a string literal was used.

In an ideal world the Event type would look like this:

interface Event<Type extends string = string> {
    readonly type: Type;
}

declare var Event: {
    new <Type extends string = string>(type: Type, eventInitDict?: EventInit): Event<Type>;
};

@landrito
Copy link

landrito commented Dec 9, 2020

If we could get the Event and all extends Event types to follow the update above, the interface for this becomes much simpler:

// No more need of this discriminated type intersection.
// type TypedEvent<T extends string, E extends Event> = {readonly type: T} & E;

// The types which the params extend become much more readable.
export class LitElement<DispatchableEvents extends Event | void = void> extends UpdatingElement {
  // You can only call dispatchEvent with Events discriminated by string literals on member `type`.
  // `string extends Type` checks if the Events is using string literal types.
  dispatchEvent(e: DispatchableEvents extends Event<infer Type> ? string extends Type ? never : DispatchableEvents : Event): boolean {
    return super.dispatchEvent(e);
  }

Looking at the usage, it also seems much more idiomatic (with one hard to understand error).

class TestElement extends LitElement<CustomEvent<string, 'testEvent'> | Event<'otherEvent'>> {
  emit() {
    // Works as intended
    this.dispatchEvent(new CustomEvent('testEvent', {detail: 'test'}));
    this.dispatchEvent(new Event('otherEvent'));

    // Compiler Error as intended
    // Error: Type 'Event<"testEvent">' is missing the following properties from type 'CustomEvent<string, "testEvent">': detail, initCustomEvent
    this.dispatchEvent(new Event('testEvent'))
    // Error: Argument of type 'Event<"notDeclared">' is not assignable to parameter of type 'CustomEvent<string, "testEvent"> | Event<"otherEvent">'
    this.dispatchEvent(new Event('notDeclared'))
  }
}

class NoEventTypes extends LitElement {
  emit() {
    // All work as intended
    this.dispatchEvent(new CustomEvent('testEvent', {detail: 'test'}));
    this.dispatchEvent(new Event('otherEvent'));
    this.dispatchEvent(new Event('testEvent'))
    this.dispatchEvent(new Event('notDeclared'))
  }
}

class EventTypesDontDeclareType extends LitElement<MouseEvent> {
  emit() {
    // Expected error here but the error message is unhelpful. Ideally the error would be in the untyped 
    // Event passed to the LitElement Type Params.
    // Error: Argument of type 'MouseEvent' is not assignable to parameter of type 'never'.
    this.dispatchEvent(new MouseEvent('hello'))
  }
}

Unfortunately I don't know how hard it would be to change the types found in https://github.com/microsoft/TypeScript/blob/master/lib/lib.dom.d.ts#L5285. Theoretically, we shouldn't get much friction if the type is correct (especially since it isn't a breaking change) but I'd have to check with the W3C spec to ensure the type param in the Event and Event subclass constructors all directly map to the type property in the Event class.

@ChristianUlbrich
Copy link

ChristianUlbrich commented Dec 10, 2020

@landrito
The idea of overwriting the type signature of this.dispatchEvent and thus making it type-safe is intriguing. However, there are several drawbacks to this approach:

  • messing around with LitElement directly; if going that route, I think sth. along a class-based mixin expression (btw. @justinfagnani nice work back then in 2015, before everyone was crazy about HOC!) would be the better solution
  • while this solution would make this.dispatchEvent type-safe, it does not foster abstraction of it
  • it is TS only; whereas LitElement IMO at least tries to also support Vanilla JS.
  • your chosen abstraction is not very declarative and thus hard to reason about, if used in AST analysis

The above reasons are good justifications for specific event emitters, i.e. dedicated functions emitting specialized events in a 1:1 mapping. This fosters abstraction of dispatching events and gives you also a single source of truth for them. It is not only a proven pattern for emitting events in other frameworks as well, but it combination with decorators this plays nicely along with the declarative approach that LitElement is currently using anyway (@property). Decorators make code generations also easy; with my above rough draft I could easily cobble up a react component generation wrapper; this

Having that said, I think your approach could benefit event emitters as well, adding type-safety to them.

@landrito
Copy link

Thanks for the detailed and thoughtful response @ChristianUlbrich!

  • messing around with LitElement directly; if going that route, I think sth. along a class-based mixin expression (btw. @justinfagnani nice work back then in 2015, before everyone was crazy about HOC!) would be the better solution

Agreed.

  • while this solution would make this.dispatchEvent type-safe, it does not foster abstraction of it
  • your chosen abstraction is not very declarative and thus hard to reason about, if used in AST analysis

Also agreed, I definitely misunderstood the problems this is issue is trying to address.

Having that said, I think your approach could benefit event emitters as well, adding type-safety to them.

This was definitely my main goal with this; to see if there was a way to make the emission of an event to be type safe based on the event name used.

@landrito
Copy link

landrito commented Dec 10, 2020

To tie this back to the original proposal:

class MyElement extends LitElement {
  @event() onFoo!: (e: FooEvent) => any) | null;

  render() {
    html`<button @click=${this._onButtonClick}>Click Me</button>`;
  }

  _onButtonClick(e) {
    this.dispatchEvent(new FooEvent());
  }
}

interface HTMLElementEventMap {
  'foo': FooEvent;
}

The interface HTMLElementEventMap above is unnecessary for Events with string literal types for their type field. Assume FooEvent is declared as follows:

class FooEvent extends Event {
  readonly type!: 'foo';

  constructor() {
    super('foo');
  }
}

Furthermore I'd propose that the Decorator Options provided are specific to the Event in question. For example, the opts @ChristianUlbrich provided above could be updated to:

export type EventName<E extends Event | void = void> =
  E extends {type: infer TypeAttribute} ? TypeAttribute : string;

export interface EventOptions<E extends Event | void = void> {
  /**
   * A string custom event name to override the default.
   */
  eventName?: EventName<E>
}

const opts: EventOptions<FooEvent> = {
  // Expected Error: Type '"Bar"' is not assignable to type '"foo" | undefined'.
  eventName: 'Bar'
}

There is a problem though. Typescript doesn't seem to support generic decorators very well meaning the type of the decorators target is not propagated to the decorator function: microsoft/TypeScript#2607.

@kevinpschaaf kevinpschaaf changed the title Declarative event emitters [lit-element] Declarative event emitters Sep 8, 2022
@kevinpschaaf kevinpschaaf transferred this issue from lit/lit-element Sep 8, 2022
@kevinpschaaf kevinpschaaf moved this to 📋 Triaged in Lit Project Board Sep 8, 2022
@LAC-Tech
Copy link

LAC-Tech commented Mar 7, 2023

This would really take Lit to the next level IMO. If we could declaratively generate statically typed onFooEvent methods for our own components they really start to feel like first class parts of html. Digging the proposed solutions.

@justinfagnani
Copy link
Collaborator Author

This should be an RFC

@sijakret
Copy link

Hi All,

I also think some resolution here (might be possible to do via mixin) would be very helpful!
As already mentioned in this thread and previous discussions. I would summarize the main aspects of a solution like this (please let me know if you agree/disagree).

  • typed dispatch:

    To make sure event types are linked with definitive event shapes/interfaces. prevents emission of illegal events.

  • conciseness/ergonimics:

    Obviously the solution should be as transparent as possible

  • single source of truth:

    Redundant annotations that need to be kept in alignment should be avoided (lik ts typings + @fires/@event annotations). This menas, for instance the web component analyzer should be able to pick up the event declarations somehow. It is unclear to me if the decorators proposed here would enable this.

  • no (or little) runtime overhead:

    Since (as opposed to pros) events are only created at runtime they need no permanent field in a component class. To me this is kind of my main gripe with the prop decorator-based solutions proposed here and in the past. Maybe the declaration can be a static field in the component?

  • enumeration of event(types):

    The ability to enumerate the events at runtime + typings would allow to fully automate the creation of react wrappers and probably for angular as well. I guess the requmrent to list events when wrapping WCs stems from the lack of enumerability of the events. Shouldn't a solution to declaring events also solve the this wrapper generation issue?

Below i try to categorize the solution proposed here (prop decorators) according to the criteria above.
A few points are not clear to me.

prop decorators (this thread, here and here)

  • typed dispatch:

    ✅ yes

  • conciseness/ergonimics:

    ✅ clear

  • single source of truth:

    It is unclear to me if the WC analyzer could pick these decorators up, can someone enlighten me?

  • no (or little) runtime overhead:

    a prop decorator will provision a property for each component instance. for a component which emits many different types of events this is not ideal i think.

  • enumeration:

    ✅ enumerating the events + their types for a component type at runtime (say for generating wrappers) should be possible with this.

Are there realistic alternatives to this even? I have sketched up an approach in the past that uses a mixin + a class that enumerates the events as static fields. It avoids creating one property per Event and instead creates a static member that also allows enumeration per component class. Unfortunately it feels a bit more clunky that decorators. If this is even of interest i can create a gist..

@sijakret
Copy link

sijakret commented May 28, 2024

This topic has come up again in our project (relatively large enterprise design system).
I was wondering:

  • are there plans to address this topic in lit?
  • has anyone taken the route of event decorators and has some feedback/learnings?
    thanks in advance!

Edit:
I also noticed that the @lit-labs/cli uses it's internal @lit-labs/analyzer and i have a question here:
Currently it is not trivial to have one source of truth for the declaration events and their types.
If you use js-doc @ fires annotations but emit something else in your code the @lit-labs/analyzer will not pick it up and it does happen that the @ fires annotation and the code are not aligned.
This requires a constant effort to align the @ fires annotations and the code.
Ideally whatever resolution is found here makes sure that there is typed event emission that also is picked up by the analyzer and thus ends up downstream operations (like documentation/wrapper generators).

So I was wondering:
why does the @lit-labs/cli not use @custom-elements-manifest/analyzer?
That one has a plugin system that would at least allow to automatically extract event definitions from the code using a custom analysis plugin. This would enable custom solutions, at least until a resolution to this issue here lands.

@TazmanianDI
Copy link

I'm very much interested in this as well. It seems that you can either handle events through callback attributes/properties or through event listeners. I know there are arguments for using the event listener since you get some decoupling and the ability for multiple listeners but the fact that the callback attributes gets you declared events and strong typing seems to be a big reason to use those over emitting events. I know you've got to deal with the annoying this binding the lit does automatically for event listeners but seems worth the annoyance.

@tbroyer
Copy link

tbroyer commented Nov 6, 2024

Fwiw, I have an implementation of such a decorator at https://github.com/tbroyer/platformer/blob/main/packages/lit/event-handler.js, that also handles onfoo="some JS code" attributes (this accounts for more than 50% of the code).

Assuming you don't want to support the attribute, this wouldn't be that much code. One thing to note: the event type would have to be an option to the decorator (for cases where it cannot directly be used as a JS identifier). It could also be applied to a setter for cases when the element needs to do things on setting.

Fwiw, for details about how HTML event handlers work on builtin elements, I published a blog post about it: https://blog.ltgt.net/html-event-handlers/

Proof of concept implementation

I'm using JS here because typing is a bit complex.

export function event({ type } = {}) {
  return function ({ get, set }, context) {
    // TODO: check that the property is public, non-static, with a name that starts with 'on'
    type ??= context.name.substring(2);
    
    const listener = (e) => {
      const callback = context.access.get();
      if (callback == null || typeof callback !== "function") return;
      let returnValue = callback.call(e.currentTarget, e);
      if (returnValue === false) {
        e.preventDefault();
      }
    };
    let abortController;
    return {
      get,
      set(value) {
        // TODO: type coercion? (WebIDL's [LegacyTreatNonObjectAsNull] callback function)
        if (value == null) {
          abortController?.abort();
          abortController = null;
        } else if (abortController == null) {
          abortController = new AbortController();
          this.addEventListener(type, listener, { signal: abortController.signal });
        }
      }
    }
  }
}

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

No branches or pull requests