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

Stopping progress propagation #2

Open
briancavalier opened this issue Dec 20, 2012 · 6 comments
Open

Stopping progress propagation #2

briancavalier opened this issue Dec 20, 2012 · 6 comments

Comments

@briancavalier
Copy link
Member

See #1

Progress handlers should be allowed to explicitly stop progress propagation. The trend in #1 seems to be toward allowing handlers to throw a duck-typed "stop propagation" error.

What should this error look like? One option is to follow builtin Error convention and have it be an Error whose .name == "<some specific name>", since we can't rely on instanceof across implementations.

Two related questions:

  1. Would it be better to return such an error rather than throw it?
  2. Would it be ok to allow returning such an error in addition to throwing it?

My instinct is that we should only support throwing the stop propagation error.

@domenic
Copy link
Member

domenic commented Dec 20, 2012

My instinct is that we should only support throwing the stop propagation error.

Mine as well. No strong feelings, but that seems like the right path.

@briancavalier
Copy link
Member Author

Yeah, it seems like if our options are only return or throw, implementations have to to "corrupt" one of those code paths with logic that checks for the stop propagation value. If we allow both, implementations have to execute that logic in both paths. I'd rather corrupt the exception path :)

It's also a decent analog to throwing StopIteration.

@domenic
Copy link
Member

domenic commented Dec 20, 2012

It's also a decent analog to throwing StopIteration.

The analogy is a bit strained since StopIteration is actually not an Error (which is weird). But yeah.

@briancavalier
Copy link
Member Author

Yeah, more that the approach of using throw is the same. And as @ForbesLindesay pointed out in #1, eventually the stop propagation value could be upgraded to something on which we can use instanceof, like StopIteration. So, yeah, it might be a better analogy in the future :)

Hmmm, that does make me think, though, that maybe an Error with a .name may not be best--i.e. since StopIteration isn't an Error. Another thought is that promises are duck-typed using the presence/type of the then property. Should we consider a similar approach here? For example: an object with a truthy stopProgressPropagation property?

To my eyes:

if(result && result.stopProgressPropagation)

is nicer than

if(result && result.name == 'StopProgressPropagation')

Thoughts?

@domenic
Copy link
Member

domenic commented Dec 20, 2012

Meh, I weakly prefer the name option. I wish StopIteration was specced somewhere. We can ask es-discuss I guess?

@ForbesLindesay
Copy link
Member

I prefer .name. I also think it should be a real error object, so I'd be inclined to have the check be:

if(result instanceof Error && result.name == 'StopProgressPropagation')

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

3 participants