-
Notifications
You must be signed in to change notification settings - Fork 165
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
Ratify 1.1 #84
Comments
Once #83 is resolved, it'll get my signoff. |
Right, as it currently stands, it has my signoff (I'm not a collaborator, so I can't tick my own box) |
Oh, if people could weigh in on #80, that'd be swell. |
Releasing 1.1 will definitely "revoke" many libraries' current compliant status (avow.js included). I'm actually ok with that, because I think it's a good forcing function for people to improve their implementations. However, it does raise the question of logistics: we may need to drop some suggestions to people (after a grace period) that they need to update their lib. |
I realize that this work is all on master. Can we contrive a branch called v1.1 that forks v1.0 and send a pull request from master to that branch so I can make editorial comments? In assimilation, we should note that As a matter of copy, all occurrences of “which” that can be changed to “that” and have the intended meaning should probably be changed to “that”. We are free and loose with “which” colloquially. |
I'm not sure that's necessary for the ways we use assimilation currently. (Happy to be proved wrong though.) If we specify assimilation generally, i.e. try to specify |
@domenic, yes and good point. |
Both @kriskowal and @briancavalier have indirectly raised the point that I should really get cracking on a test suite for 1.1, so that we can be sure we're covering everything as precisely as we've hoped. I'll do that tonight. |
Tests in progress, but there's a lot left to write for the assimilation procedure. https://github.com/promises-aplus/promises-tests/tree/spec-1.1 I think it'd be best if I put this off until the weekend. In the meantime, would be great for others to chime in :). |
Thanks @domenic. I'll have time to give the updated spec a thorough read tomorrow. |
I'm still uneasy about resolution procedure
I'd much prefer: If retrieving the property The reason for this is that I definitely view that as the correct behavior if |
I am not sure how such proxy would be practical. Nearly every duck-type test in the wild would throw. Expecting that with es6 everyone will start to wrap property checks in In case I am wrong, would an additional check with the |
@ForbesLindesay, I see your uneasiness and somewhat agree. The way I reconciled it is over in #88 (comment).
It's a tempting idea, but I don't think it's great, because it is observable by proxies (who could do weird things like return |
@domenic, what's the status of the test suite for 1.1? Can we dive in and help? |
@briancavalier It's like half-done. It's my top priority after HTML5DevConf (so after Tuesday). |
The progress is at https://github.com/promises-aplus/promises-tests/tree/spec-1.1 BTW. |
Great, thanks. |
Providing everyone else supports the current proposed behavior for throwing get accessors I'll accept it and remove my objection. Meaning it has my sign off. |
Ratified. |
(I don't know how well github does with email quoted reply format. If not
The wording you've got does fit the "there" method <
If onFulfilled is neither a function nor undefined, promise2 must be
Ah. Answers my previous worry. But perhaps this needs to be stated earlier.
The reason the primitive value check must come before the thenable check is
must fulfill p2 with true, even though true would pass the thenable check
Those take case of the macro-turn case. As long as we're mentioning these This will be worth a big discussion somewhere, but the more I think about
I don't see how this could ever be possible under the constraints of the
|
It looks like github did well enough. The only issue I spot in the above text is that the first text line of some of my replies inappropriately appear as if they are part of the previously quoted text. Knowing that, the above text is easy enough to read. |
I would suggest that you may want to summarize the text if it actually matters that people know this stuff. You only have to write it once, but I imagine there are at least 10 people who have to read this. People quoting entire e-mails is the bane of ES-Discuss, lets not make it a problem here too. |
Ok.
|
(I think you mean footnote 3) Legendary has this behavior when no callbacks are passed: promise1.then() === promise1; |
Gotcha. I hadn't considered the no-callbacks case. |
Couldn't such Far-Promises just not have a
No, the important invariant the event loop turn is trying to create is that ordering of code is at least moderately consistent. By which we mean, always asynchronous. This doesn't always mean a completely empty stack, as libraries can optimize by providing an internal representation of the stack. The stack has to empty between the
I agree, spelling it
Isn't that what the spec currently does |
A far reference is a promise and must have a then method. Otherwise, the promise patterns using the then method wouldn't work at all when encountering a far reference. These are both important invariants. That the ordering invariants are important does not contradict that the empty stack invariant is important. Boolean.prototype.then = function(){}; As I read the current spec, it will treat the above true as a thenable. It must treat it as a primitive value, fulfilling p2 with true. |
I am not too comfortable with allowing: promise1.then() === promise1; becouse promise1.then(x => x) !== promise1; Consistency is, in my opinion, much important then a minor and doubtful gain in performances. |
None of the stuff in this spec does anything with far references, it only handles the No, the empty stack was purposefully not required. It doesn't add anything on its own, it was just one way of trying to write that you can't call the function synchronously without saying "in the next tick". It was removed as a requirement because it turns out to prevent some very important optimizations that have no undesirable side effects. No, it won't treat the There are two issues with that argument.
|
@ForbesLindesay are you saying that usage of |
@ForbesLindesay You write: "I have no problem with such far-references being thenables but not Promises/A+ Promises." Well I do. I would like Q to conform to promises/A+ but be able to recognize if own far references as promises within the resolution procedure. I understand that nothing in the promises/A+ spec is about far references. I am not suggesting otherwise. I am simply saying that the promises/A+ spec should not be incompatible with far references. On the empty stack, please give an example of an optimization which is possible otherwise, and with "no undesirable side effects". An example would help me understand what you have in mind. On the order of checking and my true example, you are exactly right. Somehow I missed the "if x is an object or function" part at the beginning of point 2. Thanks. |
No, but there's equally nothing preventing you from doing something like: var oldThen = Promise.prototype.then;
Promise.prototype.then = function (cb, eb) {
var res = oldThen.apply(this, arguments);
res.parent = this; //this is allowed, and breaks your requirement anyway
return res;
}; The result would still be fully Promises/A+ compliant. Which statement of the spec is (in your view) incompatible with interoperability between promises and far references. The Q library hasn't had any issues meeting the spec thus far and I think @kriskowal (or @domenic) should jump in if they feel this might raise some issues. I don't think the spec actually causes any problems with regards to interop with As for the optimization, instead of calling next tick for each promise, you create an internal queue of promises that must be run in the next tick. You then empty this queue once the only thing in the stack is your own library, because at that point, the outside world can't tell you haven't gone to the next tick, other than the fact that if you do this forever you'll starve the IO. I don't want to go into tonnes of detail on this, but it's been discussed at some length in previous issues both here and in Q (where I think there was some talk of trying this out). |
@ForbesLindesay altering the prototype is clearly an abuse, but altering an instance could be considered fine for a user that believes that he owns that object. Maybe I am too paranoid.. |
The code I listed above could go inside a promise library and still be legal, that was the point I was attempting to make: amazing-promises.js: funciton Promise(fn) {
// magically create `resolve`, `reject`
fn(resolve, reject);
}
Promise.prototype.then = function (cb, eb) {
//magically use `cb` and `eb` apropriately and get the result
//which is promise like and call it `res`
res.parent = this; //this is allowed, and breaks your requirement anyway
return res;
}; Assuming you implemented the bits I've labelled as magic, the above would be a promises/A+ compatible library. |
@ForbesLindesay writes "Which statement of the spec is (in your view) incompatible with interoperability between promises and far references." I think it's only a problem of wording, not a deep issue. The statement I object to is "If onFulfilled is a function: If the wording of Promises/A+ were not changed in this regard, it wouldn't be a disaster. It would just push the awkward phrasing issue into the description of the Q library. Q would need to say that a far reference's fulfillment value is itself. The optimization you have in mind seems fine and is one I encourage. I don't see how it violates the empty stack constraint. The loop you're using to empty this queue is part of the implementation of the JS-enhanced-with-promises platform. No activation frames within user code are stacked up at the beginning of each iteration of your loop. Each iteration of that loop begins in an empty stack. |
@ForbesLindesay to prevent more repeating, I will stop here. Subject not particularly important anyway. |
OK, back from HTML5DevConf! Ready to dig into this. @erights:
We discussed this in #25, with the conclusion that this approach is not acceptable, as it will break too much promise code in the wild that uses
This is a tricky one. Suggestions welcome.
I tried to get this in, on your suggestion, in #70. But there were concerns about clarity and about the user-space trampoline argument discussed above; see especially this explanation from @briancavalier. In particular, capturing the subtlety of
is tricky. What would the phrasing be? "When the function execution stack is empty, except that it may also contain code from the promise implementation"? Suggestions definitely welcome.
Will do! |
I'm all for trying to find some language for this. It seems very tricky without re-defining "call stack", or somehow specifically classifying promise implementation code as part of the "platform", as @erights suggested. Or maybe something along the lines of what @domenic is suggesting, but how to phrase it nicely is stymieing me .. maybe something about only promise implementation code being allowed below onFulfilled/onRejected on the call stack?
+1
I understand the issue with remotes. My worry here, though, is that if we aren't careful, we may open the door to naive implementations doing weird things, like leaking promises into handlers when it really isn't necessary or desirable. The only thought I have right now is to somehow say that if the fulfillment value has become available but a variable/binding (whatever the correct ES term is here?) can't be provided for it, a promise for it must be provided instead. That'll def cause some head scratching :/ |
Tests are updated! https://github.com/promises-aplus/promises-tests/compare/spec-1.1 When.js passes with flying colors, RSVP and Q fall down on a few points. |
Legendary passes too (under Node v0.10), so I'm +1 ;-) |
I think 1.1 is ready :). Going to try pushing a new version to gh-pages, and thus to http://promisesaplus.com/ |
Sounds good. Some of us will probably just start looking at ES6-AP2 which |
I'd like to get signoff at least from the following people before publishing 1.1 to gh-pages. If I forgot your name, please don't feel offended!!
You can check out the full diff at this GitHub compare link, and you can see the current-and-hopefully-soon-to-be-1.1 text in master, but the major changes you are ratifying are:
onFulfilled
andonRejected
must be called as functions, i.e. with nothis
value.thenable.then
s that throw upon getting them.This last point is probably a breaking change for existing libraries. It amounts to using
thenable.then(resolve, reject)
instead ofthenable.then(fulfill, reject)
in your assimilation code, and extra edge-case handling. The test suite currently does not cover this point, but of course will if we move forward.This is also a great time to do some word-smithing. I am especially curious if anyone can do a better job with the Promise Resolution Algorithm. We also have #80 outstanding regarding possibly replacing "fulfillment value" and "rejection reason" with simply "value" and "exception."
Excited!! Let's go!!
The text was updated successfully, but these errors were encountered: