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

Draft B #2

Open
domenic opened this issue Feb 5, 2013 · 12 comments
Open

Draft B #2

domenic opened this issue Feb 5, 2013 · 12 comments

Comments

@domenic
Copy link
Member

domenic commented Feb 5, 2013

Promises/A+ Extension: Synchronous Inspection

This proposal extends the Promises/A+ specification to cover synchronous inspection of a promise's fulfillment value or rejection reason.

It is not expected that all Promises/A+ implementations will include this extension. If the features of this extension are desired, you should test for them:

if (typeof promise.inspect === "function") {
  // Use the `inspect` method, assuming it conforms to the contract below.
}

Motivation

TODO

Requirements: the inspect method

A promise implementing this specification must have an inspect method, which returns an object.

  1. When the promise is pending, the object returned by inspect
    1. must not have a property named fulfillmentValue.
    2. must not have a property named rejectionReason.
  2. When the promise is fulfilled, the object returned by inspect
    1. must have a property named fulfillmentValue, whose value is equal to the promise's fulfillment value.
    2. must not have a property named rejectionReason.
  3. When the promise is rejected, the object returned by inspect
    1. must not have a property named fulfillmentValue.
    2. must have a property named rejectionReason, whose value is equal to the promise's rejection reason.

Note

Since fulfillment values and rejection reasons can be any valid JavaScript value, including undefined, this specification hinges on the difference between a property being present and it being absent. That is, the proper way to synchronously test for a promise being fulfilled is not if (promise.inspect().fulfillmentValue) or even if (promise.inspect().fulfillmentValue !== undefined), but instead if ("fulfillmentValue" in promise.inspect()).

@domenic domenic mentioned this issue Feb 5, 2013
@domenic
Copy link
Member Author

domenic commented Feb 5, 2013

Note that this doesn't attempt to make synchronous retrieval very ergonomic, on the theory espoused by @unscriptable over in promises-aplus/promises-spec#61 (comment) that people should think twice and just use then mostly.

Implementations should feel free to provide sugar, of course:

Q.isPending = function (promise) {
  var inspected = promise.inspect();
  return !("fulfillmentValue" in inspected) && !("rejectionReason" in inspected);
};

Q.isFulfilled = function (promise) {
  return "fulfillmentValue" in promise.inspect();
};

Q.isRejected = function (promise) {
  return "rejectionReason" in promise.inspect();
};

@briancavalier
Copy link
Member

This seems very similar to the mostResolved/nearer algorithm (the code examples are very similar too!) except that it's returning a non-promise object rather than a (potentially decorated) promise. At first glance, it seems like mostResolved would cover the same use cases, plus maybe others where returning a nearer promise might be useful, although I admit that I have no idea what those use cases are right now :)

In a way, yeah, they are separate issues, but they do seem quite closely related, especially since it seems possible to solve both with mostResolved. I wonder if that's a good or bad thing ... hmmmm.

@domenic
Copy link
Member Author

domenic commented Feb 5, 2013

The problem with mostResolved is that it returns a promise with settable properties. I think this gives the false impression that you can set those properties to influence the state of the promise. And, more importantly, it allows promise consumers to spoof each other by setting those properties.

@domenic
Copy link
Member Author

domenic commented Feb 5, 2013

Also, the mostResolved algorithm necessitates making your promises non-frozen (indeed, non-extensions-prevented!), since once the most-resolved promise becomes fulfilled, the value property must become present with the fulfillment value.

@briancavalier
Copy link
Member

Good points about settable properties. I wonder if it would be a problem in practice, but it's definitely a concern.

I imagine that it's possible for mostResolved to provide some defense by returning a new promise each time rather than returning the same one, but maybe that creates problems for people using === in weird ways. That adds some overhead, but presumably inspect will have to return a new object each time to prevent similar spoofing problems if the inspection object is shared or cached. It does seem somehow less hazardous and probably less of a perf hit in the inspect case (Although, if people are using mostResolved in tight loops, they're probably doing something wrong anyway).

I'm not convinced mostResolved requires non-frozen promises. I believe the proof of concept I did (cujojs/when@f929a35) would have still been doable with frozen promises (even though we ditched Object.freeze a while back due to the v8 perf problem).

@domenic
Copy link
Member Author

domenic commented Feb 5, 2013

@briancavalier You have started describing something that is not at all mostResolved, i.e. it is not the "most resolved" promise underlying the promise it's called on. Instead what you describe returns some kind of derived promise with extra properties that are a "snapshot" of the most-resolved promise's state at the time mostResolved is called. The original algorithm proposed for mostResolved returns the most-resolved promise (often the same as the original promise it's called on); thus, if the state of the most-resolved promise changes, the properties must appear.

This is exactly the road that led me to inspect. I realized that mostResolved mixes the concerns of finding the most-resolved promise, and the concerns of synchronously inspecting a promise's state. The former does a poor job of the latter. The only reason it was proposed is that Q has historically decided it's useful not only to know a promise's fulfillment value or rejection reason, but also any promise the given promise is waiting on---and, since a promise may be waiting on a fulfilled promise, the only foolproof way to synchronously get a promise's rejection reason in Q is promise.valueOf().exception (analogous to promise.mostResolved().rejectionReason).

@domenic
Copy link
Member Author

domenic commented Feb 5, 2013

For example, given mostResolved as described originally, you could write isNotWaitingOnAnyone as

function isNotWaitingOnAnyone(promise) {
 return promise.mostResolved() === promise;
}

This does not work with your revised definition, showing the difference.

@briancavalier
Copy link
Member

Ok, I see what you're saying.

In this draft, is the object returned by inspect a snapshot of the current status? e.g. if it doesn't have a fulfillmentValue property, you're guaranteed one will never materialize onto it. It sounds like that's the case from the "Requirements" section of the draft, but want to make sure I'm understanding correctly.

That said, I still feel like I haven't seen any compelling concrete use cases for sync inspection, so I'm still wondering why we should spec anything at all. I'm certainly open to it if people have a real need for it that can't be solved in a reasonable/performant way using then().

@domenic
Copy link
Member Author

domenic commented Feb 5, 2013

In this draft, is the object returned by inspect a snapshot of the current status?

Yeah, that's the intent.

That said, I still feel like I haven't seen any compelling concrete use cases for sync inspection, so I'm still wondering why we should spec anything at all.

Did you see #3?

@novemberborn
Copy link

I like!

Does this interfere with debugging tools that may invoke inspect() to prepare console output? E.g. Node's util.inspect: http://nodejs.org/api/util.html#util_util_inspect_object_showhidden_depth_colors.

@domenic
Copy link
Member Author

domenic commented Feb 5, 2013

Oh, interesting! If it does, another name would be fine. I didn't know Node did that. On the other hand, maybe it would be a nice feature??

@ForbesLindesay ForbesLindesay mentioned this issue Feb 6, 2013
@ForbesLindesay
Copy link
Member

Yes node.js/util.inspect:

Objects also may define their own inspect(depth) function which util.inspect() will invoke and use the result of when inspecting the object.

It expects the result to be a string though. I do think that providing an implementation of util.inspect would be a nice bonus feature for promises, but perhaps it shouldn't be the name of this method.

Other than that I'm fairly happy with this proposal (since D was rejected).

How about inspectState. It's longer, and at least as clear?

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

4 participants