-
Notifications
You must be signed in to change notification settings - Fork 47
Inconsistent source signal with AbortSignal? #117
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
Comments
Priority and abort dependency is separate: the signal created by For abort, Priority dependency mirrors what we do in the DOM spec, with some slight differences. As mentioned, there's only a single priority source for a dependent As an example: const controller = new TaskController();
const someTaskSignal = controller.signal;
// This "drops" the abort behavior of `someTaskSignal`, but is dependent
// on its priority. The signal's weak set of source signals (DOM) will be
// empty, but its "source signal" (this spec) will be set to `someTaskSignal`.
// Also, `someTaskSignal` will have one dependent signal in its
// "dependent signals" set (this spec) and 0 for abort.
const signal1 = TaskSignal.any([], {priority: someTaskSignal});
// Same as above, but now `someTaskSignal` has a second dependent
// signal for priority.
const signal2 = TaskSignal.any([], {priority: someTaskSignal}); It's a bit confusing given the reuse of naming in this spec, but the AbortSignal state is private (not exported), and we don't reach into that in this spec. There's an open issue about trying to avoid the duplication, which I'm trying to figure out as I'm working to upstream this, but might do it as a follow-up PR. TBD. |
Thanks Scott, it makes sense now. I am happy to close this and #118, or do you want to leave them open to address the confusion? |
TaskSignal has weak reference for a source signal, where as the base class AbortSignal has a weak set for similar concepts.
Shouldn't TaskSignal also use that?
The text was updated successfully, but these errors were encountered: