-
Notifications
You must be signed in to change notification settings - Fork 10.3k
[Editor] Remove event listeners with AbortSignal.any()
#18586
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
[Editor] Remove event listeners with AbortSignal.any()
#18586
Conversation
There's a fair number of event listeners in the editor-code that we're currently removing "manually", by keeping references to their event handler functions. This was necessary since we have a "global" `AbortController` that applies to all event listeners used in the editor-code, however it's now possible to combine multiple `AbortSignal`s; please see https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/any_static Since this functionality is [fairly new](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/any_static#browser_compatibility) the viewer will check that `AbortSignal.any()` is available before enabling the editing-functionality. (It should hopefully be fairly straightforward, famous last words, for users to implement a polyfill to allow editing in older browsers.) Finally, this patch also adds checks and test-only asserts to ensure that we don't add duplicate event listeners in various editor-code.
/botio integrationtest |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/9b42919522445f0/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/0fdc0141f7e7afd/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/0fdc0141f7e7afd/output.txt Total script time: 8.38 mins
|
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/9b42919522445f0/output.txt Total script time: 17.83 mins
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/a6cf1e601da1f2f/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/d8804eb70eb9728/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/a6cf1e601da1f2f/output.txt Total script time: 8.35 mins
|
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/d8804eb70eb9728/output.txt Total script time: 18.01 mins
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/b0de5aceed2f82e/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/625695c9a0049b1/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/b0de5aceed2f82e/output.txt Total script time: 8.37 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/625695c9a0049b1/output.txt Total script time: 17.87 mins
|
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.
LGTM. Thank you for doing that.
In reading the docs for AbortSignal::any
I noticed AbortSignal::Timeout
, is it something we could use somewhere ?
I'm sure that could be helpful, but off the top of my head I don't know of any cases that would benefit from it. |
There's a fair number of event listeners in the editor-code that we're currently removing "manually", by keeping references to their event handler functions.
This was necessary since we have a "global"
AbortController
that applies to all event listeners used in the editor-code, however it's now possible to combine multipleAbortSignal
s; please see https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/any_staticSince this functionality is fairly new the viewer will check that
AbortSignal.any()
is available before enabling the editing-functionality.(It should hopefully be fairly straightforward, famous last words, for users to implement a polyfill to allow editing in older browsers.)
Finally, this patch also adds checks and test-only asserts to ensure that we don't add duplicate event listeners in various editor-code.