why do i need a closure in a Promise .then()?

Question

I've been learning about ES6 promises in Node (7.4.0) because I want to apply them to handle serial communications. I made a promise that is an aggregate of a number of smaller promises that allows me to use a sender, an EventListener and a timeout to serialize communication with a device. However, I don't quite understand .then() chaining, because I need to add in some additional closures that look different from what i see in many examples, which leads me to believe I am misunderstanding something fundamental. Consider this function (I removed all the prototype/this. code to make it smaller):

function sendAck(command, ack, timeout) {
  return new Promise((resolve, reject) => {
    if (gReady === undefined) reject('not ready');
    // each of these three functions returns a promise
    let sendPromise = createSend(command);
    let ackPromise = createAck(ack);
    let timeoutPromise = createTimeout(timeout);
    // p1 = we hear a response, or timeout waiting for one
    let p1 = Promise.race([ackPromise, timeoutPromise]);
    // both p1 -and- send function need to resolve
    let p2 = Promise.all([p1, sendPromise]);
    p2.then(values => resolve(values)).catch(err => {
      localCleanup(); /// otherwise just return p2, but need to do this
      reject(err)
    });
  }
}

Now when I try to chain a bunch of sendAck()s with then, I find that this usage fails as they all execute at once:

sendAck('init', 'pass', 3000)
.then(sendAck('enable a', 'pass', 3000))
.then(sendAck('enable b', 'pass', 3000))
:

So I have to wrap each in a closure to make it work, since the closure is evaluated on the then() rather than the function being evaluated by the JS interpreter. Which feels like I'm missing something very important because it looks awkward:

sendAck('init', 'pass', 3000)
.then(() => { return sendAck('enable a', 'pass', 3000) })
.then(() => { return sendAck('enable b', 'pass', 3000) })
:

I'm confusing because I see other examples online where .then() contains a function that returns a promise, like...

.then(onHttpRequest)

Which clearly is different from

.then(onHttpRequest())

It just seems weird to have to chain .then() with closures. Am I doing this correctly and just not used to it, or am I missing something?

Thanks in advance.

PT

EDIT: As discussed below, there are no closures in my issue, just anonymous functions.

Answer 1

That's not a closure

So far I don't see any closure in your example. Yes, I know some programming languages call anonymous functions "closures" but in my humble opinion the developers of those languages just have a misunderstanding of what closures are. Certainly in javascript we don't call anonymous functions closures, we call them "anonymous functions".

It's not a closure issue, it's function semantics.

First, forget about promises. Let's say you have this piece of code:

function a (x) {return x*2}

var b = a(5);

Now, in any programming language, be it Java or C++ or javascript, what would you expect the value of b to be? Do you expect b to be 10 or do you expect it to be function(){return 10}? After executing the code above do you expect to be able to do this:

console.log(b);

or do you think you must do this:

console.log(b());

Obviously you'd say b is 10, not a function that returns 10. All languages work like this right? So let's make that example a bit more complicated:

function a (x) {return x*2}

console.log(a(5));

In the code above, would you expect console.log() to print function a(x){..} or do you expect it to print 10? Obviously it would print 10. Because we know in programming languages when we call a function the result of that call is not the function itself but the return value of the function. Notice that the code above is exactly identical to:

function a (x) {return x*2}
var y = a(5);
console.log(y);

If we wanted to print the function we'd do:

console.log(a);

In a world where you can pass functions around the same way you pass numbers or strings you need to be more aware of the difference between a function and a function call. In the code above, a is a function. And you can pass it around just like you pass around any other object. a() is a function call and the result of that function call is the return value of the function.

Back to promises

Therefore, in your code, when you do:

sendAck('init', 'pass', 3000)
.then(sendAck('enable a', 'pass', 3000))
.then(sendAck('enable b', 'pass', 3000));

It is identical to doing

// Functions below are async, they return immediately without waiting
// for data to be returned but returns promises that can wait for
// data in the future:
var a = sendAck('init', 'pass', 3000);
var b = sendAck('enable a', 'pass', 3000);
var c = sendAck('enable b', 'pass', 3000);

// Now we wait for return data:
a.then(b).then(c);

Note that while the .then() resolve in sequence the sendAck are sent in parallel because we're not waiting for one to return data before calling the next.

The solution, as you've found out, is to not call sendAck until we get data. So we need to do this:

// We're not calling `sendAck` here, we're just declaring functions
// so nothing gets sent:
function a () {return sendAck('init', 'pass', 3000)}
function b () {return sendAck('enable a', 'pass', 3000)}
function c () {return sendAck('enable b', 'pass', 3000)}

// Now we can fire them in sequence:
a().then(b).then(c);

Notice that we get the ball rolling by calling a() but we don't actually call b or c - we let then do it for us. Because we're passing the functions themselves instead of calling them their bodies don't get executed.

Of course, we don't need to create named functions. As you've found out yourself we can easily use anonymous functions for this. So the above can be rewritten as:

sendAck('init', 'pass', 3000)
.then(() => { return sendAck('enable a', 'pass', 3000) })
.then(() => { return sendAck('enable b', 'pass', 3000) });

Answer 2

.then(sendAck('enable a', 'pass', 3000)) executes the sendAck function in place, synchronously, starting that promise, and passes the result of that to .then() and .then() will try to call that result later when the promise chain continues, but you've already called it, so it won't have waited for the async promise chain.

You can also shorten it to

.then(() => sendAck('enable a', 'pass', 3000))

Or you could pass in a bound function reference, which the promise will call later:

.then(sendAck.bind(null, 'enable a', 'pass', 3000))

Answer 3

When you pass in your function to the .then(..) methods, you are actually telling js to evaluate (or call) the function because you have used the parenthesis and added your parameters:

sendAck('enable a', 'pass', 3000); // runs immediately, then passes value to .then(..)

This will be evaluated before it tries to pass anything into .then(..)

If you don't want to use arrow functions, you would usually write something like

.then(function () {
    sendAck('enable a', 'pass', 3000);
});

which is effectively the same, but will have a different scope than the arrow function

Answer 4

If you make the sendAck function into a higher-order function, then you can use .then as you did originally:

const sendAck = (command, ack, timeout) => () => {
  return new Promise((resolve, reject) => {
    if (gReady === undefined) reject('not ready');
    // each of these three functions returns a promise
    let sendPromise = createSend(command);
    let ackPromise = createAck(ack);
    let timeoutPromise = createTimeout(timeout);
    // p1 = we hear a response, or timeout waiting for one
    let p1 = Promise.race([ackPromise, timeoutPromise]);
    // both p1 -and- send function need to resolve
    let p2 = Promise.all(p1, sendPromise);
    p2.then(values => resolve(values)).catch(err => {
      localCleanup(); /// otherwise just return p2, but need to do this
      reject(err)
    });
  };
};

sendAck('init', 'pass', 3000)()
.then(sendAck('enable a', 'pass', 3000))
.then(sendAck('enable b', 'pass', 3000));

Alternatively you can use the shorter syntax from Andy Ray's answer. I don't think there is any problem with your approach/usage here though.

You can also shorten

p2.then(values => resolve(values)).catch(err => {
    localCleanup(); /// otherwise just return p2, but need to do this
    reject(err)
});

to:

p2.then(resolve).catch(err => {
    localCleanup();
    reject(err);
});