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

refactor(engine): addEventListener restriction options param #1826

Closed
wants to merge 1 commit into from
Closed

refactor(engine): addEventListener restriction options param #1826

wants to merge 1 commit into from

Conversation

aputinski
Copy link
Contributor

@aputinski aputinski commented Apr 14, 2020

Details

Closes #1824

Does this PR introduce breaking changes?

  • No, it does not introduce breaking changes.

GUS work item

W-7446526

@aputinski aputinski changed the title fix(engine-restrictions): lift restrictions on addEventListener's options param for host element Lift restrictions on addEventListener's options param for host element Apr 14, 2020
@aputinski aputinski changed the title Lift restrictions on addEventListener's options param for host element fix(engine-restrictions): addEventListener options for host element Apr 14, 2020
@pmdartus pmdartus changed the title fix(engine-restrictions): addEventListener options for host element refactor(engine): restrictions on addEventListener's options param for host element Apr 14, 2020
@pmdartus pmdartus changed the title refactor(engine): restrictions on addEventListener's options param for host element refactor(engine): addEventListener restriction options param Apr 14, 2020
Copy link
Collaborator

@caridy caridy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nip on the types

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.
@caridy
Copy link
Collaborator

caridy commented Apr 14, 2020

Ok, this code is now ready, but after discussing the details with @ravijayaramappa, we looked at the code, and in fact, the custom element is being patched, eliminating the ability to use options when using synthetic shadow. We have found a couple of ways to allow this, but we will have to put some time to solve this.

@pmdartus
Copy link
Member

Closing this PR based on @caridy's feedback.

@pmdartus pmdartus closed this May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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