domenic / promises-unwrapping

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

Throw Error if constructor passed to GetDeferred doesn't pass functions? #50

Closed Nathan-Wall closed 10 years ago

Nathan-Wall commented 10 years ago

From GetDeferred:

  1. If IsConstructor(C),
    1. Let resolver be an ECMAScript function that:
      1. Lets resolve be the value resolver is passed as its first argument.
      2. Lets reject be the value resolver is passed as its second argument.
    2. Let promise be C.[[Construct]]((resolver)).

The first two arguments passed to resolver are expected to be functions, however no validation is done. Should there be an error thrown if resolve or reject aren't functions? Another option would be to fall back to use the default Promise constructor if they're not functions.

domenic commented 10 years ago

Hmm. I think the correct thing to do is a just-in-time rejection. E.g. this:

1.Let deferred be GetDeferred(C).

  1. Call deferred.[[Resolve]].[[Call]](undefined, (x)).
  2. Return deferred.[[Promise]].

Should become

  1. Let deferred be GetDeferred(C).
  2. If IsCallable(deferred.[[Resolve]]) is false, return a TypeError rejection.
  3. Call the [[Call]] internal method of deferred.[[Resolve]] with undefined as the thisArgument and a new List containing x as argumentsList.
  4. Return deferred.[[Promise]].

Here I introduce a new shorthand, "return a TypeError rejection," in a similar fashion to the ES spec's "throw a TypeError" shorthand.

This is getting complicated...

Nathan-Wall commented 10 years ago

I think that sounds like a good idea. Why not put the check inside GetDeferred, though, so that it doesn't have to be duplicated everywhere its used? Would it make sense for the [[Promise]] returned by GetDeferred to be a TypeError rejection if C was a constructor which didn't pass in two functions?

It sort of begs the question, though, why it would still return a non-rejected Promise if the C passed to GetDeferred is not a constructor. Why not return a TypeError rejection if C is a string, for instance? A string would kind of be similar to a function which didn't pass in resolve and reject functions... it's just the wrong kind of value. I think it's understandable to fall back to the built-in Promise constructor when C is undefined, but I don't know about other values.

domenic commented 10 years ago

In general ECMAScript tries to delay errors until the last minute. So e.g. Promise.resolve should still be able to work even if the promise subclass passes a non-callable for the reject parameter (i.e. the second parameter to the resolver).

But yes, for the more general case of C not being a constructor at all, I now throw in GetDeferred.

Thanks for finding this; tricky stuff.