(engine-restrictions) Lift restrictions on addEventListener's options param for host element

[ravijayaramappa]

Description

Adding an event listener on the custom element with a third options argument logs an error.

This restriction should be on the component instance and not on the host element. From the outside world, a DOM element should behave per spec when addEventListener is invoked with the options param, regardless of whether the element is a standard html element or a native custom element or an LWC host element.

Steps to Reproduce

https://playground.lwcjs.org/projects/d1fzv_d_h/3/edit

import { LightningElement, track } from 'lwc';

export default class App extends LightningElement {
    renderedCallback() {
        this.addEventListener('foo', () => {}, { passive: true });
    }
}

Reference: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener

[caridy]

FYI, this is probably just moving the following block:

lwc/packages/@lwc/engine/src/framework/restrictions.ts

Lines 338 to 370 in 9e19ce5

	addEventListener: generateDataDescriptor({
	value(
	this: HTMLElement,
	type: string,
	listener: EventListener,
	options?: boolean | AddEventListenerOptions
	) {
	const vmBeingRendered = getVMBeingRendered();
	assert.invariant(
	!isInvokingRender,
	`${vmBeingRendered}.render() method has side effects on the state of ${toString(
	this
	)} by adding an event listener for "${type}".`
	);
	assert.invariant(
	!isUpdatingTemplate,
	`Updating the template of ${vmBeingRendered} has side effects on the state of ${toString(
	elm
	)} by adding an event listener for "${type}".`
	);
	// TODO [#420]: this is triggered when the component author attempts to add a listener
	// programmatically into a lighting element node
	if (!isUndefined(options)) {
	logError(
	'The `addEventListener` method in `LightningElement` does not support any options.',
	getAssociatedVMIfPresent(this)
	);
	}
	// Typescript does not like it when you treat the `arguments` object as an array
	// @ts-ignore type-mismatch
	return originalAddEventListener.apply(this, arguments);
	},
	}),

to

lwc/packages/@lwc/engine/src/framework/restrictions.ts

Line 405 in 9e19ce5

const descriptors: PropertyDescriptorMap = {

[git2gus]

This issue has been linked to a new work item: W-7446526

[ekashida]

I'd have to dig in a little more but giving users the ability to subscribe to the capture phase of focusin might require us to refactor our delegatesFocus implementation. It might be fine as long as we always run first, not sure...

[ekashida]

Looks like we only restrict this for the host element:
https://playground.lwcjs.org/projects/LY4Z_tvOv/1/edit

[ekashida]

When introducing support for options, we'll also need to update the existing logic to prevent multiple listeners for the same event since we currently only consider the identity of the listener function.

Some points to consider when determining whether a listener is "identical":

1. Only the configuration parameters of the option object is considered and the values are read once when binding (it doesn't matter if the values change over time). This means that passing in true and { capture: true } are equivalent.

2. IE11 doesn't support option objects, it just cares whether the third argument is truthy or not. We'd need to detect browser behavior to support this since different option objects should be considered equivalent.

[bk-dev]

Hello, there. I'm also seeing this behavior in our project. I am using LWCs as web components in my app and would like to use the third optional parameter when binding event listeners to them. When running in development mode the LWC runtime throws an error for each usage. @pmdartus

[nolanlawson]

Updated repro using StackBlitz for posterity

FWIW it seems to log an error, not throw an error.

[fafnirical]

I just ran into this issue when trying to use LWC in Storybook 7, which requires lit-html/lit v2. It doesn't seem to be an issue with lit-html v1, though it still logs the error (but doesn't throw it). Have there been any updates since 2021?

[nolanlawson]

@fafnirical Is the issue just that it's logging an error? Or is there an actual functional difference?

[fafnirical]

@fafnirical Is the issue just that it's logging an error? Or is there an actual functional difference?

@nolanlawson There's an actual functional difference. In addition to the logged error (LWC error]: The `addEventListener` method in `LightningElement` does not support any options.) I see when rendered by both lit-html v1 and v2, lit-html v2 causes LWC to throw the following error in Storybook:

vendors~main.iframe.bundle.js:1475 Uncaught (in promise) TypeError: Invalid second argument for Element.addEventListener() in [object HTMLElement] for event "changesegment". Expected an EventListener but received [object Object].
    at HTMLBridgeElement.addCustomElementEventListener (vendors~main.iframe.bundle.js:1475:13)
    at HTMLBridgeElement.patchedAddEventListener$1 [as addEventListener] (vendors~main.iframe.bundle.js:4318:42)
    at HTMLBridgeElement.value [as addEventListener] (engine-dom.js:1422:41)
    at L._$AI (vendors~main.iframe.bundle.js:72132:84)
    at S.v (vendors~main.iframe.bundle.js:71920:112)
    at M.g (vendors~main.iframe.bundle.js:71991:13)
    at M._$AI (vendors~main.iframe.bundle.js:71964:191)
    at c.setValue (vendors~main.iframe.bundle.js:71166:122)
    at _callee$ (vendors~main.iframe.bundle.js:71624:65)
    at tryCatch (vendors~main.iframe.bundle.js:71540:1357)

The source of the error is https://github.com/salesforce/lwc/blob/v2.43.0/packages/@lwc/synthetic-shadow/src/faux-shadow/events.ts#L210-L214

And the code for the story:

/// myLwcComponent.stories.ts
import { html } from 'lit-html';

const handleEvent = (e: CustomEvent) => console.log(e);

export const example = () => html`<my-lwc-component @changesegment=${handleEvent}></my-lwc-component>`;

[nolanlawson]

I see, so this is a synthetic shadow issue. Synthetic shadow is deprecated, and we had not planned to make any changes to it. But I can understand if you cannot remove synthetic shadow from your storybook testing.

[nolanlawson]

Ah, I also just noticed that this error is only thrown in non-prod mode. Maybe we should tie this into #3245 then.

lwc/packages/@lwc/synthetic-shadow/src/faux-shadow/events.ts

Lines 208 to 216 in 3770a62

	if (process.env.NODE_ENV !== 'production') {
	if (!isFunction(listener)) {
	throw new TypeError(
	`Invalid second argument for Element.addEventListener() in ${toString(
	this
	)} for event "${type}". Expected an EventListener but received ${listener}.`
	);
	}
	}

[nolanlawson]

@fafnirical Actually, I don't understand how you're hitting this error. Your error message is:

Invalid second argument for Element.addEventListener() in [object HTMLElement] for event 
"changesegment". Expected an EventListener but received [object Object].

How is the second argument to addEventListener an object and not a function? Per MDN, the second argument should always be a function.

[nolanlawson]

Nevermind, I get it. The second argument can be an object with a handleEvent method. Looks like that is exactly what Lit is using.

[jmsjtu]

This issue only logs a warning, it doesn't throw an error. Adding to our trust backlog.

[git2gus]

This issue has been linked to a new work item: W-15704155