1

I have a list of items being stored in an array. For each item in the array, I need to make an API request and add some of the data that comes back into another array so that I can perform operations on it later.

I think the issue is that my get request is asynchronous, and as a result the data is not necessarily loaded when I am trying to add it to the array, but I thought that's what .then was supposed to cover.

var cardRequestList = ["Scapeshift","Ghostform", "Daybreak Chaplain"]

var url = "https://api.magicthegathering.io/v1/cards?name=%22";

var cardInfo =[];

for (var cardName in cardRequestList){
  var results = getCard(cardRequestList[cardName]);
  cardInfo.push(results);
}

console.log(cardInfo); // results should come back here

function getCard(cardName){
    var cardUrl = url.concat(cardName,"%22");
    $.get(cardUrl).then(
    function(data) {
      var temp = [data.cards[0].name,data.cards[0].printings]
      return temp;
    }, function() {
      alert( "$.get failed!" );
    }
  );
}
convoke
  • 191
  • 11
  • 1
    Your `.then()` is going to cover it for a single request, but in order to wait for multiply ones, you need `Promise.all()`. If you put a `return` in front of your `$.get(...` you can replace your for loop with `Promise.all(cardRequestList.map(getCard)).then(results => ...);` –  Jul 01 '19 at 16:32
  • 1
    @Taplar I re-opened this as the common dupe target isn't quite what the OP was asking. They are already handling a single request correctly, their goal is to merge the data from multiple requests. – Rory McCrossan Jul 01 '19 at 16:35
  • 1
    @RoryMcCrossan are you sure? The getCard method doesn't return anything. – Taplar Jul 01 '19 at 16:37
  • 1
    It doesn't work, but the OP is attempting to: `return temp` and then `push()` that in to an array – Rory McCrossan Jul 01 '19 at 16:37
  • 1
    I know, which was why I marked it as a duplicate. The OP doesn't understand the async nature he's trying to perform – Taplar Jul 01 '19 at 16:38
  • Thanks everyone for the comments. @Chris G, I had to muddle with it a bit, but I think your `Promise.all` solution is going to work. Thanks! – convoke Jul 01 '19 at 22:44

1 Answers1

1

then() only works for the specific request it's invoked on.

To solve the issue you have you could make your requests in a loop, adding the jqXHR object returned from those calls to an array which you can then apply to $.when() to execute some other logic once all the requests have completed.

Within the requests themselves you need to add the returned data to an array as you cannot return anything from an async call.

With all that said your code would look something like this:

var cardRequestList = ["Scapeshift", "Ghostform", "Daybreak Chaplain"]
var url = "https://api.magicthegathering.io/v1/cards?name=%22";
var cardInfo = [];

var requests = cardRequestList.map(function(cardName) {
  return getCard(cardName);
});

function getCard(cardName) {
  var cardUrl = url.concat(cardName, "%22");
  return $.get(cardUrl).then(function(data) {
    cardInfo.push([data.cards[0].name, data.cards[0].printings]);
  }, function() {
    alert("$.get failed!");
  });
}

$.when.apply($, requests).done(function() {
  // all requests finished and cardInfo has been populated, place logic here...
  console.log(cardInfo);
});
Rory McCrossan
  • 331,213
  • 40
  • 305
  • 339
  • Hi Rory. Thanks for your answer, but I am having some issues with your code. I think the stuff you did with `.map` had some side effects. `return getCard(cardRequestList[cardName]);` was coming back as undefined. At this point, I think `cardName` is a string. I changed it to `getCard(cardName)`, and that seemed to solve the problem. However, after fixing that, I noticed something odd. The array `cardInfo` seems to return with 6 elements (instead of the three I would expect), and the first three are undefined. This is better than what I had initially, but there's still some weirdness. – convoke Jul 01 '19 at 21:29
  • Here is the link to the fiddle I've been working on: https://jsfiddle.net/convoke/0h32nvks/15/ Once again, thank you so much for your help. – convoke Jul 01 '19 at 21:30
  • 1
    Sorry, my mistakes. I've updated the answer for you – Rory McCrossan Jul 01 '19 at 21:44
  • Thanks again for your help. I still think the line `return getCard(cardRequestList[cardName]);` needs to be changed to `return getCard(cardName);`. But aside from that, I'm still having issues. In your `$.when.apply..` function, there is a `console.log(cardInfo)`. When I run this code with the console open, it IMMEDIATELY returns `[]`. When I expand it, it contains the correct data, but there is a message that says `value below was evaluated just now`. If I change it to `alert(cardInfo);`, the alert comes back empty. It seems that `cardInfo` is still not populated when called by that function. – convoke Jul 01 '19 at 22:23
  • I have updated the fiddle a bit: https://jsfiddle.net/convoke/0h32nvks/34/ – convoke Jul 01 '19 at 22:24
  • You're right that was another mistake - that's what I get for not testing my code properly. I've edited it again for you and validated it works in the fiddle: https://jsfiddle.net/17zysd4t/ – Rory McCrossan Jul 02 '19 at 07:44