Parent discussion: Event listener binding conventions (#8221)
Context
There was a recurring bug: .bind(this) called separately at addEventListener and removeEventListener produces two different function objects each time, so removal silently never matches and the listener is never detached.
Separately, slotted/consumer-owned elements (post-menu-trigger.tsx, post-number-input.tsx) never get their listeners cleaned up at all. Native Stencil @Listen() solves this well for host/window/document/body, but cannot target arbitrary slotted elements, (confirmed this by testing it directly: TypeScript rejects other targets, and forcing it past the type system either crashes the compiler or is silently ignored at runtime).
Decision
Two options, to be voted on:
Option 1: Lint rule only. Add an ESLint rule flagging .bind(this) (or any non-identifier expression) passed directly into addEventListener/removeEventListener, and requiring a matching removal in disconnectedCallback for any manual listener. This also means picking one of our two existing correct patterns as the standard going forward, so the rule has a single answer for "do it like this instead":
Option 1a: Class-field arrow function
private readonly handleResize = () => {
// ...
};
connectedCallback() {
globalThis.addEventListener('resize', this.handleResize);
}
disconnectedCallback() {
globalThis.removeEventListener('resize', this.handleResize);
}Option 1b: Constructor-bound method
constructor() {
this.handleResize = this.handleResize.bind(this);
}
private handleResize() {
// ...
}
connectedCallback() {
globalThis.addEventListener('resize', this.handleResize);
}
disconnectedCallback() {
globalThis.removeEventListener('resize', this.handleResize);
}Option 2: Custom @Listen-style decorator + lint rule. It is already confirmed native @Listen() can't reach slotted elements directly, so we'd build our own decorator handling stable binding and automatic cleanup for slotted/arbitrary targets, plus the same lint rule to enforce its use.
Consequences
- Option 1: Cheapest to set up, but still mean writing the full manual pattern by hand each time, and the lint rule only catches mistakes after they're written, it doesn't prevent them.
- Option 2: Closes the bug class structurally for slotted elements too, nothing left to misuse. Costs ongoing maintenance of a custom decorator.
- Either way: whichever we pick, sub-option of Option 1 or Option 2, means going through the existing components and changing them over to the chosen pattern; none of these are purely additive.
Parent discussion: Event listener binding conventions (#8221)
Context
There was a recurring bug:
.bind(this)called separately ataddEventListenerandremoveEventListenerproduces two different function objects each time, so removal silently never matches and the listener is never detached.Separately, slotted/consumer-owned elements (
post-menu-trigger.tsx,post-number-input.tsx) never get their listeners cleaned up at all. Native Stencil@Listen()solves this well forhost/window/document/body, but cannot target arbitrary slotted elements, (confirmed this by testing it directly: TypeScript rejects other targets, and forcing it past the type system either crashes the compiler or is silently ignored at runtime).Decision
Two options, to be voted on:
Option 1: Lint rule only. Add an ESLint rule flagging
.bind(this)(or any non-identifier expression) passed directly intoaddEventListener/removeEventListener, and requiring a matching removal indisconnectedCallbackfor any manual listener. This also means picking one of our two existing correct patterns as the standard going forward, so the rule has a single answer for "do it like this instead":Option 1a: Class-field arrow function
Option 1b: Constructor-bound method
Option 2: Custom
@Listen-style decorator + lint rule. It is already confirmed native@Listen()can't reach slotted elements directly, so we'd build our own decorator handling stable binding and automatic cleanup for slotted/arbitrary targets, plus the same lint rule to enforce its use.Consequences