-
Notifications
You must be signed in to change notification settings - Fork 95
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
Spec creates strict requirements on subclass constructors #87
Comments
This appears to be done in order to make (new MyPromise(...)).then(...) instanceof MyPromise; return true. I.e. |
Jonas, I'd encourage you to look at how array subclassing works in ES6. That is the model promise subclassing is based on, and we will not be breaking parity with that model. It would be horrible if .then() returned something of the wrong type; it defeats the point of subclassing if a simple functional transform like .then(JSON.parse) strips away all the subclass capabilities. Similar to .map on an array subclass instance. The constructor contract is used for much more than .prototype.then, of course. It is used by literally every method, e.g. .resolve, .reject, .all, .race, … For your network request idea, it is not encouraged to subclass Promise to implement higher-level objects of that sort, such that they cannot be created by user code that is given the power to resolve or reject. If something is a Promise subclass, its constructor must follow the expected Promise constructor contract. |
As an analogy, you would not create an Array subclass ArrayOfNetworkData such that you do |
Note that none of ".resolve, .reject, .all, .race," will by default live on subclasses. Neither on the instances of the subclass, nor on the subclass' constructor object. Of course, the subclass could manually copy any of those functions over to the constructor object. I assume that was your intended use?
In other languages you most definitely would. I'm not as familiar with how subclassing is intended to work in JS so I can't speak for that. Which was why I opened this issue. |
Subclasses inherit methods of the constructor. I am not sure why you would think otherwise? |
Let me know if you think there's anything else that needs to be clarified; closing for now. We can also tag in @allenwb to get his thoughts, since ES6's subclassable built-ins are largely his creation and I'm just happily copying what is IMO an elegant pattern. |
There are several interesting design issues in this thread. I'm going to do several comments addressing various issues. First, I've never been all that happy with the way the es6 collection methods use the constructor protocol to create new objects derived from the class of the original collection. Most of the steps that do this sort of constructor call have a margin comment that says "It would be nice to have a more explicit way to create a collection with a pre-specified number of elements". We talked about adding a new static method on Array constructors (maybe Array.sized(n) to do that. Whenever you are do this sort of "cloning" you have to have some protocol that you follow to accomplish it. In the case of Arrays, the array constructor with one argument just happened to already provide the protocol that we needed and so that's what we used. However, as Jonas points out, that precludes subclasses from providing argument protocol that differ from their superclasses and sometimes that is actually what you want to do. We could avoid this issue for promises defining a static method on Promise that creates a derived promise. Essentially we could expose the GetDeferred abstract operation as that static method. |
Hmm, thanks for your thoughts, Allen. I'm reluctant to expose GetDeferred ( Indeed, the reason GetDeferred is used so much throughout the spec is not because it's idiomatic or how you would implement these things in ES. It's because creating closures in the meta-language is a heavyweight and (apparently) undesirable operation, so consolidating that into a single "Deferred Construction Function" closure is the workaround we use. For example, compare the ES implementations of I thought once you were mentioning using a symbol, e.g. However, I'd worry about how that impacts subclasses that want to override I am still not convinced this is a problem, either for the collection classes or for Promise. I think it's OK for constructors to be assumed to have a certain signature. But I'll reopen and we can track this possibility if you want to work together on something that applies to both promises and collection classes. |
Symbols don't hide things. They avoid only accidental collisions. |
It also isn't clear to me why the promised returned by 'then' and other methods that use GetDeferred should necessarily be of the same class as the original promised that was then'ed. Using Jonas' NetworkRequester as an example, I wouldn't necessarily expect the value of (new NetworkRequest(url)).then() to be a NetworkRequest. If it was, that would mean that the url or perhaps some other state might have to be propagated. Instead what 'then' returns is essentially just a promise on the seattlement of the then. This seems to me like it should be just a regular Promise or perhaps its own Promise subclass (ChainedPromise??) One way to generalize the behavior would to use a promise method to determine the class of a derived promise instead of always using the promise's constructor to create the derived promise. Let's give that method the working name 'deferalClass'. the default 'then' method would use an expression like: let deferred = this.deferalClass().getDeferred(); to generate its return value. The default definition of deferalClass might be return this.constructor; or it might be return Promise In either case a subclass could over-ride it how seemed appropiate. |
I think the essential problem is that the NetworkRequest example is pretty unnatural, and should be implemented with composition, not inheritance. Things that "is a" promise would be e.g. cancellable promises, or progress promises (a bad idea but an illustrative example), or other generic containers that are still representations of future values, but with extra features. As such, it's pretty important for the return value of I don't think you would want a solution like |
This is not so crazy at all. In fact it is exactly how the very successful Smalltalk collection hierarchy was designed. It has a method named
They are also good for explicitly distinguishing the implementation laying of an abstraction from the usage API of the abstraction. A class that is well designed for subclassing exposes a distinct subclassing contract. The subclassing contract is typically represent by methods that implement abstract algorithms that are designed with parameters/extension points that can be utilized by subclass implementations to customize the algorithms. The parameters/extension points are typically resented by self-callls that may be over-ridden by subclasses. Typically a lot of design thought needs to go into deciding what are the fixed part of the abstract algorithm that all subclasses must share and what are the appropriate dimensions of customization exposed by the extension points. It's hard to get this just right, but too many extension points give you a mess that nobody can understand and too few gives you a rigid class that can't really be usefully extended. Using Symbols to name the subclassing contract methods (and especially the extension points) are a fine way to layer the class implementation such the the subclass contract doesn't complicate the public usage API. So, what is the subclassing contract for Promises? |
I'm not overly concerned about whether composition or inheritance is the better design approach for any particular situation. I'm happy to leave that up to the framework or application developer. Once you start down the path of providing a subclassing contract you have given up control of how a developer may use it. But there is one important thing to point out. The constructor parameter list and body of a class is the only class element that isn't directly inherited by a subclass. It is not at all uncommon for subclass to have a completely different constructor signature from that of their superclass. Arguably, the ability to change the If you assume that subclassing Promises is going to be useful, then you also should assume that some subclasses will want to have different constructor signatures. |
Symbol-named properties are publicly accessible. We don't currently have even a convention for distinguishing symbol-named properties that are meant to be publicly accessed from those that are meant only to be accessed only within an inheritance hierarchy, i.e., the main "public" vs "protected" distinction in C++ and Java. (Though "protected" in both is more complex in different ways.) WeakMaps / Relationships can provide both truly "private" as well as truly "protected" access, according to who has access to the WeakMap. And it correctly virtualizes across membranes -- both transparently and securely. It is the right tool for the job. |
A first cut at a subclassing constract for promises: class Promise {
//sublcass contract
static [Symbol.newDeferred] () {
var resolve, reject, promise;
/* engine magic that is approximately equivalent to
promise = new this((r,x)=> ({resolve,reject}={r,x}));
but which doesn't depend upon the constructor signature
*/
return {promise, resolve, reject}
}
[Symbol.getDeferred]() {
return this.constructor[Symbol.newDeferred]()
}
//public class contract
static resolve(x) {
let deferred = this[Symbol.newDeferred]();
deferred.resolve(x);
return deferred.resolve;
}
// etc. for other class methods
//public instance contract
then(onFulfilled, onRejected) {
let deferred = this[Symbol.getDeferred]]();
...
}
// etc. for other instance methods
constructor(executor) {/* engine magic, as currently specified */}
} A couple things to note. If you have a reference to a Promise subclass you can use @@newDeferred to create an instance of it. This doesn't leak a new capability because you could have directly On the instance side @@getDeferred encapsulates access to the appropriate Promise subclass and doesn't directly leak it. The |
@erights Essentially all uses of Symbols within the ES6 spec. are to stratify extension points in this manner. They are very useful for establishing a clear distinction between the extensibility contract and the usage contract of an abstraction. |
How would |
Simpler is better. This is future-friendly as well, since a "mapClass" property can be added at a later date to override this behavior. |
That same argument applies to Array.prototype.map. Whichever we do for one should go for the other. |
I can agree with that. Probably then, the most straightforward way to remove the restrictions on the constructor signature is just to expose |
Also, although I admit that the closure pattern is a nice way of implementing async functions in the absence of async functions, I don't necessarily agree that And, after all, we have to give bloggers something to write about ; ) |
@zenparsing I am interested in an example of a real situation where you believe using deferreds is really what you want... |
Take a look at http://static.googleusercontent.com/media/research.google.com/en/us/pubs/archive/40673.pdf Page 15, Fig. 2, Line 14 Notice that it's not possible to refactor that particular example using the closure pattern alone, without extracting the resolver function. Also, if you ever need to pass the resolve/reject capability out of the originating function, you'll need to perform that annoying extraction thing. Also, if you want to avoid indentation (and that's a valid choice, too)... I don't see a problem with the statement "In most cases, you ought to use the closure pattern."
|
The extraction is pointless anyway. This code does exactly the same thing: var escrowExchange = (a, b) => { // a from Alice , b from Bob
return Q.promise(resolve => {
resolve(Q.race([
Q.all([
transfer(decisionP, a.moneySrcP, b.moneyDstP, b.moneyNeeded),
transfer(decisionP, b.stockSrcP, a.stockDstP, a.stockNeeded)
]),
failOnly(a.cancellationP),
failOnly(b.cancellationP)
]));
});
};
Yet you never need to do that in a real situation. |
What about
Never ever ever? Says you ; ) |
Also, the promise, the resolver, and the rejecter have always, by design, been separate capabilities which in principle can be passed around independently to different actors. It is central to the design. |
Ok I didn't notice that but I think it's still possible to do without extraction: var escrowExchange = (a, b) => { // a from Alice , b from Bob
var decisionP = Q.resolve().then(() =>
Q.race([
Q.all([aPromise, bPromise]),
failOnly(a.cancellationP),
failOnly(b.cancellationP)
]);
);
var aPromise = transfer(decisionP, a.moneySrcP, b.moneyDstP, b.moneyNeeded);
var bPromise = transfer(decisionP, b.stockSrcP, a.stockDstP, a.stockNeeded);
return decisionP;
}; |
s/you never/you would hope to never/ I agree that constructor method is the ideal way to go, unfortunately sometimes it is not practical/possible to refactor some entangled code. I suppose my point is, constructor pattern should always be possible, sometimes not practical but we should strongly encourage it. If user must for some reason needs the deferred pattern, they can quickly implement the pattern themselves or grab some code that does. This mitigates my foot-gun concerns. |
Also, adding |
@domenic I've been contemplating adding this mechanism to the Array/etc, specs. for a while and this discussion has convinced me that I should just go ahead and do it. For Promises I still suggest we go with the @@getDeferred/@@newDeferred design I gave in a previous comment. However, there is one aspect of that design I'd like some feedback on. The @@getDeferred method is design to not directly expose the class object chosen to create the deferred promise. So handing out a promise instance is not a general capability on the deferred constructor object. Does anybody care? Replacing the @@getDeferred method with a simple @@deferredConstructor data property wold be a simpler design. |
I am not sure I understand the need for two methods; can you clarify? |
@zenparsing class PromiseWithDeferred extends Promise {
static deferred() {return this[Symbol.newDeferred]()}
} |
@allenwb Since engines want to implement now, does that dictate any requirement to keep symbols out of the core API? |
Engines are generally not bothering to implement the subclassability of any of the built-ins, including promises, for now. So I don't think any of the concerns discussed here are that pressing. |
@domenic Promise[Symbol.newDeferred] is a (potentially) subclass specific factory method whose sole responsibility is to create a deferred instance of that subclass. It does so without using the subclasses constructor body so its default implementation doesn't have to worry about the constructor argument pattern (Jonas' concern). Promise.prototype[Symbol.getDeferred] is a (potentially) subclass specific instance method whose primary responsibility is to determine which Promise subclass should be used to create deferred promises corresponding to settlement of operations upon the Promise object getDeferred is invoked upon. In other words the distinct concerns (and extension points) are:
Here is the spec. that I would use for PromiseSymbol.newDeferred:
(the above is essentially the same as what I currently have as the definition of the GetDeferred abstraction operation except that the result is an object instead of an internal data record. |
OK. So newDeferred is the "how do my static methods behave," and getDeferred is "how do my instance methods behave." I am not sure distinguishing between these is valuable. Again I would draw a parallel with Array: is it vauable to have Also. I am not sure why you have the constructor doing engine magic in your design above: why doesn't it just reuse newDeferred? It seems now that if people overrode newDeferred to tack on new creation behavior, they would have to also override the constructor in order to make sure that new creation behavior happened for directly-created instances and not just those created by static methods. (Also, these names are pretty bad, but we can leave the bikeshedding for later.) |
Can't resist: Since this method creates a new fresh thing, neither "Promise.deferred()" nor "Promise.getDeferred()" are good names. "Promise.makeDeferred()" is better. "Promise.defer()" is both best and traditional. What is makes and returns is a "deferred". |
|
I wonder if the Array constructor should be changed to work in terms of Array.of? |
More generally, I wonder if the pattern should be that all classes that want to support subclassing also have a static "create a new instance of me not using my constructor" method, like we have for Array and it sounds like we now want for Promise. |
+1 On Tue, Dec 31, 2013 at 11:07 AM, Domenic Denicola <[email protected]
Text by me above is hereby placed in the public domain Cheers, |
Yes! This is the same sort of separation of concern. But, of course these would be written as Conflating those two responsibility into a single method results in a less flexible subclassing contract. For example, a subclass may need to have multiple variations of |
That's pretty much were we are heading. I'd be fine with naming such a method Regardless of the name, such a method isn't the same think as |
Yeah, that is what makes it weird. What if |
Ah, I not sure what you're saying. I intended Regarding reliably stashing/accessing original values, you can always do things like: |
I am just trying to find a way to create a unifying abstraction that is not promise-specific ("deferreds"), and instead something like a generic stashed-away pseudo-constructor that will work for Array, Promise, etc. I am not sure my idea was well thought out, but I think this is a good goal, rather than having Array have its own size-accepting method, promise its own deferred-returning method, etc. If we're trying to establish a set of robust subclassing best practices, we should make them less idiosyncratic. |
I think newInstance (under what ever name) what you are looking for. It's default is definition is essentially:
|
So would a valid version for the base static [Symbol.newInstance](resolver) {
let obj = super[Symbol.newInstance](); //or perhaps this[Symbol.create]()
let resolve = makeResolveViaMagic(obj);
let reject = makeRejectViaMagic(obj);
resolver(resolve, reject);
return obj;
} ? Then we would change GetDeferred(C) to use |
@domenic Also, I doesn't think you need the resolver argument. It certainly isn't need to implement GetDeferred. In most situations, it is probably best to think about I still think that the interface I proposed is the right one for creating defers (ignoring naming concerns). Because of their nature, crating a Promise without also gaining access to its resolve/reject functions isn't very useful. the @@newDefered method fills the role of @@newinstance but also makes resolve/reject available. I don't see why anybody would every call Promise.@@newinstance instead of @@newDeferred. |
I was trying to eliminate newDeferred, since it seems weird to have an extra pseudo-constructor thing that doesn't return an instance of the desired type. I was hoping to encapsulate that into newInstance.
I agree, which is why it seems like a reasonable approach that whatever method this is takes a |
@domenic There really isn't a "deferred object" the result object is just the ES mechanism used to result multiple values from a function. |
Exactly. Promises have this special nature which doesn't really apply to collection types. In order to be useful to the base class, the subclass must provide some way of getting a |
@domenic I'm not sure why you say promises are "generic containers". Would it not be useful to have something like email.getHeader().getSubject() where Also, arranging for I'm not sure how to arrange for this. It's pretty trivial to do "by hand", but it feels like it should be possible via EmailPromise.prototype.getHeader = function () {
var self = this;
return new HeaderPromise(function (resolve, reject) {
self.then(resolve, reject);
});
} |
As the spec is currently defined, it creates some rather strict requirements on any subclass of Promise.
Specifically it requires that any subclass of Promise has a constructor that can be called using
where
resolve
andreject
must resolve and reject the returned instance respectively.This means that I can't create a subclass of Promise which implements a network request and which uses a signature like
new NetworkRequest(url)
.Is it expected that built-in classes will create such restrictions on subclassing?
The text was updated successfully, but these errors were encountered: