Adds support for event proxying and event options #856
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit f2f2d67:
|
matt-gadd
force-pushed
the
passive-events
branch
from
8d2bc7d to
d570feb
Compare
Codecov Report
@@ Coverage Diff @@
## master #856 +/- ##
==========================================
- Coverage 94.83% 94.71% -0.12%
==========================================
Files 127 127
Lines 8049 8085 +36
Branches 1870 1884 +14
==========================================
+ Hits 7633 7658 +25
- Misses 416 427 +11
Continue to review full report at Codecov.
|
agubler
left a comment
agubler
left a comment
There was a problem hiding this comment.
Just a couple of type suggestions, otherwise 👍
src/core/vdom.ts
Outdated
| if (eventCallback) { | ||
| domNode.removeEventListener(eventName, eventCallback); | ||
| const proxyEvents = _eventMap.get(domNode) || {}; | ||
| let proxyEvent: { proxy: EventListener; callback: Function; options: any } | undefined = |
There was a problem hiding this comment.
Could this be EventMapValue | undefined?
There was a problem hiding this comment.
yeah it can actually just be inferred, i just forgot to remove it.
src/core/vdom.ts
Outdated
| return props; | ||
| } | ||
|
|
||
| type EventMapValue = { proxy: EventListener; callback: Function; options: any } | undefined; |
There was a problem hiding this comment.
Could we type options? passive: boolean or similar?
There was a problem hiding this comment.
I wanted to actually use EventListenerOptions but I think the type didn't have passive. I'll type it though.
2 participants
Type: feature
The following has been addressed in the PR:
prettieras per the readme code style guidelinesDescription:
This adds support for 2 features:
Event Proxying
Before this change, events were removed and re-attached if the callback changed across renders. Unfortunately this is a very common occurrence due to inline functions encouraged. With this change, a wrapping proxy is used which simply points to the callback, which means we do not have to re-attach the event.
Event Options
Added a new special typed property called
oneventoptionswhich currently supportspassive; an array of supported eventlisteners. This allows you to set whether an event is passive or not.(https://developers.google.com/web/updates/2016/06/passive-event-listeners).Squatting on a property on a
VNodePropertiesis not ideal, but it is at least prefixed with the existingonsquat. And this method is the least intrusive to the existing behaviour.Example usage:
Event options are not supported on
addEventListenerat all in older browsers (IE) so a sniff has been added tohascalleddom-passive-event.Resolves #810