Support instances of EventTarget as a signal
#1001
Conversation
Node has added its own AbortController in 15.0.0. The AbortController signal inherits from EventTarget. This changes the check to also accept signals of type EventTarget. https://nodejs.org/docs/latest-v15.x/api/globals.html#globals_class_abortsignal
Cool - have looked forward for this! More spec'ed browser apis!
Cool - Now we also got a spec'ed EventTarget also :) |
EventTarget as a signal
EventTarget as a signalEventTarget as a signal
| object[NAME] === 'AbortSignal' | ||
| typeof object === 'object' && ( | ||
| object[NAME] === 'AbortSignal' || | ||
| object[NAME] === 'EventTarget' |
There was a problem hiding this comment.
I don't understand this. On Node.js v15.3.0, I see
> new AbortController().signal[Symbol.toStringTag]
'AbortSignal'So it seems like a bug to accept non-AbortSignal EventTarget objects.
There was a problem hiding this comment.
Just notice this. yea, it's weird that we check for the name EventTarget. a proper test was not even created... still using the polyfill for this...
node v12 goes EOL soon, so we are thinking of dropping support for node below v14.17 so that we can start using AbortController/AbortSignal provided by NodeJS so that we can pass this signal directly to node's own http.request.
Will create an issue for this. (targeted for v4)
Node has added its own AbortController in 15.0.0. The AbortController signal
inherits from EventTarget. This changes the check to also accept signals of
type EventTarget.
https://nodejs.org/docs/latest-v15.x/api/globals.html#globals_class_abortsignal
What is the purpose of this pull request?
What changes did you make? (provide an overview)
Modified the
isAbortSignalcheck to acceptEventTargetas well.Which issue (if any) does this pull request address?
Incompatibility with the new node
AbortController