-
Notifications
You must be signed in to change notification settings - Fork 386
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
fix: event listener not being invoked for native web components #4012
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks reasonable to me, and I imagine the risk of breakage is low since it only affects native-within-synthetic.
One thing that may be useful is to add a test for a native shadow LWC component inside of the parent synthetic shadow container. AIUI, this doesn't have anything to do with lwc:external
or third-party custom elements – it just has to do with native-shadow-inside-synthetic-shadow.
packages/@lwc/integration-karma/test/mixed-shadow-mode/contextualComposedPath/index.spec.js
Outdated
Show resolved
Hide resolved
...ages/@lwc/integration-karma/test/mixed-shadow-mode/contextualComposedPath/x/parent/parent.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great starting point, let's beef up the tests next week!
packages/@lwc/synthetic-shadow/src/polyfills/event-target/polyfill.ts
Outdated
Show resolved
Hide resolved
packages/@lwc/integration-karma/test/mixed-shadow-mode/contextualComposedPath/index.spec.js
Outdated
Show resolved
Hide resolved
packages/@lwc/integration-karma/test/mixed-shadow-mode/contextualComposedPath/index.spec.js
Outdated
Show resolved
Hide resolved
packages/@lwc/integration-karma/test/mixed-shadow-mode/contextualComposedPath/index.spec.js
Outdated
Show resolved
Hide resolved
...ages/@lwc/integration-karma/test/mixed-shadow-mode/contextualComposedPath/x/parent/parent.js
Outdated
Show resolved
Hide resolved
4dd18b2
to
fe0abfc
Compare
* fix: event listener not being invoked for native web components * chore: simplify test * chore: test tweaks * chore: also avoid wrapping listeners for non-Node EventTargets * chore: account for light dom nodes * chore: account for window --------- Co-authored-by: Eugene Kashida <ekashida@gmail.com>
… (#4080) Co-authored-by: Jason Ye <40873183+jye-sf@users.noreply.github.com>
Details
Addresses #4011.
This PR attempts to bypass our
addEventListener()
patch when working with event targets under a native shadow root.Remaining work:
removeEventListener()
to avoid inconsistent behavior?Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?
This changes the composed path in some scenarios. The full, explicit list of scenarios may need to be explored.
GUS work item