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

Test returnedPromise fulfilled/rejected with a promise #20

Conversation

nobuoka
Copy link

@nobuoka nobuoka commented Feb 13, 2013

Hello.

I develop a Promises/A+ library Ten.Promise, which aims to have interface-level compatibility with WinJS.Promise.

Now, my library Ten.Promise is different from WinJS.Promise in treatment of a returnedPromise fulfilled/rejected with a promise.

var sentinel = {};
var sentinelPromise = Promise.wrap(sentinel);

// `p` is a fulfilled promise
var p = Promise.wrap(100);

// a pseudo promise which pass a promise to callback functions
var pseudoPromObj = {
    then: function (s) {
        s(sentinelPromise);
    }
};

var p2 = p.then(function () {
    return pseudoPromObj;
});
p2.then(function (val) {
    // Here is difference:
    //   `val` is `sentinel` in case of using WinJS.Promise,
    //   while `val` is `sentinelPromise` in case of using Ten.Promise
});

I think Ten.Promise is compliant to the Promises/A+ specification. However, either implementation passes the current promises-tests, so I added tests about this behavior to promises-tests.

Thanks.

@domenic
Copy link
Member

domenic commented Feb 13, 2013

This is a great gap in the tests; thank you for finding!

I'm having a hard time deciding what's correct but after going back and forth, I think you're right. When attempting to adopt pseudoPromise's state, you can only rely on the then method, which is clearly telling you that it's fulfilled with sentinelPromise. In particular, it's not telling you pseudoPromise is waiting on sentinelPromise; if that were the case, it wouldn't call you back immediately.

We might want to change the spec to account for this case, since ideally one should avoid fulfilling with promises whenever possible, but it'd require a decent bit of thought. Discussion over at promises-aplus/promises-spec#75

Much appreciated! And also, Ten.Promise sounds like an amazing idea!! We'll add it to the list of implementations over at promises-aplus/promises-spec#72

@nobuoka
Copy link
Author

nobuoka commented Feb 16, 2013

We might want to change the spec to account for this case, since ideally one should avoid fulfilling with promises whenever possible, but it'd require a decent bit of thought. Discussion over at promises-aplus/promises-spec#75

I see. If the spec is changed, I'm willing to change the implementation of my library.

And thank you for adding “Ten.Promise” to the list of implementation!

@domenic
Copy link
Member

domenic commented May 16, 2013

This spawned a huge discussion that radically changed the Promises/A+ spec in version 1.1 to specify the assimilation of foreign thenables in much more depth. There are tests for 1.1 in a branch, which will become master once we nail down 1.1 in its entirety (soon!).

Thanks so much for bringing this up. It's had a big impact.

@domenic domenic closed this May 16, 2013
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 this pull request may close these issues.

2 participants