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

Consider Symbol.cancelSignal as a cross-platform protocol to support cancellation. #16

Closed
rbuckton opened this issue Sep 11, 2017 · 24 comments

Comments

@rbuckton
Copy link
Collaborator

rbuckton commented Sep 11, 2017

At the last TC39 meeting there were two main points of contention around the previous cancellation proposal:

  1. The Cancellation API, as proposed, is not compatible with the cancellation mechanism specified by WHATWG
  2. There is no syntactic abstraction over cancellation

One possible approach is to leverage a mechanism similar to iteration, by establishing a well-defined protocol that indicates support for the minimum capabilities of cancellation. As such, I propose the following minimal cancellation protocol:

interface Signal {
  signaled: boolean;
  advise(cb: Function): void;
  unadvise(cb: Function): void;
}

interface Cancelable {
  [Symbol.cancelSignal](): Signal;
}

The Signal and Cancelable interfaces above do not represent runtime values, but rather the shape of the protocol. This is similar to the relationship between an Iterable object (e.g. an object that has a [Symbol.iterator] method), and an Iterator object (e.g. an object that has, at the very least, a next() method).

Defining a standard protocol allows WHATWG to continue along its current course of innovating new types of controllers and signals, while at the same time allowing TC39 to establish a core set of primitives. This gives WHATWG the ability to special-case the handling of advanced features, such as progress notifications, while at the same time maintaining a minimal protocol for basic cancellation needs.

Defining a standard protocol also allows for the possibility of a future syntax that could leverage this well defined protocol.

edit: Updated the API above to use less divisive terminology, per #16 (comment)
edit: Removed Source from the api as it does not directly contribute to the discussion

@rbuckton
Copy link
Collaborator Author

// CC: @bterlson, @domenic, @wycats

@domenic
Copy link
Member

domenic commented Sep 12, 2017

As discussed, Chrome is not interested in supporting two separate mechanisms, one from TC39 and one for the web. A protocol for adapting between two incompatible APIs is explicitly not what we agreed upon; we need public API compatibility. I thought we were fairly clear on this.

@rbuckton
Copy link
Collaborator Author

rbuckton commented Sep 12, 2017

@domenic I'm considering additional avenues, and this is one of them. I've removed the shim example above as it may be controversial and might derail this discussion. While I appreciate your position about API compatibility, I would appreciate a more thorough discussion about the approach:

  • What are the benefits and limitations of this approach?
  • Is there a better shape for the protocol?
  • Does this align with existing principals of ECMAScript?

In the end we may not use this approach, but TC39's advancement of Cancellation to Stage 1 was under the condition that we investigate the problem domain. I would appreciate an open mind.

@RangerMauve
Copy link

What does TC39 have against the AbortController API? I'm assuming that that is what @domenic is referring to.

It appears to get the same job done as this proposal, but with a more DOM-y naming convention for the interfaces.

@rbuckton
Copy link
Collaborator Author

@RangerMauve, the issue is not with naming. We are exploring alternatives to find a consensus API that does not necessarily depend on ECMAScript natively adopting DOM events, but still meets the needs of DOM. It may be that TC39 will need to consider DOM events (or possibly a paired-down implementation) first, but its worthwhile to consider other options.

A native event system is a whole other complex topic I'd rather not discuss in this topic, to keep the topic focused.

@rbuckton
Copy link
Collaborator Author

rbuckton commented Sep 12, 2017

I suppose even the terminology above is divisive. As such, perhaps we should consider a more abstract definition:

interface Source {
  signal: Signal;
  set(): void;
}
interface Signal {
  signaled: boolean;
  advise(cb: Function): void;
  unadvise(cb: Function): void;
}

@RangerMauve
Copy link

Maybe someone from the fetch issue could weigh in on this?

It seems that the issue is that AbortController is dealing with an EventTarget for registering callbacks instead of functions that take callbacks.

I really doubt that TC39 will add DOM events to the spec, so there's really no way of adding the same interface to the language.

@domenic Saying that Chrome won't want to implement two types of cancellation, and the DOM answer to cancellation not being viable for JS really points to this effort being useless unless either the WHATWG decouple their spec from EventTarget, or Chrome starts being OK with the two ways to do the same thing (one of them only being used in the DOM)

@rbuckton
Copy link
Collaborator Author

My main concern about both DOM EventTarget and NodeJS EventEmitter is that there is no nominal definition of a specific event in either system. In both systems, events are de rigueur: While I can know that document.addEventListener("load", cb) will invoke cb when the document loads, I can't be certain that foreignObject.addEventListener("load", cb) will ever actually be raised. I have no contract guarantee that foreginObject supports the "load content and raise event" protocol. This can be somewhat mitigated in the DOM as it has a standard Event API, but things are worse with EventEmitter because event arguments are arbitrary.

On the other hand, symbols provide a nominal identifier for an established protocol. Consider Symbol.iterator: If an object has a Symbol.iterator method, then the runtime will treat the object as iterable. Here, Symbol.iterator indicates the object supports the iteration protocol.

Taking that one step further: If we had, say, a Symbol.cancelSignal that indicates an object supports the "cancellation signal protocol", then an object with a Symbol.cancelSignal method is expected to return a Signal primitive that the runtime understands. Symbol.cancelSignal is nominal, its presence on the object indicates it supports the protocol, while its lack indicates it does not support the protocol. This would then allow both native and host APIs a way to guard against invalid arguments and opens up the possibility of future syntactic sugar around cancellation that some on TC39 would prefer.

@rbuckton
Copy link
Collaborator Author

rbuckton commented Sep 12, 2017

I'm not trying to propose "two ways to do the same thing", but rather "one way to do cancellation that is easily built upon in any environment". @domenic has already indicated that DOM wants to support other kinds of Signals and other kinds of Controllers. In my opinion, this design meshes well with that approach rather than takes away from it.

Just as every iterator need not be an ArrayIterator to be used in for..of, neither does every cancellation signal need to be AbortSignal or CancellationToken. In this way implementers and the development community can augment and extend built in functionality and remain interoperable.

@rbuckton
Copy link
Collaborator Author

I've updated the issue description to use the terminology from #16 (comment)

@rbuckton rbuckton changed the title Consider Symbol.cancelToken as a cross-platform protocol to support cancellation. Consider Symbol.cancelSignal as a cross-platform protocol to support cancellation. Sep 12, 2017
@RangerMauve
Copy link

So what's the main argument for using a new Signal type instead of the existing Observable?

@rbuckton
Copy link
Collaborator Author

There isn't a specific one. We previously opted to not tie cancellation to Observable as there are still concerns in that proposal that are blocking its advancement. The initial api design I proposed in this issue used subscribe(cb: Function): { unsubscribe(): void; } which is actually fairly consistent with observable's API with respect to subscriptions.

@jakearchibald
Copy link

jakearchibald commented Sep 13, 2017

In terms of this proposal vs abortable fetch I defer to @domenic.

Abortable fetch is a long-awaited feature that's already seen years of delay. We have two implementations (Edge, Firefox) nearing completion, and I'm not particularly interested in throwing that away and subjecting developers to further delay. Especially as that delay may, once again, leave us no closer to shipping abortable fetch.

@ikokostya
Copy link

ikokostya commented Sep 13, 2017

Looks like AbortController doesn't support cancellation graph (which is very useful in some scenarios #10). Is it right?

@jakearchibald
Copy link

Correct. It may be added later, but in the meantime you can use events:

const controller = new AbortController();
otherSignal.addEventListener('abort', () => controller.abort());

@bterlson
Copy link
Member

@jakearchibald what is the delay you refer to? I believe the idea here is to find a suitable primitive for inclusion in ECMAScript that is compatible with existing functionality like AC/AS.

@jakearchibald
Copy link

@bterlson we were originally blocked on cancellable promises.

@benjamingr
Copy link

And then on cancel tokens after that. The important thing here is the capability to abort a fetch and not the particular sugar. People can't both abort a request before the headers arrived and read a chunk from the response right now and that's a pain for years.

Can we please just stick with AbortController? Fetch is meant to be an API for exposing capabilities.

@rbuckton
Copy link
Collaborator Author

@jakearchibald, the reason I filed this issue is to investigate the feasibility of a cancellation mechanism that doesn't block or delay AbortController. Defining a protocol for cancellation, just as we have a protocol for iteration, allows it to be added to existing host objects just as iteration was added to NodeList, etc.

@zenparsing
Copy link
Member

zenparsing commented Oct 4, 2017

I agree with @rbuckton that alternatives should be explored. Sorry, but I'm just going to say what I think here: it would be insane to spec addEventListener in ECMA262.

@zenparsing
Copy link
Member

zenparsing commented Oct 4, 2017

To expand a bit, adding DOM-compatible addEventListener as a language primitive does not make a whole lot of sense. Particularly so, since we've already gone through the exercise of designing a well-tested compositional push-notification primitive for JS (i.e. Observable)

@benjamingr
Copy link

What does TC39 have against the AbortController API? I'm assuming that that is what @domenic is referring to.

I defer to both @jakearchibald and @domenic here but that was actually the one fetch meeting I was in.

When we decided on an AbortController like API in the meeting we explicitly said we don't care about the API specifics and that we just needed the capability, something like AbortController was the most straightforward way to add cancellation as a capability to fetch (which is again, a low level API that enables capabilities XHR doesn't have).

I don't think AbortController should be our model for a cancellation API for the language in general - although it's "as powerful" as other token-based APIs.

@Lucretiel
Copy link

@jakearchibald

 const controller = new AbortController();
 otherSignal.addEventListener('abort', () => controller.abort());

Correct me if I'm wrong, but this has the flaw that if otherSignal is already aborted, nothing will be propogated to signal, correct?

@rbuckton
Copy link
Collaborator Author

In an attempt to reframe this discussion, I am closing this issue and reopening it with a (hopefully) clearer set of goals and semantics in #22.

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

9 participants