Polyfill the new EventListener API and use passive: true where makes sense.
#5109
Conversation
|
Code LGTM. I'd rather see this as an exported function, instead of a polyfill. |
|
I don't feel strongly either way but considering FF and Chrome have it and Safari has it in nightly already, I think polyfill makes sense. |
|
One benefit of a function would be that we could make passive the default :) |
|
Let's think about this in terms of API: would we rather continue using |
| * TODO(aghassemi, #5105): Type check | ||
| * @suppress {checkTypes} | ||
| */ | ||
| (() => { |
There was a problem hiding this comment.
this way I can get @Suppress to only apply to that code (I tried to do this with code blocks in {} too but looks like these annotation only work at function scope)
|
|
||
| /** | ||
| * TODO(aghassemi, #5105): Type check | ||
| * @suppress {checkTypes} |
There was a problem hiding this comment.
What's complication with type checking? Is it still not in externs? If so, please file a bug against closure.
There was a problem hiding this comment.
I talked to them, there is an open PR to update the extern, they said they will review and merge in a week or so.
| /** | ||
| * Decides whether capture argument should be true/false based on the value of | ||
| * then given options argument. | ||
| * @param {*} options |
| /** | ||
| * @param {!Window} win | ||
| */ | ||
| export function install(win) { |
There was a problem hiding this comment.
This is another thing that needs to be polyfiled in a friendly frame. If we do proceed with this, please also call this polyfill from installPolyfillsInChildWindow in extensions-impl.js.
|
looks like consensus is not polyfilling, I will move this to I will also create an issue for us to migrate all sounds good? |
|
👍 |
|
/cc @dvoytenko |
With this polyfill we can pass an
optionsobject as the third argument toaddEventListenerandremoveEventListenerregardless of browser support. The real need for this polyfill arises from the fact that browsers that expect third-argument to be theboolean capture, would interpret an object as truthy and setcapturetotrue, which is not what we want.Please note that actual individual options are not polyfilled (something like
passivecan't truly be polyfilled anyway)Closes #4819
See #5105 for Closure Compiler extern issues related to this change and hence the
@suppressblocks./to @jridgewell for review
/cc @dvoytenko @cramforce