Skip to content

Promise.resolve is not bound to Promise object #127

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

Closed
dmail opened this issue Nov 3, 2015 · 9 comments
Closed

Promise.resolve is not bound to Promise object #127

dmail opened this issue Nov 3, 2015 · 9 comments
Labels

Comments

@dmail
Copy link

dmail commented Nov 3, 2015

Promise.resolve.call(null);

Running the above line of code throw an error when using core-js.

Using your librabry you get the error : Object is not a function at line 230
Using firefox Promise it returns a promise fullfilled with undefined

I suggest to bind Promise functions (resolve, reject, race, all) to the constructor.

resolve: function resolve(x){
    return isPromise(x) && sameConstructor(x.constructor, this)
      ? x : new this(function(res){ res(x); });
}.bind(PROMISE)
@zloirock
Copy link
Owner

zloirock commented Nov 3, 2015

It's not very convenient, but it's a correct behavior by the spec and FF bug, see step 2 and note. Binding constructor will break subclassing.

@zloirock zloirock closed this as completed Nov 3, 2015
@dmail
Copy link
Author

dmail commented Nov 3, 2015

I just figured this out when I saw the check useNative with firefox subclassing. Thanks you for the fast answer.

@medikoo
Copy link

medikoo commented Nov 4, 2015

Binding constructor will break subclassing.

I think It's not really true. It's just Promise constructor functions that behave that way, e.g. you won't see that problem with Array.from, which you can freely call as Array.from.call(null, []), and Array remains perfectly subclassable.

Still, that's just a side note, as indeed it's how it was specified, but I'm not sure why.

@zloirock
Copy link
Owner

zloirock commented Nov 4, 2015

@medikoo it's really true for a bound, not default constructor, like in Array.from. Why Promise.resolve by the spec does not use the same logic? No ideas, possible because before this last-minute change it had @@species logic. It seems that soon we will have one more review of this logic for Promise methods, see 1, 2, so you can mention current problem there.

@medikoo
Copy link

medikoo commented Nov 5, 2015

it's really true for a bound, not default constructor, like in Array.from

Thing is that, when it's bound to null or undefined it just fallbacks to native, and it's highly expected, thanks to that we can use from in stand alone form as e.g. var from = Array.from; from([]);

Unfortunately it's not the case with Promise.resolve which to me looks as spec issue (it introduced inconsistency to ECMAScript, as it's first ever case where constructor function needs to be called in context of constructor to not fail, so it's a first ever "constructor method").
Do you have any reference to exact place where it was justified? I browsed links you provided but I didn't find it there.

@zloirock
Copy link
Owner

zloirock commented Nov 5, 2015

Do you have any reference to exact place where it was justified?

...

No ideas

It's also interesting for me.

@domenic can you explain?

@stefanpenner
Copy link

This is by design.

It is as such to make inheritance work, this is the inherited constructor for a static method like resolve/reject/race/all etc

Subclass.resolve(Promise.resolve()).constructor === Subclass

@zloirock
Copy link
Owner

zloirock commented Nov 6, 2015

@stefanpenner sure, it's my answer from the first comment, question here - why Promise methods hasn't fallback for calling without context, like Array.from :)

SubArray.from([]).constructor === SubArray
Array.from.call(null, []).constructor === Array

SubPromise.resolve(Promise.resolve()).constructor === SubPromise
Promise.resolve.call(null, Promise.resolve()).constructor // Error

@stefanpenner
Copy link

@zloirock thats what i get for quickly reading and answering on my phone :P Now i feel silly.

You are correct, Array.from appears to handle this more gracefully sec-array.from 6.b.i I suspect this could be added in some future iteration of the spec. Although yes, it would be great to get @domenic's context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants