I have the following functions with promises:

const ajaxRequest = (url) => {
  return new Promise(function(resolve, reject) {
    axios.get(url)
      .then((response) => {
        //console.log(response);
        resolve(response);
      })
      .catch((error) => {
        //console.log(error);
        reject();
      });
  });
}


const xmlParser = (xml) => {
  let { data } = xml;
  return new Promise(function(resolve, reject) {
    let parser = new DOMParser();
    let xmlDoc = parser.parseFromString(data,"text/xml");

    if (xmlDoc.getElementsByTagName("AdTitle").length > 0) {
      let string = xmlDoc.getElementsByTagName("AdTitle")[0].childNodes[0].nodeValue;
      resolve(string);
    } else {
      reject();
    }
  });
}

I'm trying to apply those functions for each object in array of JSON:

const array = [{"id": 1, "url": "www.link1"}, {"id": 1, "url": "www.link2"}]

I came up with the following solution:

function example() {
    _.forEach(array, function(value) {
        ajaxRequest(value.url)
            .then(response => {
                xmlParser(response)
            .catch(err => {
                console.log(err);
            });
        });
    }
 }

I was wondering if this solution is acceptable regarding 2 things:

1. Is it a good practice to apply forEach() on promises in the following matter.

2. Are there any better ways to pass previous promise results as parameter in then() chain? (I'm passing response param).

I have the following functions with promises:

const ajaxRequest = (url) => {
  return new Promise(function(resolve, reject) {
    axios.get(url)
      .then((response) => {
        //console.log(response);
        resolve(response);
      })
      .catch((error) => {
        //console.log(error);
        reject();
      });
  });
}


const xmlParser = (xml) => {
  let { data } = xml;
  return new Promise(function(resolve, reject) {
    let parser = new DOMParser();
    let xmlDoc = parser.parseFromString(data,"text/xml");

    if (xmlDoc.getElementsByTagName("AdTitle").length > 0) {
      let string = xmlDoc.getElementsByTagName("AdTitle")[0].childNodes[0].nodeValue;
      resolve(string);
    } else {
      reject();
    }
  });
}

I'm trying to apply those functions for each object in array of JSON:

const array = [{"id": 1, "url": "www.link1."}, {"id": 1, "url": "www.link2."}]

I came up with the following solution:

function example() {
    _.forEach(array, function(value) {
        ajaxRequest(value.url)
            .then(response => {
                xmlParser(response)
            .catch(err => {
                console.log(err);
            });
        });
    }
 }

I was wondering if this solution is acceptable regarding 2 things:

1. Is it a good practice to apply forEach() on promises in the following matter.

2. Are there any better ways to pass previous promise results as parameter in then() chain? (I'm passing response param).

• javascript
• scope
• ecmascript-6
• promise
• es6-promise

• I think it's fine to use promises in a loop like you are doing ; it won't bite. not sure if there is a better way in fact – Timothy Groote Commented Jul 25, 2017 at 7:29
• I personally think that there are no particular problem with your code. It looks OK. What I might do is: 1. Use Promise.all() in order to execute all AJAX calls together, and then _.map the results to the xmlParser. But your code looks good, IMHO. BTW, +1 for the nice question. – Catalyst Commented Jul 25, 2017 at 7:29
• You don’t need lodash, simply use for (var i in array) { ... } – Arthur Guiot Commented Jul 25, 2017 at 8:00
• Avoid the Promise constructor antipattern! – Bergi Commented Jul 25, 2017 at 9:42
• Your parenthesis look very mismatched. Can you fix them (and/or the indentation) to show us what you are actually doing? – Bergi Commented Jul 25, 2017 at 9:44

4 Answers 4

You can use .reduce() to access previous Promise.

function example() {
    return array.reduce((promise, value) =>
       // `prev` is either initial `Promise` value or previous `Promise` value
       promise.then(prev => 
         ajaxRequest(value.url).then(response => xmlParser(response))
       )
    , Promise.resolve())
 }
 // though note, no value is passed to `reject()` at `Promise` constructor calls
 example().catch(err => console.log(err)); 

Note, Promise constructor is not necessary at ajaxRequest function.

const ajaxRequest = (url) => 
    axios.get(url)
      .then((response) => {
        //console.log(response);
        return response;
      })
      .catch((error) => {
        //console.log(error);
      });

The only issue with the code you provided is that result from xmlParser is lost, forEach loop just iterates but does not store results. To keep results you will need to use Array.map which will get Promise as a result, and then Promise.all to wait and get all results into array.

I suggest to use async/await from ES2017 which simplifies dealing with promises. Since provided code already using arrow functions, which would require transpiling for older browsers patibility, you can add transpiling plugin to support ES2017.

In this case your code would be like:

function example() {
  return Promise.all([
    array.map(async (value) => {
      try {
        const response = await ajaxRequest(value.url);
        return xmlParser(response);
      } catch(err) {
        console.error(err);
      }
    })
  ])
}

Above code will run all requests in parallel and return results when all requests finish. You may also want to fire and process requests one by one, this will also provide access to previous promise result if that was your question:

async function example(processResult) {
  for(value of array) {
    let result;
    try {
      // here result has value from previous parsed ajaxRequest.
      const response = await ajaxRequest(value.url);
      result = await xmlParser(response);
      await processResult(result);
    } catch(err) {
      console.error(err);
    }
  }
}

Another solution is using Promise.all for doing this, i think is a better solution than looping arround the ajax requests.

const array = [{"id": 1, "url": "www.link1."}, {"id": 1, "url": "www.link2."}]

function example() {
    return Promise.all(array.map(x => ajaxRequest(x.url)))
        .then(results => {
            return Promise.all(results.map(data => xmlParser(data)));
        });
}

example().then(parsed => {
    console.log(parsed); // will be an array of xmlParsed elements
});

Are there any better ways to pass previous promise results as parameter in then() chain?

In fact, you can chain and resolve promises in any order and any place of code. One general rule - any chained promise with then or catch branch is just new promise, which should be chained later.

But there are no limitations. With using loops, most mon solution is reduce left-side foldl, but you also can use simple let-variable with reassign with new promise.

For example, you can even design delayed promises chain:

function delayedChain() {
  let resolver = null
  let flow = new Promise(resolve => (resolver = resolve));
  for(let i=0; i<100500; i++) {
    flow = flow.then(() => {
      // some loop action
    })
  }

  return () => {
    resolver();
    return flow;
  }
}

(delayedChain())().then((result) => {
  console.log(result)
})