Which of the following codes is the correct way of using Promises in JavaScript?

Question

So, I would consider this more of a discussion rather than a question. I was working with Promises and my code was fully functional and everything was working fine but then I came across another way of achieving the same result, but I feel that this way is not the best way of using Promises. So, I wanted to discuss how to effectively use Promises.

So, what I am trying to do is fetch the data of different pokemons using the pokeapi and then display the names and the types of the pokemons in the order which they appear. (NOTE: It is important that the order doesn't break).

I fetch this url first https://pokeapi.co/api/v2/pokemon?limit=150, then .json() the response and then this json contains an array results that contains urls for each pokemon in order. Now I use Promise.all() to fetch all these urls and then again Promise.all() for converting these responses into jsons. Note: I use Promise.all() twice this is important because the second approach uses it only once which bothers me.

So, this is the code that I wrote

const container = document.querySelector(".container");

fetch('https://pokeapi.co/api/v2/pokemon?limit=150')
  .then(response => response.json())
  .then(json => {
    const responseArr = [];
    json.results.forEach(el => {
      responseArr.push(fetch(el.url));
    });

    return Promise.all(responseArr);
  })
  .then(responses => {
    const jsonArr = [];
    responses.forEach(el => {
      jsonArr.push(el.json());
    });

    return Promise.all(jsonArr);
  })
  .then(jsons => {
    jsons.forEach((json, index) => {
      const pokemonName = json.name;
      const pokemonType = json.types[0].type.name;
      container.innerHTML += `(${index+1}) ${pokemonName} - ${pokemonType} <br>`;
    });
  })

<div class="container"></div>

And this is the second approach

const container = document.querySelector(".container");

fetch('https://pokeapi.co/api/v2/pokemon?limit=150')
  .then(response => response.json(), e => {
    console.error(e);
    throw e;
  })
  .then(json => {
    Promise.all(json.results.map(el => fetch(el.url)))
      .then(arr => {
        arr.map(response => response.json())
          .forEach((result, index) => {
            result.then(el => {
              const pokemonName = el.name;
              const pokemontype = el.types[0].type.name;
              container.innerHTML += `(${index+1}) ${pokemonName} - ${pokemontype} <br>`;
            })
          })
      }).catch(e => {
        console.error(e)
        throw e;
      });
  }).catch(e => {
    console.error(e)
    throw e;
  });

<div class="container"></div>

In the second solution, this portion bothers me.

arr.map(response => response.json())
          .forEach((result, index) => {
            result.then(el => {
              const pokemonName = el.name;
              const pokemontype = el.types[0].type.name;
              container.innerHTML += `(${index+1}) ${pokemonName} - ${pokemontype} <br>`;
            })
          })

Because response.json() will also return promise so it should also be chained properly, here the one's whose .json() is fulfilled first will get added into the inner HTML first, so the order of listing the pokemons might get disturbed.

Also, I found something weird just now while writing this, if the api endpoint is changed to https://pokeapi.co/api/v2/pokemon?limit=15 i.e. requesting only for 15 pokemons, the order gets disturbed for the second approach everytime the code is executed but surprisingly it stays intact when requesting for 150 pokemons.

Answer 1

For the record, I recommend TTY112358's solution. But to answer your initial question, which was not about performance:

Promises were designed, among other things, so you could avoid the nesting in your second example. So yeah, the first version is more "correct" — or to be more precise, the second version is just as correct but it's ugly, written in what is considered a bad style.

BTW, your first example could be simplified by using map, like this, and you'll see that async/await isn't necessarily superior to a well-written chain of thens — it really depends on whether the logic is indeed a chain and whether you have ready-to-use functions that can be used in it:

fetch('https://pokeapi.co/api/v2/pokemon?limit=150')
  .then(response => response.json())
  .then({results} => Promise.all(results.map(el => fetch(el.url))))
  .then(responses => Promise.all(responses.map(el => el.json()))
  .then(jsons => {
    jsons.forEach((json, index) => {
      const pokemonName = json.name;
      const pokemonType = json.types[0].type.name;
      container.innerHTML += `(${index+1}) ${pokemonName} - ${pokemonType} <br>`;
    });
  })

Answer 2

What about the following solution which prints pokemon as fast as possible and keeps the order at the same time.

const container = document.querySelector(".container");

async function fetchPokemons(){
  const response = await fetch('https://pokeapi.co/api/v2/pokemon?limit=150');
  const {results} = await response.json();
  const promises = results.map(async (result, index) => {
    const {url} = result;
    const pokemon = await (await fetch(url)).json();
    await Promise.all(promises.slice(0, index));
    const pokemonName = pokemon.name;
    const pokemontype = pokemon.types[0].type.name;
    container.innerHTML += `(${index+1}) ${pokemonName} - ${pokemontype} <br>`;
    return;
  });
  return Promise.all(promises);
}

fetchPokemons();

<div class="container"></div>

Answer 3

so the order of listing the pokemons might get disturbed.

Yes indeed. Do not use this approach if it is broken.

However, this doesn't mean that you need to use Promise.all twice. Instead of first waiting for all responses and then waiting for all response bodies, make a separate promise chain for each item in the array, and then only wait for all end results at once:

const container = document.querySelector(".container");

fetch('https://pokeapi.co/api/v2/pokemon?limit=150')
  .then(response => response.json())
  .then(json => {
    const jsonPromises = json.results.map(el => {
      return fetch(el.url).then(response => response.json());
    });
    return Promise.all(jsonPromises);
  })
  .then(arr => {
    for (const [index, json] of arr.entries()) {
      const pokemonName = json.name;
      const pokemonType = json.types[0].type.name;
      container.innerHTML += `(${index+1}) ${pokemonName} - ${pokemonType} <br>`;
    });
  })