NewPromiseCapability & GetCapabilitiesExecutor Functions

[raulsebastianmihaila]

The way GetCapabilitiesExecutor is used in NewPromiseCapability doesn't make sense to me.

First, there are some resolve and reject functions that are set as properties of the [[Capability]] slot of the executor that results from the operation. However, these resolve and reject functions are not passed as arguments in NewPromiseCapability. And, even if they were, in step 5 of NewPromiseCapability, the value of the [[Capability]] slot of the executor is replaced with the promise capability that was created in NewPromiseCapability, whose resolve and reject functions are undefined.

On the readability side, I think it would be an improvement to make GetCapabilitiesExecutor look more like other abstract operations: GetCapabilitiesExecutor(resolve, reject). And make it return F (the executor) instead of undefined. And explicitly pass resolve and reject from NewPormiseCapability.

[thefourtheye]

Closely related to #355

[domenic]

And as before, people just seem confused. GetCapabilitiesExecutor is not an abstract operation, and having it return something would be pointless, since nobody cares about its return value. The entire point of NewPromiseCapability is to turn a constructor C into a record of { {[Promise]], [[Resolve]], [[Reject]] }, i.e. a PromiseCapability. So having it take resolve and reject arguments would defeat its purpose. And the reason for setting the [[Capabilty]] slot of the executor is so that the executor can set its [[Capability]]'s [[Resolve]] and [[Reject]], so of course they start out undefined.

[thefourtheye]

@domenic I believe that, it should be explained better in the spec. Every time I go through the spec I get confused at that point and looks like I am not the only one.

[raulsebastianmihaila]

I see now. I think the most confusing part is that the... section... is called GetCapabilitiesExecutor Functions. My mind was telling me that it was a getter that was supposed to create an executor. But it's not. The executor is never created, it just is.

[domenic]

It's created in step 4 of NewPromiseCapability.

[raulsebastianmihaila]

Yep, but not in GetCapabilitiesExecutor, as the name suggests.

[domenic]

Maybe it would be clearer if you thought of it as "ExecutorThatGetsCapabilities"

[raulsebastianmihaila]

👍 Yep, it would be even clearer if it was named that way. :)

[bterlson]

I think it's fine as-is naming wise but would accept a PR for a informative note regarding the purpose of this function.

[rauschma]

A class (or object) diagram would also be helpful, to get an overview of the entities that are involved in this part of the spec (PromiseReaction Record, PromiseCapability Record, Promise Reject Function, Promise Resolve Function, CapabilitiesExecutor Function, PromiseReactionJob, PromiseResolveThenableJob, Promise).