Confused about JS promises

[Captain]

I am writing a function that breaks up a complex view model and saves by making async $.posts. Because of how the values are structured, some of the save operations depend on previous operations. Since AJAX is async, I asked here previously and was told to try some frameworks for Promises.

OK. I'm not quite there yet. I'm still working on getting my head around promises. I think they sort of define an either monad, where then is roughly equivalent to Haskell's >>=. But that doesn't match up with the behavior I'm seeing.

My (cleaned up) code is:

function (item) {
  var personJson = ko.toJSON(item.person);
  new Promise(function (res, rej) {
  
    $.post('/api/people/' + item.person.id(), personJson, function(resp) {
      item.person = ko.mapping.fromJS(resp);
      res();
    });
  }
}).then(function() {
  // note this line depends on the first save happening, since we get the id
  // from the database
  item.client.personId(item.person.id());
  var clientJson = ko.toJSON(item.client);
  $.post('/api/clients', clientJson, function (resp) {
    item.client = ko.mapping.fromJS(resp);
  }
}).then(function() {
  // again, depends on the client getting saved
  item.demographics.personId(item.client.id());
  $.post(...);

The client post is working, but apparently the demographics post is getting run first, because the demographics.clientId is getting passed to the server as a null value.

Am I just misunderstanding what then does? Is it enough to wrap $.posts in a then to force the order? Or do I need to make a new promise for each post? And if that's the case, what does that look like? Something like:

.then(function() {
  return new Promise( ... )
}) 

[LB_]

@captain said in Confused about JS promises:

I'm still working on getting my head around promises

Basically, there's two ends: the code that calculates the result has a promise object they must fulfill, and the code that wants the result has a future object that they can query for the status. The promise and the future are two sides of the same coin and you can't really have one without the other - setting the result in the promise makes it available in the future in an async threadsafe way. I'm not sure how that works in JavaScript though.

From what I understand, .then is just a way to avoid continually nesting lambdas in callback parameters - e.g. like this.

Disclaimer: I know very little JavaScript and only learned about promises and futures a few months ago.

[ben_lubar]

Does async function(){} throw a syntax error or does your JS interpreter allow it?

Because in an async function, you can just use the await operator.

[justhelpingout]

You are not fulfilling the contract of >>= (which, as you so rightly point out, Promise mostly fulfills). Your second function (item.client) is not a -> m b but a -> void. Since this is JS, we just go on to the next >>= call the minute item.client completes (because, right). Change the type of your second function and your third will follow.

// Assuming that $.post is a recent version of jQuery it also returns promises
// as long as you don't supply the callback parameter
.then(() => {
  item.client.personId(...);
  // Returning a new promise here properly types this function as `a -> m b`
  return $.post('/api/clients', clientJson)
    .then(resp => item.client = ko.mapping.fromJS(resp));
})

Of course, as @ben_lubar points out, you can also just use async / await:

const fromJS = ko.mapping.fromJS;
const toJSON = ko.toJSON;
async function storeItem(item) {
  item.person = fromJS(
    await $.post('/api/people/' + item.person.id(), toJSON(item.person))
  );
  item.client.personId(item.person.id());
  item.client = fromJS(
    await $.post('/api/clients/', toJSON(item.client))
  );
  item.demographics.personId(item.client.id());
  item.demographics = fromJS(
    await $.post(...)
  );
}

[Zecc]

What @justhelpingout said.

Don't forget to return a promise on the first block of code too.
So either just add a return before new Promise( ...) { on the third line; or convert to the more fluent:

function (item) {
  var personJson = ko.toJSON(item.person);
  return $.post('/api/people/' + item.person.id(), personJson)
    .then( function(resp) {
      item.person = ko.mapping.fromJS(resp);
    });
  }
}

Forgetting to return your promises is a popular mistake that can affect anyone.

[cartman82]

For what you are trying to do, you don't need promises at all. Since your ajax library is callback based, just use callbacks.

function (item) {
  var personJson = ko.toJSON(item.person);
  
  return $.post('/api/people/' + item.person.id(), personJson, function(resp) {
    item.person = ko.mapping.fromJS(resp);
    item.client.personId(item.person.id());
    var clientJson = ko.toJSON(item.client);
    
    return $.post('/api/clients', clientJson, function (resp) {
      item.client = ko.mapping.fromJS(resp);
      item.demographics.personId(item.client.id());
    });
  });
};

To fully utilize promises, you really want to wrap that ajax library so that it itself returns promise (it could already support that, check the docs). Something like this:

function ajaxPost(endpoint, body) {
  return new Promise((res, rej) => {
    $.post(endpoint, body, function(resp) {
      res(resp);
      // Don't forget error handling in real code
    });
  });
}

function (item) {
  var personJson = ko.toJSON(item.person);
  return ajaxPost('/api/people/' + item.person.id(), personJson)
    .then(function (resp) {
      item.person = ko.mapping.fromJS(resp);
      item.client.personId(item.person.id());
      var clientJson = ko.toJSON(item.client);

      // Notice you must return a promise here, to continue the chain
      return ajaxPost('/api/clients', clientJson);
    })
    .then(function (resp) {
      item.client = ko.mapping.fromJS(resp);
      item.demographics.personId(item.client.id());

      // ...
    });
};

[Captain]

Thanks for your help everyone. I sort of get it.

OK, another couple of quick questions. I am using a recent version of jQuery. How do I interleave logic?

For example, I only want to do $.posts when some condition is satisfied. Can I just attach a .then to an if block, like:

if (item.person.id()) {
  return $.post(...);
} else {
  return $.post(...);
}.then(...)

Or do I need some other kind of "container" around the logic?

Edit: looking at a BNF grammar for JS, it looks like I do need some kind of container around the block. Suck.

Also, .then is a lot like Haskell's >>=. Is there an equivalent to Haskell's return or pure (i.e., a function with type a -> Promise a that builds a minimal promise based on a given value)?

Edit: this is my attempt. Is it sane?

pure = function(obj) {
  return new Promise(function (res, rej) {
    res(obj);
  });
}

Edit: this stuff would be so easy if it was an actual monad with >>=, join, and pure.

[Zecc]

@captain You can do:

(
  item.person.id() ?
  $.post(...) :
  $.post(...)
).then(...)

Or maybe formatted like this instead:

(
  item.person.id()
  ? $.post(...) :
  : $.post(...)
).then(...)

It doesn't look very idiomatic to me though. Generally you'd go:

let nextStep;

if (item.person.id()) {
  nextStep = $.post(...);
} else {
  nextStep = $.post(...);
}

nextStep.then(...)

Thought I'll admit that second code block looks cool.

@captain said in Confused about JS promises:

a function with type a -> Promise a that builds a minimal promise

Promise.resolve(a); // returns a promise that will resolve to `a` come next tick around the event loop
Promise.reject(e);  // returns a promise that will fail with error `e` come next tick around the event loop

While you're at it, check out the other methods of Promise, all and race. They do what you'd expect.

[ben_lubar]

@zecc said in Confused about JS promises:

next tick around the event loop

Why isn't that "immediately"? Why would those functions make an async work item?

[anotherusername]

@captain Does that code even run? Your parentheses/braces aren't matched up properly. I can't really figure out how the flow of the code is supposed to go because I can't parse it. Based on the indentation, your function (item) { appears to end with }).then(..., but since it doesn't return a promise, that couldn't work even if the braces and parentheses were matched correctly. Was everything else actually supposed to be inside that function?

edit: also, any particular reason why you're using function (res, rej) { ... } instead of (res, rej) => { ... }?

[anotherusername]

I think if you remove line 1 and 9 and then fix up the indentation, it would be what you meant to post. And yeah, you have an issue there: you can chain promises with .then, but unless you return another promise, every .then in the chain is going to depend on the original promise resolving or rejecting. Which means that all of them will immediately run as soon as the original promise resolves or rejects. But if you return a new promise, then subsequent .thens will resolve or reject when that promise resolves or rejects.

new Promise((resolve, reject) => {
  setTimeout(() => {
    console.log('Resolving first promise...');
    resolve();
  }, 1000);
}).then(() => {
  console.log('First .then');
  new Promise((resolve, reject) => {
    setTimeout(() => {
      console.log('Resolving second promise...');
      resolve();
    }, 1000);
  });
}).then(() => {
  console.log('Second .then');
});

Even though I created a second promise, I didn't do anything with it; the second .then is still chaining off the original promise, so if you run this, you get:

Resolving first promise...
First .then
Second .then
Resolving second promise...

Now, if I actually return the second promise, then subsequent .thens chain off that new promise, and the output is what you probably wanted:

Resolving first promise...
First .then
Resolving second promise...
Second .then

[Zecc]

@ben_lubar said in Confused about JS promises:

@zecc said in Confused about JS promises:

next tick around the event loop

Why isn't that "immediately"? Why would those functions make an async work item?

Reasons.

The expectation with promises is that the callbacks you've provided will only run in the future, after you're done with the current execution context.

For one thing, you want to give surrounding code a chance to set up their own listeners on the promise you've just created before it resolves. You might also want to give the external environment (eg DOM) a chance to catch up to any changes you've just made.

But the truth is: I lied.
I'm not sure if it actually waits until the next full run of the event loop.
It might just run immediately after you've emptied the current call stack, but before new events are processed.

I think it's best not to rely on these implementation details in any case, and just think of it like "it will resolve some time in the future".

[ben_lubar]

@zecc is there a guarantee that this will print foo and then bar and not the other way around?

Promise.resolve(null).then(function() { console.log("bar"); });
console.log("foo");

How about this one?

var p1 = Promise.resolve(null);
Promise.resolve(null).then(function() {
    p1.then(function() { console.log("bar"); });
    console.log("foo");
});

[Zecc]

@ben_lubar Yes and yes.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then#Return_value says:

Return value

A Promise in the pending status. The handler function (onFulfilled or onRejected) then gets called asynchronously (as soon as the stack is empty).

https://promisesaplus.com/#point-34 says:

onFulfilled or onRejected must not be called until the execution context stack contains only platform code.

Edit: no such guarantee if you have:

var p1 = Promise.resolve(null);
Promise.resolve(null).then(function() {
    p1.then(function() { console.log("foo"); });
    p1.then(function() { console.log("bar"); });
});

Just like you have no guarantee with:

var p1 = Promise.resolve(null);
p1.then(function() { console.log("foo"); });
p1.then(function() { console.log("bar"); });

[Captain]

OK, super confused now.

I got sort of frustrated dealing with all the syntax and plumbing with Promises, so I gave the async/await solution a try. This is my real code, with spacing adjusted:

, saveItem: async function(item) {

    if (item.person.id()) {
      item.person = ko.mapping.fromJS(
        await $.post('/api/people/' + item.person.id(), ko.toJSON(item.person))
      );
    } else {
      item.person = ko.mapping.fromJS(
        await $.post('/api/people', ko.toJSON(item.person))
      );
    }

    item.address.personId(item.person.id());
    item.address = ko.mapping.fromJS(
      await $.post('/api/addresses', ko.toJSON(item.address))
    );

    item.client.personId(item.person.id());
    if (item.client.id()) {
      item.client = ko.mapping.fromJS(
         await $.post('/api/clients/' + item.client.id(), ko.toJSON(item.client))
      );
    } else {
      item.client = ko.mapping.fromJS(
        await $.post('/api/clients', ko.toJSON(item.client))
      );
    }

    item.itsClient.clientId(item.client.id());
    item.itsClient = ko.mapping.fromJS(
      await $.post('/api/itsClients', ko.toJSON(item.itsClient))
    );

    item.demographics.clientId(item.client.id());
    item.demographics = ko.mapping.fromJS(
      await $.post('/api/demographics', ko.toJSON(item.demographics))
    );
  }

Unfortunately, Chrome is telling me that:

Uncaught (in promise) TypeError: item.person.id is not a function
    at Object.saveItem

I ask Chrome what it's talking about and it says the error happens "here", where I try to set the address's personId. OK. But then I look at the network tool and it tells me the $.post to /api/addresses WORKED. The headers actually show the correct personId. In fact, all the $.posts worked

And when I inspect the item.person after the whole saveItem run, it gives me THIS garbage:

[]

[ben_lubar]

@captain Do you have anything that modifies item after saveItem returns? Because the function will return a Promise object immediately after the first if statement's condition is computed and ko.toJSON is called for the first time.

[Captain]

@ben_lubar I'm going to take a closer look now, but I DON'T think my code is modifying item. item is a knockout view model, though, so presumably filling in the form and stuff would modify it. I don't know if Knockout is doing it behind my back in its cycle.

[anotherusername]

@captain So the line that's giving you the error is this?

item.address.personId(item.person.id());

Try logging stuff right before that line to make sure that item.person got set as you thought it should.

Also remember like ben said, for all intents and purposes saveItem will return immediately, so if you're fucking around with the item argument before it finishes its asynchronous run, you could break it and/or not see its contents yet. Since saveItem is async, it returns a promise, and as soon as it finishes (once execution gets to the closing } in the function), the promise resolves, which means you can chain off it:

saveItem(item).then(() => {
  // code HERE will run after saveItem finishes
});

If you're used to writing stuff like

saveItem(item);
// code here, assuming saveItem finished

you're going to run into problems, because saveItem is async, so it hasn't finished. The await syntax is a lot more similar to what you're used to, so it may actually be more natural to write:

await saveItem(item); // saveItem is an async function
// code here will run after saveItem finishes

...but just remember, you can only use await inside of an async function. If you can't put the code into an asynchronous function, then you're going to have to use .then to provide a callback for when it resolves.

[Captain]

@anotherusername said in Confused about JS promises:

@captain So the line that's giving you the error is this?
item.address.personId(item.person.id());

Try logging stuff right before that line to make sure that item.person got set as you thought it should.

Yes, that's the line. I confirmed that none of my code is changing item after invoking saveItem. saveItem is bound to a Knockout click binding, like:

<button data-bind="click: saveItem">Save

Apparently this is a Heisenbug. I put in the console.log(item.person) line and got no error.

I took the the logging line out and got no error.

These are the ONLY changes I've made since I documented the bug behavior yesterday.

[Captain]

FML! Heisenbug is back.

[DCoder]

@captain said in Confused about JS promises:

item.address.personId(item.person.id());
item.address = ko.mapping.fromJS(
  await $.post('/api/addresses', ko.toJSON(item.address))
);

That's not right. Your item.address was already an object with an observable property personId, you updated that property, and then you replaced the whole object with a brand new one. Result:

1. item.address will now be a plain JS Object, even if you previously created it as an instance of some specific class. It will only have the properties sent in the API response. If personId is not part of that response, mapping will not create that property on the new object.
2. Because item.address is not an observable itself, the fact that you changed it will not trigger any updates or dependencies.
   Any observables or subscriptions you may have depending on item.address.personId will refresh after the first line, but not after the second line replaces the object, or ever again (unless they get recreated by a change higher up the chain). This will be hell to debug, or you will have a lot of code papering over that and triggering explicit updates.

You want to update the existing viewmodel object in-place instead:

    item.address.personId(item.person.id());
    ko.mapping.fromJS(
      await $.post('/api/addresses', ko.toJSON(item.address)),
      item.address
    );

This is one of the nastier traps in Knockout – recreating viewmodels all the time can cause chaos when some DOM fragments don't update (because they are observing a property of an object that is no longer part of the viewmodel you're changing), and making whole sub-objects observable themselves can cause excessive DOM repaints and destroy state like scroll position or selected text on the page. You should try to maintain the same viewmodel object with the same sub-objects, only updating their observable properties.

Yes, that is completely contrary to the "don't mutate your current state, make and modify a copy of it" idea promoted by React and friends.

---

@captain said in Confused about JS promises:

Chrome is telling me that:

Uncaught (in promise) TypeError: item.person.id is not a function
    at Object.saveItem

I ask Chrome what it's talking about and it says the error happens "here", where I try to set the address's personId. But then I look at the network tool and it tells me the $.post to /api/addresses WORKED.

/api/addresses is not relevant here, since the error happens before that endpoint is called. The problem is that item.person is no longer what you think it is, because it got transformed by the item.person = ko.mapping.fromJS(...) a few lines earlier. See my comment above – you should be updating the contents of item.person, not replacing item.person outright.

---

@captain said in Confused about JS promises:

I look at the network tool and it tells me the $.post to /api/addresses WORKED. The headers actually show the correct personId. In fact, all the $.posts worked
And when I inspect the item.person after the whole saveItem run, it gives me THIS garbage:
[]

You are interested in the response state, not the headers. Is the returned data really what you expected? Is it really a single object, not an array of objects? Sanity check the response before you blindly update your viewmodels. What if the data you sent was not acceptable – would you still see 200 OK and the object in the response?

(In my opinion, API calls that create/update data should only return an identifier for that data, and the data itself should be loaded through a separate GET call afterwards. And I would use GUIDs for identifiers, so they can be generated by the client beforehand without duplicates… But all that is not the point of this topic.)

[cartman82]

@captain said in Confused about JS promises:

I got sort of frustrated dealing with all the syntax and plumbing with Promises, so I gave the async/await solution a try.

In an unrelated note, be mindful of browser compatibility. Since it doesn't look like you are using a compiler, async/await will only work on latest evergreens, no internet explorer at all. Even native Promises are a bit iffy, check the compatibility on MDN and add a polyfill if needed.