Fix the EventTarget.addEventListener and EventTarget.removeEventListener useCapture argument - related to issue #14188 #224
Conversation
|
@Diullei, It will cover your contributions to all Microsoft-managed open source projects. |
|
@Diullei, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request. |
|
This |
|
The appveyor error means when replacing the default lib.d.ts with your changed version in TypeScript repo, some TypeScript tests failed (in this case the The test is a simple check: https://github.com/Microsoft/TypeScript/blob/master/tests/cases/compiler/verifyDefaultLib_webworker.ts At a glance it looks like you defined the two new types only in the |
|
Ok, thanks for your help! I'll fix this. |
|
Hi @zhengbli the appveyor build has been fixed. |
baselines/dom.generated.d.ts
Outdated
| @@ -3701,9 +3701,9 @@ declare var Event: { | |||
| } | |||
|
|
|||
| interface EventTarget { | |||
| addEventListener(type: string, listener?: EventListenerOrEventListenerObject, useCapture?: boolean): void; | |||
| addEventListener(type: string, listener?: EventListenerOrEventListenerObject, useCapture?: boolean | AddEventListenerOptions): void; | |||
There was a problem hiding this comment.
According to the spec, the third argument should be named "options" instead of useCapture, can you also correct this?
baselines/dom.generated.d.ts
Outdated
| } | ||
|
|
||
| interface AddEventListenerOptions extends EventListenerOptions { | ||
| passive: boolean; |
There was a problem hiding this comment.
The fact that this type is a dictionary in the spec, meaning every property should be optional. Otherwise things like { once: true } would not be accepted. Can you update? The same with EventListenerOptions.capture
|
@zhengbli done! |
|
Thanks @Diullei ! |
…ontainer is document.body BUG: when $container is document.body, chrome version higher than 56 will display a warning, since the default value of passive is true on document. see https://www.chromestatus.com/features/5093566007214080 FIX: add {passive: false} to add/removeEventListener, since Typescript’s default signature of `option` in addEventListener is boolean, and in removeEventListener is bool | targetEventOption, add static cast to any to avoid this bug. microsoft/TypeScript#9548 microsoft/TypeScript-DOM-lib-generator#224 TODO: may need remove this cast in later Typescript version
According to https://dom.spec.whatwg.org/#interface-eventtarget
EventTarget.addEventListeneruseCaptureargument must be (AddEventListenerOptions or boolean)EventTarget.removeEventListeneruseCaptureargument must be (EventListenerOptions or boolean)Fix issue microsoft/TypeScript#14188