Re: [whatwg/dom] Expose an `aborted` promise on AbortSignal? (#946)

Maybe `signal.race` is enough. I think I was focusing too much on having the 'abort' actions in the same closure as the actions, so stuff didn't need to be assigned out of the closure, but maybe that's overcomplicating things? Might be worth testing with more real-world examples.

One of the nice things about `signal.race(…).finally(…)` is the abort steps can be async.

I don't think `signal.aborted` is enough, as it makes it too easy to create the leak issue that we're trying to solve https://github.com/whatwg/dom/issues/946#issuecomment-773392318.


Is this your implementation of `signal.race`?

```js
AbortSignal.prototype.race = function(promise) {
  if (this.aborted) throw new DOMException('', 'AbortError');
  let onAbort;

  return Promise.race([
    new Promise((_, reject) => {
      onAbort = () => reject(new DOMException('', 'AbortError'));
      this.addEventListener('abort', onAbort);
    }),
    promise,
  ]).finally(() => this.removeEventListener('abort', onAbort));
}
```

There'd need to be some education around usage of all of these patterns, as:

```js
// Good (assuming './search-helpers.js' has no side-effects)
const response = await fetch(searchURL, { signal });
const { addSearchResults } = await signal.race(import('./search-helpers.js'));
await addSearchResults(response, { signal });

// Very bad
await signal.race((async () => {
  const response = await fetch(searchURL, { signal });
  const { addSearchResults } = await import('./search-helpers.js');
  await addSearchResults(response, { signal });
})());
```

…since the latter may still add search results after the operation appear successfully aborted.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/issues/946#issuecomment-845906896

Received on Friday, 21 May 2021 12:12:34 UTC