search

WICG / idle-detection

A proposal for an idle detection and notification API for the web
Other
68 stars 22 forks source link

Replace stop() with an AbortSignal #19

Closed reillyeon closed 4 years ago

reillyeon commented 4 years ago

It is not currently possible to stop the IdleDetector. An AbortSignal should be added. An option question is whether this should be added as a constructor parameter or a start() parameter.

domenic commented 4 years ago

IMO it depends on whether you want to abort all future uses of the IdleDetector, or only the current start/stop pair.

tomayac commented 4 years ago

What happened to the IdleDetector.stop() method? The commit messages do not provide meaningful details, when and why was it removed? It seems to be still in the implementation and the WebIDL ([link](https://github.com/samuelgoto/idle-detection#user-content-webidl:~:text=void%20stop()%3B)).

reillyeon commented 4 years ago

Thank you for pointing out my error. I've changed the title to reflect what we should actually do.

It feels weird to have one set of options on the constructor and one set for start() so maybe all the options should move to start().

If I were making this change to the Generic Sensor API and wanted to maintain backwards compatibility I would use the same options dictionary for both and have options passed to start() override the constructor. In that case, to answer @domenic's point, passing a new AbortSignal to start() would allow the IdleDetector to be reused.

domenic commented 4 years ago

It feels weird to have one set of options on the constructor and one set for start()

I don't necessarily agree. The constructor allows you to configure options that are independent of any individual idle observation period. So for example maybe whenever you want to observe, you want the same timeout. Whereas the start() method would allow options that are only for a particular observation period, e.g. an AbortSignal.

Stated another way, I don't think there's any value in reusing an IdleDetector object if that object has no configurable properties.

However, I can appreciate that drawing this distinction this might be a bit web-developer-unfriendly, even if it makes sense in theory. If that's the case, then it's probably worth revisiting the decision to have separate construction and start phases, which is basically #10.