1

I'm working with JavaScript promises, but somewhat new to them and having some issues. In the code below "todoService.addTodo(description);" is an asynchronous call to a server. However, the code does not wait for "todoService.addTodo(description);" to finish, and instead continues executing the code, which means "test" is always equal to null. I'm using promises because I thought this would help deal with the asynch calls, do I'm I simply not understanding the concept, or just doing something wrong syntactically?

let promise = new Promise(function (resolve, reject){
  let test = todoService.addTodo(description);
  console.log(test)

  if (test) {
     console.log("GOOD");
     resolve("OK");
  } else {
      console.log("BAD");
      reject("Unable to connect to server");
  }
});

promise.then(function(result) {
       console.log(result);
    }, function(err) {
        alert(err);
    });

Here is the implementation of addTodo():

addTodo: function (description) {
        serv.todoUrl ='http://localhost:8080/add?description=' + description;
        serv.todoResource = $resource(serv.todoUrl);
        serv.todoResource.save().$promise.then(function (data) {
            console.log(data);
            let toReturn = {result: data.result, details: data.details};
            console.log(toReturn);
            return toReturn;
        }, function (error) {
            return null;
        });
Joe Smith
  • 59
  • 1
  • 6
  • 1
    Unless todoService.addTodo(description) returns a promise there is no way to stop execution on it. you should be using the promise from within todoService.addTodo(description) – Dov Benyomin Sohacheski Jun 09 '18 at 19:16
  • Is `toolService.addToDo` marked as `async`? Or is it a promise? In the first case, marking your inner function as async and `await` the todo service is the way to go, in the other, returning todoService.addTodo(description) with a chain of then would be the way to go – Icepickle Jun 09 '18 at 19:17
  • You're not understanding the concept. I'd recommend reading up on it first, see if you can figure out the problem yourself once you get a clearer picture. MDN is usually a good place to start: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Using_promises -- If you're still having trouble afterward, we can go from there. – Máté Safranka Jun 09 '18 at 19:20
  • @DovBenyominSohacheski I added my implementation of addTodo so you can see it. It itself actually implements a promise. – Joe Smith Jun 09 '18 at 19:21
  • Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it), and just `return` the promise chain from `addTodo`! – Bergi Jun 09 '18 at 20:08

1 Answers1

4

If test is a promise, then if (test) is not the right way to deal with it. What's more, you should not create a new Promise when addTodo already provides the promise.

All you would need is this:

todoService.addTodo(description).then(function(result) {
    console.log(result);
}, function(err) {
    alert(err);
});

Now that you also added the implementation of addToDo in your question, it turns out that it lacks a return of the promise you create there, so it just returned undefined, which obviously is not a promise. Add return here:

addTodo: function (description) {
    serv.todoUrl ='http://localhost:8080/add?description=' + description;
    serv.todoResource = $resource(serv.todoUrl);
    return serv.todoResource.save().$promise.then(function (data) {
//  ^^^^^
        console.log(data);
        let toReturn = {result: data.result, details: data.details};
        console.log(toReturn);
        return toReturn;
    }, function (error) {
        return null;
    });

Note that the other return statements are in the relative callback functions which execute asynchronously, so they don't provide the return value for addToDo, but they provide the resolving value for the promise.

NB: Since you treat the rejection case in addToDo and don't cascade the error, but just return null, addToDo will not represent a rejected promise in that case, but a fulfilled one. If you prefer to have the caller of addToDo to get a rejected promise in that case, then just remove the rejection handler function from within addToDo.

trincot
  • 317,000
  • 35
  • 244
  • 286
  • Thanks so much for the help. What you are saying makes sense, but I'm still a bit confused. From what I understand, in a promise you have to specify a resolve() and reject(). If "serv.todoResource.save().$promise" is a promise object, then does that mean it's already implemented resolve() and reject()? And if this is the case, shouldn't I just be returning "serv.todoResource.save().$promise" not serv.todoResource.save().$promise.then(function){...};? – Joe Smith Jun 09 '18 at 19:41
  • First: yes `save().$promise` returns a promise object, so no need for doing a `resolve` or `reject`: you only need that when you need to synthesise a promise from the ground up. Whether you return `$promise` as is, or add `then` that is up to you. Apparently you wanted something to be done after that `$promise` resolved, and so you added a `then` to do that. But that `then` also returns a promise, so you are still OK with that. This is the power of chaining promises: you keep getting a promise back from a `then`-chain. – trincot Jun 09 '18 at 20:06