mootools / mootools-core

MooTools Core Repository
https://mootools.net
2.65k stars 507 forks source link

Class.Thenable #2743

Closed timwienk closed 8 years ago

timwienk commented 8 years ago

This is code I made almost a year ago, I finished it (or actually mostly its specs) now we use Mocha, so I could integrate the Promises/A+ tests.

New Class mixin. When implemented in a Class, it makes the class "thenable" in the Promises/A+ sense of the word, meaning it can be used in Promise style flows by using its then method.

The implementation, however, is more than just a "then" method. Any instance of a Class implementing Class.Thenable is a Promises/A+ compliant object (generally referred to as "a Promise") with only one exception: it is possible to reset the Class's value resolution state fully (rejecting pending reactions, and starting empty) to support a Class instance living for longer than just the lifetime of one value resolution.

Example using Request:

var request = new Request();
request.send().then(function(response){ console.log(response.text); });

Example hooking into a native Promise:

var request = new Request();
var promise = Promise.resolve(request);
request.send();
promise.then(function(response){ console.log(response.text); });

It's even quite easy to create an actual Promise Class if you want, but that seemed a bit redundant to include in MooTools Core. I used the actual way to do this as example in the Docs, though:

var Promise = new Class({
    Implements: Class.Thenable,
    initialize: function(executor){
        if (typeof executor !== 'function'){
            throw new TypeError('Promise constructor takes a function argument.');
        }

        try {
            executor(this.resolve.bind(this), this.reject.bind(this));
        } catch (exception){
            this.reject(exception);
        }
    },
    resetThenable: function(){
        throw new TypeError('A promise can only be resolved once.');
    }
});

This also solves issue #2643.

Full Travis run:

anutron commented 8 years ago

I guess my only real comment here is if we want this integrated into core. That is to say whether or not this feature needs to be a requirement for Fx, Request, etc. I get it; you can't drop it in on its own (at least, not without something hacky like Class.Refactor), so if you want it in Fx/Request you have to put it here, but in some ways this adds bytes to the core library for functionality that maybe isn't primarily useful for most developers.

I'll let others sound off on whether that's the case or not. The code itself looks fine to me. Just a matter of whether we want to add the bytes for this feature.

fakedarren commented 8 years ago

I like it. Gzipped the code will be next to nothing, and promises are pretty much required nowadays. +1 from me. It makes a good blog post!

kentaromiura commented 8 years ago

:+1:

SergioCrisostomo commented 8 years ago

:+1:

arian commented 8 years ago

It's still true that request.send() === request?

Isn't it usually that some action, in the case of request .send() returns a new promise object, which can be seen as a 'placeholder' until the promise is fulfilled or rejected?

timwienk commented 8 years ago

Thanks for the feedback and questions. Quite happy to see practical responses.

@anutron, I agree with the question you put forward, I guess it's quite obvious I agree with @fakedarren's answer, otherwise I wouldn't have written the code, but it's a good thing to bring up. I think this implementation offers something new and very useful by not attempting to add "yet another Promise implementation", but instead offering a way to work with Promise implementations in a compliant manner and still working fine stand-alone.

To answer @arian I'll need a bit more text, because it gets a bit detailed, so brace yourselves (and blame @arian, he asked!):

[TL;DR] I'm generally available on IRC if anyone prefers to discuss/ask there in case this text isn't clear or is too long to parse.

Yes, request.send() still returns itself. Classes and their instances work as they always have, nothing changed in that respect, and everything works as we expect the Classes to. What this code does is not actually implement Promises itself, it implements a mixin to make any instance of the Class itself Promises/A+ compliant (or to actually implement Promises if you want to, see the example in the description of the PR).

There is no rule (or reason for) saying that an arbitrary object cannot act the same way as a Promise, because its goal is to (quite literally) promise that a value will be resolved at some point. You can basically interpret new Request as the instantiation of very specific kind of Promise that goes to resolve its value by calling send. Another way to look at it is that a new Request keeps a Promise internally which you interact with, but the former seems more accurate.

If you do want an actual Promise to work with, you could do Promise.resolve(request) (or Promise.resolve(request.send()) if you feel that's nicer), which works because of part 2.3.3.3 of the spec: the Promise calls request.then and uses that to resolve itself (it basically adopts the request's state).

The usual (and even mandatory) thing is that calling then does not return itself and returns a new object that gets resolved with the value returned by the argument(s) you pass to then, which is indeed what it does.

I do want to point out that referring to Class.Thenable as a Promise implementation is not technically correct, because of the exception I mentioned in the description of this PR, which is related but not the same as what you mention, @arian. As I mentioned, the case is not that send() should return a new Promise(-like) object, but it is the case that a Promise can only be used/resolved once. Because Class instances are often used more than once (e.g. multiple request.send or fx.start calls), this implementation makes the state resettable. Using the "internal Promise" analogue I used earlier: think of it as ditching the old Promise (rejecting it if it's not yet resolved) and taking on a new one. This way nothing is left unresolved, and it's similar to either doing a new send after things are done, or canceling the old send.

The only thing that does get interesting is using Class.Thenable style interaction together with Chain style interaction, these are two different ways of chaining actions. I noted this in the docs. (Probably a minor issue anyway, because of defaults and the unlikelihood of people using chain if they can use then. We could even consider adapting Chain to use Class.Thenable, but let's not get into that right now.)

arian commented 8 years ago

:ship: it

anutron commented 8 years ago

I think your description here Tim belongs in the blog post. Yes, it's fairly technical, but so is our audience (what's left of it).

On Fri, Oct 9, 2015 at 4:22 AM, Tim Wienk notifications@github.com wrote:

Thanks for the feedback and questions. Quite happy to see practical responses.

@anutron https://github.com/anutron, I agree with the question you put forward, I guess it's quite obvious I agree with @fakedarren https://github.com/fakedarren's answer, otherwise I wouldn't have written the code, but it's a good thing to bring up. I think this implementation offers something new and very useful by not attempting to add "yet another Promise implementation", but instead offering a way to work with Promise implementations in a compliant manner and still working fine stand-alone.

To answer @arian https://github.com/arian I'll need a bit more text, because it gets a bit detailed, so brace yourselves (and blame @arian https://github.com/arian, he asked!):

[TL;DR] I'm generally available on IRC if anyone prefers to discuss/ask there in case this text isn't clear or is too long to parse.

Yes, request.send() still returns itself. Classes and their instances work as they always have, nothing changed in that respect, and everything works as we expect the Classes to. What this code does is not actually implement Promises itself, it implements a mixin to make any class Promises/A+ compliant (or to actually implement Promises if you want to, see the example in the description of the PR).

There is no rule (or reason for) saying that an arbitrary object cannot act the same way as a Promise, because its goal is to (quite literally) promise that a value will be resolved at some point. You can basically interpret new Request as the instantiation of very specific kind of Promise that goes to resolve its value by calling send. Another way to look at it is that a new Request keeps a Promise internally which you interact with, but the former seems more accurate.

If you do want an actual Promise to work with, you could do Promise.resolve(request) (or Promise.resolve(request.send()) if you feel that's nicer), which works because of part 2.3.3.3 of the spec https://promisesaplus.com/#point-56: the Promise calls request.then and uses that to resolve itself (it basically adopts the request's state).

The usual (and even mandatory) thing is that calling then does not return itself and returns a new object that gets resolved with the value returned by the argument(s) you pass to then, which is indeed what it does.

I do want to point out that referring to Class.Thenable as a Promise implementation is not technically correct, because of the exception I mentioned in the description of this PR, which is related but not the same as what you mention, @arian https://github.com/arian. As I mentioned, the case is not that send() should return a new Promise(-like) object, but it is the case that a Promise can only be used/resolved once. Because Class instances are often used more than once (e.g. multiple request.send or fx.start calls), this implementation makes the state resettable. Using the "internal Promise" analogue I used earlier: think of it as ditching the old Promise (rejecting it if it's not yet resolved) and taking on a new one. This way nothing is left unresolved, and it's similar to either doing a new send after things are done, or canceling the old send.

The only thing that does get interesting is using Class.Thenable style interaction together with Chain style interaction, these are two different ways of chaining actions. I noted this in the docs. (Probably a minor issue anyway, because of defaults and the unlikelihood of people using chain if they can use then. We could even consider adapting Chain to use Class.Thenable, but let's not get into that right now.)

— Reply to this email directly or view it on GitHub https://github.com/mootools/mootools-core/pull/2743#issuecomment-146838222 .

SergioCrisostomo commented 8 years ago

++ also for a blog post!