domenic / promises-unwrapping

The ES6 promises spec, as per September 2013 TC39 meeting
1.23k stars 94 forks source link

Make subclassing work automatically #31

Closed domenic closed 10 years ago

domenic commented 10 years ago

This is not final in any way, and as mentioned previously this is not urgent since @@create is the only way to make this relevant, but I wanted to get it out there.

Drawing heavily on Allen's work on Array and so on, this series of commits uses a series of techniques to make Promise subclasses Just Work(TM). The static methods will all return instances of the new type, and in fact will in most cases execute any special behavior for when you do something like:

class InstrumentedPromise extends Promise {
    constructor(resolver) {
        super((resolve, reject) => {
            resolver(
                x => {
                    console.log("resolve called");
                    resolve(x);
                },
                r => {
                    console.log("reject called");
                    reject(r);
                }
            );
        });
    }
}

(stare at this example for long enough and I'm pretty sure it will make sense.)

Promise.cast is a bit special since we want to ensure it always gives you back whatever type you call it on (e.g. InstrumentedPromise.cast(x) always gives you an InstrumentedPromise). The technique used there is a bit tricky and could be strengthened; I'm not a huge fan of it right now but we'll see.


Example for @slightlyoff of passing in new capabilities to the resolver:

class PromiseWithSpecialReject extends Promise {
    constructor(resolver) {
        super((resolve, reject) => resolver(resolve, reject, {
             specialReject() { reject(new Error("Specific special error")); }
        });
    }
}

const speciallyRejected = new PromiseWithSpecialReject((resolve, reject, { specialReject }) => specialReject());
erights commented 10 years ago

What does "real constructor" mean?

domenic commented 10 years ago

What does "real constructor" mean?

It would mean however the promise was created, i.e. if it was created via XPromise[@@create](), then its "real constructor" would be XPromise.

domenic commented 10 years ago

Updated to address feedback, including adding the new [[PromiseConstructor]] internal property as something Promise.cast can check without observable side effects. Also updated the testable implementation, which helped find a typo. Merging.

petkaantonov commented 10 years ago

Could the requirement for checking if reading the property this.constructor throws be relaxed? Unlike the same routine when dealing with thenables, this is impossible to do without seeing a performance hit in normal use. Try catching in the busiest method isn't exactly free even with the try-catch isolation optimization.

I have seen issues that have actually no, or marginal, effect on performance drive some decisions so I hope this feature which appears to actaully have no practical use case could be removed.

domenic commented 10 years ago

@petkaantonov unfortunately I don't think it can be relaxed. We need to guarantee that then never throws synchronously. I can't really see any other way to do this; can you?

domenic commented 10 years ago

Implementers in C++ should have access to some tricks, like optimistically assuming that this.constructor === Promise and only deoptimizing if it gets reassigned.

skaegi commented 10 years ago

I'm starting to look at what's involved in creating a CancellablePromise subclass which adds a new cancel method to the promise and follow the behavior we worked out at Promises/A+. One thing I wonder is if there needs to be sub-class affordances for the case where a sub-class is resolved with another promise that is not the same sub-class.

For example:

var pending = pending(); // vanilla promise
var cancellableDeferred = getDeferred(CancellablePromise);

cancellableDeferred.resolve(pending);

// does a sub-class get to decide if this a no-op or can I still cancel the promise
cancellableDeferred.promise.cancel(); 

The rough behavior I'm looking at for Cancel is...

Cancel(p)

  1. If p.[[Following]] is set
    1. If IsFunction(p.[[Following]].cancel) then call p.[[Following]].cancel()
    2. otherwise unset p.[[Following]] and call SetReason(p, CancellationError)
  2. Otherwise if p.[[Value]] is unset and p.[[Reason]] is unset then call SetReason(p, CancellationError)
domenic commented 10 years ago

@skaegi sorry for the delay in replying, let's see here...

does a sub-class get to decide if this a no-op or can I still cancel the promise

You should definitely still be able to cancel the promise; resolve should transfer all of the state from pending to cancellableDeferred.promise. I think that's how it's specced now.

The rough behavior I'm looking at for Cancel is...

Hmm, I think I see what you're trying to do here. In contrast to the proposal at https://github.com/promises-aplus/cancellation-spec/issues/6, it would make the following work:

const cp = new CancellablePromise(() => { }, () => console.log("cancelled cp"));

const cp2 = new CancellablePromise(resolve => resolve(cp));

cp2.cancel(); // causes "cancelled cp" to be logged.

which seems like a good thing. Very nice.


So... I guess, I'm not sure what affordance you would be looking for? Things seem to work pretty well as-is?

domenic commented 10 years ago

@skaegi I just had a related thought, or perhaps it was the same thought you were having and I didn't quite understand. Namely... how can you do subclassing of the type you want, in pure JS?

We want to make CancellablePromise, and all such things, possible in pure JS. But your above text makes use of spec-internal properties like [[Following]] and [[Value]]. Which is fine for a spec, but might indicate that CancellablePromise is not implementable in pure JS, if there's no way to express the same operation using JS semantics.

Can you think of a way to implement your cancel operation without using the internal properties? If not, that may mean that we need to expose them to subclasses somehow.

skaegi commented 10 years ago

Yes, probably easiest to talk in terms of code so let me show you my ES5 implementation (which is using your new "then" sub-classing technique which helps). It's based on sub-classing Deferred but I'll put up a repo when I get a moment to show an implementation based on Promise.

function CancelableDeferred() {
    Deferred.apply(this);
    var resolve = this.resolve;
    var reject = this.reject;
    var promise = this.promise;
    var then = promise.then;
    var called = false;

    var canceler;
    // ideally protected
    Object.defineProperty(promise, "_canceler", {
        set: function(value) {
            canceler = value;
        }
    });

    this.resolve = function(value) {
        if (!called) {
            called = true;
            if (value && typeof value.then === "function") { //IsPromise
                canceler = value;
            }
        }
        return resolve(value);
    };
    this.reject = function(reason) {
        if (!called) {
            called = true;
        }
        return reject(reason);
    };

    promise.then = function(onResolve, onReject, onProgress) {
        var derived;

        function wrap(f) {
            return function() {
                var result = f.apply(null, arguments);
                if (result && typeof result.then === "function") { //IsPromise
                    derived._canceler = result; //ideally protected access
                }
                return result;
            };
        }
        derived = then(wrap(onResolve), wrap(onReject), onProgress);
        derived._canceler = promise; //ideally protected access
        var derivedCancel = derived.cancel;
        derived.cancel = setTimeout.bind(null, derivedCancel, 0);
        return derived;
    };
    promise.cancel = function() {
        if (canceler && typeof canceler.cancel === "function") {
            canceler.cancel();
        } else {
            if (!called) {
                var cancelError = new Error("Cancel");
                cancelError.name = "Cancel"
                reject(cancelError);
            }
        }
        return promise;
    };
}

This implementation passes the tests I originally wrote at Promises/A+ (https://gist.github.com/skaegi/4736504) as well as the Orion Deferred tests so it's not entirely a pipe dream.

I ran into a few issues which I worked around but...

  1. To get at the equivalent of "following" and "delegated" I had to introduce "_canceler"
  2. I couldn't re-use my internal async trampoline.
skaegi commented 10 years ago

I've moved my implementation across to using Promises instead of Deferreds as the basis. Here's a link to a repo that has my current work -- https://github.com/skaegi/promises-cancel.

CancellablePromise.js (and Promise.js) in the repo above are now Promises/A+ 1.1 compliant as well as matching the "Promise Object" spec so it appears this sort of sub-classing is possible albeit requiring a fair bit of gymnastics.

In addition to the two issues above another is...

  1. In my cancel sub-class I need to check if a value is a then-able however the Promises/A+ spec only lets me "get" the then method once. I ended up having to create a new object to hold the "then" field to work around this restriction. e.g.
result = Object.create(result, {
    then: {
        enumerable: true,
        configurable: true,
        writable: true,
        value: resultThen.bind(result)
    }
});

Since I was now passing the wrapped object down to the superclass method this means I also had to do the identity check in my sub-class to ensure I was not resolving the promise in my sub-classes code.