Is this type of promise nesting good practice?

Question

I just wanted to know if it is considered good practice to nest promises like in this example, or is there better alternatives ?

getDatabaseModel.searchId(idNoms).then(function([noms, elements]) {
        getDatabaseModel.getLocalisations().then(function(localisations){
            getDatabaseModel.getStates().then(function(states){
                //Some code
            })
        })
    })

You're reinventing callback hell, which is what promises were designed to avoid. Return a new promise into the chain: `a.b().then(function (args) { return a.c(); }).then(function (args) { return a.d(); }).then(...)` — deceze, Jan 14 '21 at 07:52
Also, it doesn't look like your calls really depend on each other, so you might want to fire them all in parallel, and accumulate the results: `Promise.all([a.b(), a.c(), a.d()]).then(function ([b, c, d]) { ... })`. — deceze, Jan 14 '21 at 07:54
[Aren't promises just callbacks?](https://stackoverflow.com/q/22539815) — VLAZ, Jan 14 '21 at 09:38

Sudhanshu Kumar · Answer 1 · 2021-01-14T08:02:40.353

3

Promises were made to avoid callback hell, but they are not too good at it too. People like promises until they find async/await. The exact same code can be re-written in async/await as

async getModel(idNoms){
  const [noms, elements] = await getDatabaseModel.searchId(idNoms);
  const localisations = await getDatabaseModel.getLocalisations();
  const state = await getDatabaseModel.getStates():
  // do something using localisations & state, it'll work

}

getModel(idNoms);

Learn async/await here

edited Jan 14 '21 at 08:02

answered Jan 14 '21 at 07:53

Sudhanshu Kumar

1,926
1
9
21

`await` keyword much says the code should wait until the async request is finished and then afterwards it'll execute the next thing. While those promises are indepentent. So `promise.all` is better than :) – Nguyễn Văn Phong Jan 14 '21 at 08:32
I understand what you mean and it's not that I am not aware of Promise.all, going by OP's code, I do see they are not dependent on each other, but I also noted that OP has nested them coz of some reason instead, otherwise he would have not nested them and called 3 promises serially and hence awaiting, I put the results into variables too. @Phong – Sudhanshu Kumar Jan 14 '21 at 08:54
2

Seems like Promise.all() was the best way in my case, but i'm sure your example will be helpful in the future ! Thank you very much for your help ! – Hugo Jan 14 '21 at 09:02

Nguyễn Văn Phong · Accepted Answer · 2021-01-14T09:00:50.537

Obviously, your promises are independent. So you should use Promise.all() to make it run parallel with the highest performance.

The Promise.all() method takes an iterable of promises as an input, and returns a single Promise that resolves to an array of the results of the input promises

var searchById = getDatabaseModel.searchId(idNoms);
var getLocalisations = getDatabaseModel.getLocalisations();
var getStates = getDatabaseModel.getStates();

var result = Promise.all([searchById, getLocalisations, getStates]);
result.then((values) => {
  console.log(values);
});

For example, Let's say each promise takes 1s - So it should be 3s in total, But with Promise.all, actually it just takes 1s in total.

var tick = Date.now();
const log = (v) => console.log(`${v} \n Elapsed: ${Date.now() - tick}`);

log("Staring... ");
var fetchData = (name, ms) => new Promise(resolve => setTimeout(() => resolve(name), ms));
var result = Promise.all(
            [
              fetchData("searchById", 1000), 
              fetchData("getLocalisations", 1000),
              fetchData("getStates", 1000)
            ]);
result.then((values) => {
  log("Complete...");
  console.log(values);
});

Besides, If you're concern about asyn/await with more elegant/concise/read it like sync code, then await keyword much says the code should wait until the async request is finished and then afterward it'll execute the next thing. While those promises are independent. So promise.all is better than in your case.

var tick = Date.now();
const log = (v) => console.log(`${v} \n Elapsed: ${Date.now() - tick}`);

var fetchData = (name, ms) => new Promise(resolve => setTimeout(() => resolve(name), ms));
Run();

async function Run(){
  log("Staring... ");
  
  var a = await fetchData("searchById", 1000);
  var b = await fetchData("getLocalisations", 1000);
  var c = await fetchData("getStates", 1000);
  
  log("Complete...");
  console.log([a, b, c]);
}

Sam R. · Answer 3 · 2021-01-14T08:14:42.867

IMO it's a little hard to read and understand. Compare with this:

getDatabaseModel.searchId(idNoms)
    .then(([noms, elements]) => getDatabaseModel.getLocalisations())
    .then(localization => getDatabaseModel.getStates());

As @deceze pointed out there are two things to note:

These functions are called serially
They don't seem to depend on each other as the noms, elements and localization are not used at all.

With Promise.all you can mix and match however you want:


// Call `searchId` and `getState` at the same time
// Call `getLocalisations` after `searchId` is done
// wait for all to finish
Promise.all([
  getDatabaseModel.searchId(idNoms).then(([noms, elements]) => getDatabaseModel.getLocalisations()),
  getDatabaseModel.getStates()
]).then(([result1, result2]) => console.log('done'));


// Call all 3 at the same time
// wait for all to finish
Promise.all([
  getDatabaseModel.searchId(idNoms),
  getDatabaseModel.getLocalisations(),
  getDatabaseModel.getStates(),
]).then(([result1, result2, result3]) => console.log('done'));

Is this type of promise nesting good practice?

3 Answers3