2

ESlint, and the rest of the world, tells me to avoid nesting Promises. However, how can you apply a function to a number of items individually where the previous promise returns an array? For example, I want to run doSomethingWithIt on each element returned by the previous Promise, and also do something with the result of that.

function getSomeThings() {
   return new Promise((resolve, reject) => {
     resolve(['one', 'two', 'three'])
   })
}

function doSomethingWithIt(theThing) {
  return new Promise((resolve, reject) => {
     console.log(theThing)
     resolve("Did a thing")
  })
}

getSomeThings().then((things) => {
   things.forEach((thing) => {
      doSomethingWithIt(thing)
       .then((result) => {
          console.log(`Result was: ${result}`)
       })
   })
})

In reality, rather than just printing the return value, that function might return an ID or something useful to log when an operation has completed (e.g. commit to a database).

How can this be achieved without nesting the promises? Would I need to add a step which uses Promise.all to aggregate the results? Or is there a neater way of doing this...

naxxfish
  • 117
  • 1
  • 7
  • 4
    In your example, you're trying to call `then` on the result of `forEach`, which returns `undefined`...? If you fix that, I don't see any promise nesting. (There's also no reason for that wrapper around the call to `doSomthingWithIt`, just do `.then(doSomethingWithIt)`.) Here's a corrected version: ``getSomeThings().then((things) => { things.forEach(doSomethingWithIt); return things; }).then((result) { console.log(`Result was: ${result}`) });`` – T.J. Crowder Oct 14 '18 at 13:44
  • Depending on what you need to modify it might also be easier to use `Array#map()` instead of `forEach` – charlietfl Oct 14 '18 at 13:46
  • 1
    You seem to be looking for ``getSomeThings().then((things) => { console.log(`Result was: ${result.map(doSomethingWithIt)}`); })``? – Bergi Oct 14 '18 at 13:46
  • 1
    Notice that Eslint and the rest of the world are wrong when making their claim universal. Sometimes, nesting promise callbacks is just the right thing to do. (I can't tell however what you are trying to do, so I can't comment on whether this is one of these cases). – Bergi Oct 14 '18 at 13:48
  • Fixed the code so it does what it should (thanks @T.J.Crowder). I'm not sure there's an alternative to this, other than somewhat needlessly passing on the array via another .then(). – naxxfish Oct 14 '18 at 16:08
  • This video entirely explains the solution to this, in the context of Firebase /Google Cloud Functions https://www.youtube.com/watch?v=d9GrysWH1Lc&t=0s – naxxfish Oct 19 '18 at 00:12

2 Answers2

1

You'd use Promise.all on those promises you're creating in the loop. Not just to make ESLint happy, but so that you maintain a full success/failure chain:

getSomeThings()
    .then((things) => Promise.all(things.map(doSomethingWithIt)))
    .then((results) => {
        for (const result of results) {
            console.log(`Result was: ${result}`);
        }
    })
    .catch((error) => {
        // Handle/report errors
    });

Live Example:

function getSomeThings() {
   return new Promise((resolve, reject) => {
     resolve(['one', 'two', 'three'])
   })
}

function doSomethingWithIt(theThing) {
  return new Promise((resolve, reject) => {
     console.log(theThing)
     resolve("Did a thing")
  })
}

getSomeThings()
    .then((things) => Promise.all(things.map(doSomethingWithIt)))
    .then((results) => {
        for (const result of results) {
            console.log(`Result was: ${result}`);
        }
    })
    .catch((error) => {
        // Handle/report errors
    });

Yes, that does mean you don't get the console.log for any of them until all of those doSomethingWith promises have resolved. Often that's a good thing. But if it isn't in this case, then it might be reasonable to break the ESLint rule (which isn't, as bergi said in a comment, universal)

T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
1

ESlint, and the rest of the world, tells me to avoid nesting Promises.

Yes, you should avoid that when you can. However, there are many situations where nesting is totally appropriate - and the magic of promises is that they can be nested while still producing the same end result.

The ESlin advice is akin to "avoid indentation". That's true, you shouldn't write nested blocks when you don't need them. However, your use case is to "apply an operation to each result", and that requires a loop, which does justify nesting/indentation.

Of course you should avoid forEach, and return an appropriate promise for all operations of you loop to have finished:

getSomeThings().then(things => {
  return Promise.all(things.map(thing => {
    return doSomethingWithIt(thing).then(result => {
      console.log(`Result for ${thing} was ${result}`);
    });
  }));
}).then(() => {
  console.log("Did all things");
});
Bergi
  • 630,263
  • 148
  • 957
  • 1,375