Skip to content
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

Clarify appropriate and inappropriate use cases for AbortController #138

Closed
reillyeon opened this issue Sep 20, 2019 · 6 comments
Closed
Assignees

Comments

@reillyeon
Copy link
Contributor

The design principles recommend using an AbortController for canceling asynchronous operations. This guidance has filtered down to other specifications such as w3c/web-nfc#225 and w3c/wake-lock#183. In the case of Wake Lock this resulted in something which was felt to be a very awkward design and during the Devices and Sensors F2F meeting at TPAC 2019 we resolved on w3c/wake-lock#214 to remove the AbortSignal in favor of the same ID pattern used by APIs such as setInterval()/clearInterval().

The decision within that group was that an AbortSignal is appropriate for a Promise-returning API which requires cancellation of an operation before the Promise is resolved. Non-Promise-returning APIs, or APIs which resolve the Promise after the operation has been initiated, should not take an AbortSignal.

As an example, the NFCReader interface's scan() method does not return a Promise and may generate multiple "reading" events or an "error" event. It should not take an AbortSignal and instead have a stopScan() or similar method. In contrast the NFCWriter interface's push() method performs a single write operation, after which the returned Promise is resolved. This method should take an AbortSignal.

We would like feedback from the TAG on this decision and clarification in the design principles.

@annevk
Copy link
Member

annevk commented Sep 21, 2019

cc @jakearchibald @domenic

@reillyeon
Copy link
Contributor Author

On w3c/wake-lock#226 @domenic says,

I strongly urge the working group to avoid numeric IDs for cancelation. setTimeout's use of those is a very bad pattern from the old days of the web, and not to be emulated. Objects should be preferred to numbers.

Can you elaborate on the reason why this is a bad pattern to emulate? This information should also probably end up in the design principles.

@domenic
Copy link
Member

domenic commented Sep 23, 2019

Sure. Integer handles are not strongly typed. This makes for a bad experience in various ways for developers, and also opens your system up to inadvertent forgery of the capabilities in question: e.g. for (let i = 0; i < 10000; ++i) { clearTimeout(i); }. Objects, with identity, better encapsulate the capability to do something like release a lock, instead of passing numeric handles between standalone functions.

@jakearchibald
Copy link
Contributor

If the API returns something representing the ongoing operation, an abort method on that object seems reasonable, unless it's desirable to separate ownership of the ability to abort and the ability to consume. As @domenic said, integers are bad because the ownership becomes a grey area.

The streams API is a good example of AbortController. A readable stream has a cancel method, since the stream object represents the ongoing operation. Whereas the readable stream's pipeTo method takes a signal, since there's no object representing the ongoing operation.

@domenic
Copy link
Member

domenic commented Sep 23, 2019

Although, I'll note that we're working on allowing you to pass an AbortSignal to the stream contructor as an alternate API, and it's unclear if we would have included the cancel method if we were designing the Streams API with those primitives already in the platform.

@plinss
Copy link
Member

plinss commented Mar 4, 2020

@ylafon and I looked at this during our Wellington F2F. We feel that the use of AbortController is generally appropriate, and the original issue that brought this to the surface was more of an inappropriate use of Promises. We're filing an issue on the Promise guide to add clarity there. It's not clear there's enough API experience to expand on the existing advice around AbortController.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants