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

Clarify when onFulfilled/onRejected must *not* be called #129

Closed
briancavalier opened this issue Jun 16, 2013 · 6 comments
Closed

Clarify when onFulfilled/onRejected must *not* be called #129

briancavalier opened this issue Jun 16, 2013 · 6 comments

Comments

@briancavalier
Copy link
Member

See promises-aplus/promises-tests#36. Copied from my comment.


It seems there is nothing that explicitly prevents an implementation from calling onRejected between the instant a promise is fulfilled and the instant onFulfilled is called, which then makes section 3.2.2 into a contradiction: onFulfilled must be called (3.2.2.1), but also must not be (3.2.2.3)!

It also seems that there's nothing that explicitly prevents an implementation from calling either onFulfilled or onRejected while a promise is pending.

These things are certainly implied, and if an implementation chose to do them, it seems that would make it impossible (afaict) to satisfy the other requirements of the spec (specifically 3.2.2, 3.2.3 and 3.2.5).

Perhaps we need to add a few explicit statements to clarify? Something like:

  • onFulfilled and onRejected must not be called while promise is pending.
  • onFulfilled must not be called after promise is rejected.
  • onRejected must not be called after promise is fulfilled.

Another alternative might be to add "only" to existing requirements:

  • 3.2.2.1 it must be called after, and only after, promise is fulfilled, with promise’s fulfillment value as its first argument.
  • 3.2.3.1 it must be called after, and only after, promise is rejected, with promise’s rejection reason as its first argument.

What do others think? Do we have some holes here we need to patch, or are the current implications strong/obvious enough?

@lsmith
Copy link
Contributor

lsmith commented Jun 17, 2013

Interesting. I think a language change is reasonable. I prefer your first suggestion of the explicit bullets.

@domenic
Copy link
Member

domenic commented Jun 17, 2013

I'm curious whether anyone can make an implementation that follows the existing spec/passes the existing tests, but still disobeys these bullets?

@briancavalier
Copy link
Member Author

@domenic I actually don't believe it's possible. The problem, though, is that the current spec simply may not be clear enough, and may cause confusion, which seems to be what happened in promises-aplus/promises-tests#36

@briancavalier
Copy link
Member Author

Hmmm, what about a promise that remains pending forever? Seems the spec allows an implementation to call either onFulfilled or onRejected (but not both) once, whenever it feels like it in that case.

@domenic
Copy link
Member

domenic commented Jun 17, 2013

Hmmm, what about a promise that remains pending forever?

Nice catch, agreed it's a hole. Kind of a symptom of our requirements-based (as opposed to algorithm-based) spec, but what can you do.

briancavalier added a commit to briancavalier/promises-spec that referenced this issue Jun 18, 2013
…t not be called before promise is fulfilled or rejected.
@briancavalier
Copy link
Member Author

Ok, I took a shot at it in #130. I think it's possible to specify what we want by simply changing two bullets, rather than adding more. Let me know if you think this captures what we want and also doesn't open any new holes :)

briancavalier added a commit to briancavalier/promises-spec that referenced this issue Jun 18, 2013
…t not be called before promise is fulfilled or rejected.
briancavalier added a commit that referenced this issue Jun 20, 2013
Fixes #129 by prohibiting `onFulfilled` and `onRejected` from being called before the promise is respectively fulfilled or rejected. An example of behavior the current spec allows would be for an implementation to call `onFulfilled` or `onRejected` once, even on a promise that remains pending forever.
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

Successfully merging a pull request may close this issue.

3 participants