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

Default constructor for Promise statics #544

Closed
zloirock opened this issue Apr 14, 2016 · 12 comments
Closed

Default constructor for Promise statics #544

zloirock opened this issue Apr 14, 2016 · 12 comments

Comments

@zloirock
Copy link

zloirock commented Apr 14, 2016

I had some issues like this and I also can't understand this inconsistency - why Promise statics hasn't a fallback for calling without a context, like Array statics:

SubArray.from([]).constructor === SubArray
var from = Array.from
from([]).constructor === Array

SubPromise.resolve(Promise.resolve()).constructor === SubPromise
var resolve = Promise.resolve
resolve(Promise.resolve()).constructor // Error

Sometimes shortcuts for Promise.resolve or Promise.all can be usefull. Maybe makes sense add this possibility?

IIRC it was interesting also for @ljharb and @stefanpenner.

@stefanpenner
Copy link

I spoke with @domenic about this, and he articulated the reasoning, maybe he can do so again in a written form. I don't want to accidentally misspeak.

@ljharb
Copy link
Member

ljharb commented Jul 29, 2016

@domenic if you could elaborate on the reasoning for making (0,Promise.resolve)() throw instead of being equivalent to Promise.resolve(), it would be very helpful.

@domenic
Copy link
Member

domenic commented Aug 1, 2016

A large part of defining subclass-friendly classes is not hard-coding knowledge of specific types into the base classes. I think it was a mistake to do so for arrays, and did not want to do so for promises. These are fundamentally methods, and adding hacks so that they can also be treated as functions is not good. It would also be very surprising if (0,SubPromise.resolve)() returned a Promise, when SubPromise.resolve() returns a SubPromise.

@ljharb
Copy link
Member

ljharb commented Aug 1, 2016

Thanks for the reply! While I'm not convinced overall, I think the latter example (0,SubPromise.resolve) is a very very strong argument, enough that I consider it not worth pursuing further.

@zloirock, please feel free to close if you agree.

@allenwb
Copy link
Member

allenwb commented Aug 1, 2016

The fall backs that exist in Array are primarily there to preserve backwards compatibility for edge cases of functionality that existed in ES3-ES5. They wouldn't be there if we were starting from a clean slate

@stefanpenner
Copy link

@allenwb thanks, could you also share the reasoning why, in an ideal world, the fallback wouldn't be present. As I believe that question is at the root of this thread.

@ljharb
Copy link
Member

ljharb commented Jan 21, 2017

@allenwb friendly ping on @stefanpenner's question :-)

@erights
Copy link

erights commented Jan 27, 2017

Just for the record, since I already lost this one a long time ago:

I was against class-side inheritance for exactly this reason. Prior to classes, by far the dominant pattern for static methods (i.e. methods on the class object) was for them not to be this-sensitive. As non-this sensitive functions, they can be extracted into separate first class values that need not be bound to preserve their functionality. That is why the precedent from Smalltalk, where class-side inheritance is quite pleasant, does not apply to JavaScript. Smalltalk methods are do not double as standalone functions but for funny this-binding rules.

@allenwb
Copy link
Member

allenwb commented Jan 28, 2017

@stefanpenner For the reasons that both @domenic and @erights have mentioned.

assume:

class SubPromise extends Promise {};

what kind of promise do you expect to be the value of sp below:

let forceSubPromise = SubPromise.resolve;  //resolve inherited from Promise
let sp = forceSubPromise(getAThenable());  //is sp a Promise or a SubPromise or an error?

I assume the desired result is SubPromise. If Promise.resolve has a fallback to Promise you won't get that result. Better an error than silently returning something unexpected.

For that reason, the appropriate way to extract such a method is:

var forceSubPromise = SubPromise.resolve.bind(SubPromise);
//or
var forceSubPromise = Promise.resolve.bind(SubPromise);

Finally, in regard to my original answer above. I was really thinking about the fallbacks in Array.prototype methods when I wrote that answer. The legacy compatibility logic doesn't really apply to Array.from and Array.of which did not exist prior to ES2015. I suspect we were over zealous in giving them explicit fallbacks. Note that the Typed Array version of those methods do not have a fallback and will throw similarly to Promise.resolve when extracted. I wish we had also done that for Array.from and Array.of. Too late now.

@zloirock
Copy link
Author

Thanks for clarification -)

@TiddoLangerak
Copy link

Arguably automatic delegating to subclasses isn't intuitive either: delegating to the subclasses only works when the subclasses have a compatible constructor signature. If the signatures don't match then at best you get a TypeError, and at worst you get unexpected behaviour. For example, if I extend Promise to wrap node-style async functions then calling .resolve function on the subclass will throw:

class NodePromise extends Promise {
    constructor(nodeFn, ...args) {
        super((resolve, reject) => {
            nodeFn(...args, (error, result) => error ? reject(error) : resolve(result));
        });
    }
}
//Intended usage:
const file = await new NodePromise(fs.readFile, myFile);

//Calling statics throw:
NodePromise.resolve(3); //throws TypeError (reject function isn't callable)

And if I extend Promise to provide a bridge with Rx-style subscribers then I get unexpected behaviour:

class RxSubscriberPromise extends Promise {
    constructor(subscribeFunc) {
        super((resolve, reject) => {
            const results = [];
            subscribeFunc(item => results.push(item), err => reject(err), () => resolve(results));
        });
    }
}
//Intended usage:
const myRange = Rx.range(1, 3);
const results = await new RxSubscriberPromise(myRange.subscribe.bind(myRange)); 

//This works accidentally:
RxSubscriberPromise.reject(new Error());
//But this results in a promise that will never resolve:
RxSubscriberPromise.resolve(3);

In the general case I highly doubt that subclass constructors are compatible with their super constructors, neither of my examples can be implemented in such a fashion already.

To get back to @dominic's reason for using this:

A large part of defining subclass-friendly classes is not hard-coding knowledge of specific types into the base classes.

Using this in statics doesn't help with that. It not only hard-codes the knowledge about specific types (constructor signature), it also significantly restricts what a subclass can do. Requiring subclasses to overwrite these statics isn't a solution either, since any new constructor-calling statics on the base class would immediately break these subclasses.

@zloirock
Copy link
Author

@WebReflection

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

No branches or pull requests

7 participants