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

Use AbortController/*Signal instead of cancelWatch(optional long id); #147

Closed
kenchris opened this issue Jul 5, 2018 · 10 comments
Closed
Labels
tag-tracker Group bringing to attention of the TAG, or tracked by the TAG but not needing response.

Comments

@kenchris
Copy link
Contributor

kenchris commented Jul 5, 2018

https://developer.mozilla.org/en-US/docs/Web/API/AbortController

That is a much more modern way of doing this, but one that didn't exist when cancelWatch was added to the spec

@kenchris
Copy link
Contributor Author

kenchris commented Jul 5, 2018

The signal should be added to the NFCPushOptions and NFCWatchOptions

Example on how to add to the spec: https://wicg.github.io/geolocation-sensor/#dom-readoptions-signal

@kenchris
Copy link
Contributor Author

kenchris commented Sep 27, 2018

This definitely makes sense for push (which I think we should just rename to write to be consistent with Web Bluetooth etc).

This even allows us to get rid of the timeout:

const controller = new AbortController();
const signal = controller.signal;

const message = {
  records: [{ recordType: "text", data: 'Hello World' }]
};

navigator.nfc.write(message, {signal});

// timeout after 3 seconds
setTimeout(() => controller.abort(), 3000);

@zolkis
Copy link
Contributor

zolkis commented Sep 27, 2018

I agree with getting rid of a separate cancel(id), but I don't like this proposal of using AbortController. There is no network operations to be aborted with a watch, this is about canceling a local operation or subscription. Therefore IMHO it would be better to apply a simpler pattern. Setting up a watch could return an object that has a cancel() method. Looks like Observables have the best fit here, just like with #152.

@kenchris
Copy link
Contributor Author

kenchris commented Sep 27, 2018

AbortController is for cancelling any promise, there is no networking tie in. It is just that it is used only with fetch so far: https://dom.spec.whatwg.org/#interface-abortcontroller

It basically came out of the whole cancellable promises discussion.

Don't expect anything like observables on the web platform for a long time.

AbortSignal is already being used in the GeolocationSensor: https://w3c.github.io/geolocation-sensor/#dictdef-readoptions

@zolkis
Copy link
Contributor

zolkis commented Sep 27, 2018

push (which I think we should just rename to write

But push was an NFC-specific term.

const controller = new AbortController();

I'd prefer watch() return the AbortController.

var controller = navigator.nfc.write(message, options);
setTimeout(() => controller.abort(), 3000);

In some cases we cannot guarantee cancellation. What happens then? It is not specified for AbortController or AbortSignal.

@kenchris
Copy link
Contributor Author

But push was an NFC-specific term.

Well, not really, I am pretty sure that I came up with that due to the Android UX for peer to peer

In some cases we cannot guarantee cancellation.

That would be the same with fetch right. It might have completed

@kenchris
Copy link
Contributor Author

kenchris commented Sep 27, 2018

Works as expected:

If fetch completes, abort does nothing. Reusing same signal again, and the fetch is aborted up front.

image

I think returning the AbortController would be a bit weird and not how it is intended to be used. You can reuse the same signal to cancel multiple things

@kenchris
Copy link
Contributor Author

As always @domenic 's comments would be useful

@domenic
Copy link
Contributor

domenic commented Sep 29, 2018

Strong +1 to using AbortController. As @kenchris has said, it is not related to network operations, and it's just how you cancel async operations on the platform.

I can understand how it can seem complex in isolation. But as with promises, streams, etc., reusing the same primitive everywhere has multiplicative effects throughout the platform. In particular, there's a common pattern of using a single AbortSignal for a bunch of ongoing operations, and then aborting them (with the corresponding AbortController) when e.g. the user presses cancel, or a single-page-app navigation occurs, or similar. So the minor extra complexity for an individual API leads to a large reduction in complexity when used with multiple APIs together.

Right now AbortController is only used in Fetch and Web Locks to my knowledge. But we're soon going to be using it in Streams, a promise-returning version of setTimeout (whatwg/html#617), and probably other upcoming APIs like writable files. Being able to cancel all these things together using a single tool is the dream, just like today you can use a single tool (promises) for async operations.

In some cases we cannot guarantee cancellation. What happens then?

A call to abort() is a request to abort. How you react to it depends on your spec. We could have given it a verbose name like requestAbort() but we went for brevity.

@zolkis
Copy link
Contributor

zolkis commented Sep 29, 2018

OK, we could do that. I see there is a polyfill in Node.js too. We could even use that in the Web of Things as well as a generic pattern.

Being able to cancel all these things together using a single tool is the dream

Right. A bit unrelated, but I am sad to see Observables go down, as it was a powerful single tool to handle events, streams with possibility to filtering at source or on consumption. Whatever. It's more important to have a workable consensus than to not have the best one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag-tracker Group bringing to attention of the TAG, or tracked by the TAG but not needing response.
Projects
None yet
Development

No branches or pull requests

4 participants