Add the step to check subscriber's active state to next(value)/error(error)/complete() as a shortcut #129
Conversation
…error)/complete() as a shortcut
tetsuharuohzeki
force-pushed
the
add-subscriber-shortcut
branch
from
1fd6812
to
ad708d0
Compare
|
Thanks! Do you think we can make this simplification go a bit further by making https://wicg.github.io/observable/#subscriber-next-algorithm, https://wicg.github.io/observable/#subscriber-error-algorithm, and https://wicg.github.io/observable/#subscriber-complete-algorithm all non-null, and then not clearing them in https://wicg.github.io/observable/#close-a-subscription? |
spec.bs
Outdated
| @@ -210,6 +212,8 @@ The <dfn attribute for=Subscriber><code>signal</code></dfn> getter steps are to | |||
| <div algorithm> | |||
| The <dfn for=Subscriber method><code>error(|error|)</code></dfn> method steps are: | |||
|
|
|||
| 1. If [=this=]'s [=Subscriber/active=] is false, then return. | |||
There was a problem hiding this comment.
This is correct, but there's an additional step. If the Subscriber/active is false, we'll want to report the |error| with reportError, because it can't be handled and we don't want it to be completely swallowed or ignored. cc @domfarolino
There was a problem hiding this comment.
There was a problem hiding this comment.
|
I'm not quite sure how the build is failing, maybe @domfarolino needs to look at it, or maybe it just needs rebased... however, it seems that @tetsuharuohzeki you'll need to link your account to a W3C account to pass on of the checks above. |
|
@benlesh I think your commit touched the wrong method |
…s inactive." This reverts commit f1b0503.
|
@domfarolino oops, fixed. |
I resolved that a moment ago.
I would like to have a time to answer it to switch my brain to this spec :) |
I think you are proposing to do this in a separate issue/follow-up, but I think this should be done in this PR. It is a simple change that is very inline with the changes this PR makes. I view the changes I've requested as a way to "finish up" the work started here, which would be great to do here! |
Fix #114
Preview | Diff