lit-element
lit-element copied to clipboard
[feature request] create an listen decorator for host listeners
Description
- Who wants the functionality
- people who want to declaratively add listeners to host
- What it is they want
- A
@listen
decorator to declaratively add event listeners to host - I don't necessarily think a "listeners" block should need exist for JS. A helper for TS is sufficient I think.
- A
- Why they want it
- it's declarative
- helpful for serialization
- having to deal with unregistering listeners on disconnect
- having to deal with possibly
.bind
and listener references is annoying
- Functional description of task/subtask
- Create a new decorator called
@listen
- Create a new decorator called
Acceptance criteria
What the card must do in order to accept it as complete. Acceptance Criteria must be concrete or measurable. A TS decorator to add event listeners to host.
@listen('click')
@listen('click', { target: 'document' })
@listen('resize', { target: 'window' })
Not sure whether you'd want to allow addEventListener
options here. Since there is already @eventOptions
My attempt at this:
type EventHandler = (event: Event) => boolean | void;
interface CustomElement extends Element {
connectedCallback: () => void;
disconnectedCallback: () => void;
}
export function listen(eventName: string) {
return (target: CustomElement, key: string): void => {
const { connectedCallback, disconnectedCallback } = target;
let eventHandler: EventHandler;
target.connectedCallback = function () {
connectedCallback.call(this);
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
eventHandler = (event: Event) => (this as any)[key](event);
this.addEventListener(eventName, eventHandler);
};
target.disconnectedCallback = function () {
this.removeEventListener(eventName, eventHandler);
disconnectedCallback.call(this);
};
};
}
We've considered this before and haven't done it yet mainly because this.addEventListener()
in the constructor works pretty well already for host listeners.
Where we've added decorators so far have been cases where we need to transform an existing JS construct (@property and @internalProperty) or we want a declarative form for analysis (@customElement) or a declarative form to have a spot to hang type information (@query and friends).
I'm not sure if @listen()
fits those or not, in that we don't need to transform the method, need type info on the method, or analyze it. It's "just" sugar. Sugar is a fine reason to add something sometimes though, if we can come up with a form that fits enough of the use cases. The one I find most compelling is listeners not on the host that need to be removed in disconnected to prevent memory leaks. This case requires hooking lifecycle callbacks.
Since we already had a controller API in lit-next that can hook lifecycles, it seems like all we need to allow @listen()
to be implemented without base class support or patching is a way to add initializer hooks to the class, which can in turn add controllers to instances. I filed https://github.com/Polymer/lit-html/issues/1663 to track this, and @sorvell is working on it.
They're also good to prevent spots where anti-patterns may arise like memory leaks from element references due to listeners or when I was speaking to a community member about the @watch
directive they created, calling it after super.update
which may cause unnecessary rerenders.
This not to mention that internally we aren't even allowed to use .bind
and must rewrap all of our method references in () =>{}
and pass ags and return values
Coming back to this because of the frustration the following pattern gives me:
@customElement('my-element')
class MyElement extends LitElement {
private boundOnClick = (_e: MouseEvent) => {}; // hopefully some type that satisfies addEventListener in TS
connectedCallback() {
super.connectedCallback();
this.boundOnClick = this.onClick.bind(this);
// internally at google the line above has to be
// this.boundOnClick = (e: mouseEvent) => (this.onClick(e));
// because we are not allowed the use of `bind`
this.addEventListener('click', this.boundOnClick, {capture: true});
}
disconnectedCallback() {
super.disconnectedCallback();
this.removeEventListener('click', this.boundOnClick);
}
onClick(e: MouseEvent) {...}
}
as opposed to something like:
@customElement('my-element')
class MyElement extends LitElement {
@eventOptions({capture: true}) // tbh not sure if the implementation of eventOptions would allow this
@hostListener('click', {removeOnDisconnect: true, bind: true}) // these likely could be default
onClick(e: MouseEvent) {...}
}
Closing in favor of https://github.com/lit/rfcs/issues/11 since this should be included and/or covered by that proposal.