0

I am trying to process all images in an array and push all of the processed images in other array.

The processing stuff runs async, so I am using await Promise.all to wait until all of the images has been processed. Anyways, the array is undefined when the code finishs.

Here is what I am doing:

async function processImage(image) {
    // ... stuff
    return new Promise((resolve, reject) => {
      (async () => {
        // Validate each image uploaded by the user
        const images = await Promise.all( // <--------------------------------
          imagesIds.map(async (id) => {
             // ... async stuff

            if(!error)
              return processedImage; // <-------------------------------
            else {
              reject(error);
            }  
          }
        }
        // ... stuff
        console.log(images); <-------- undefined
        resolve();
      })();
  }  

Any ideas what I am doing wrong? Thanks.

Raul
  • 2,673
  • 1
  • 15
  • 52
  • 4
    `async (resolve, reject)` is an anti-pattern. – Marco Aug 04 '20 at 19:06
  • `images` will never be `undefined`. `Promise.all` fulfills with an array or rejects, and not even your `// ... stuff` will be able to change the `const images`. – Bergi Aug 04 '20 at 19:29
  • [Never pass an `async function` as the executor to `new Promise`](https://stackoverflow.com/q/43036229/1048572). And no, your edit of moving it to an IIEFE does not solve that problem. – Bergi Aug 04 '20 at 19:32
  • I have created an async function and invoke it on the Promise.all(). Do you think it is correct? Just: const images = await Promise.all(imagesIds.map((imageId) => processImage(imageId)) – Raul Aug 04 '20 at 19:41
  • @Bergi but moving the async executor to an IIFE wouldn't solve the anti-pattern? – Raul Aug 04 '20 at 19:41
  • 1
    @Raul Yes, `const images = await Promise.all(imagesIds.map(processImage));` is totally fine. You don't need any of that `new Promise` or IIFE stuff around it. – Bergi Aug 04 '20 at 19:46
  • @Bergi the thing is that it is an https callable function and I need to run this code inside a new Promise because the async function which is called in the map can throw errors which have to be rejected. – Raul Aug 04 '20 at 20:01
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/219223/discussion-between-raul-and-bergi). – Raul Aug 04 '20 at 20:05
  • 1
    @Raul No, you don't need `new Promise`. If the async functions throw exceptions/return rejected promises, they will automatically reject the surrounding promise. You should just write `throw err;` it will work like the `return processedImage`. – Bergi Aug 04 '20 at 20:18

1 Answers1

0

Promise.all takes an array of Promises that you want to be processed, so in this case, you want your imageIds.map function to return a list of promises. You do not need to do the async stuff in the map function, but just return a promise that can be taken care of by the Promise.all. One example is that if you are doing network requests for each image id have the map function return fetch(yourEndPoint + id) and the Promise.all function will handle the async processing.

See this: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all

User81646
  • 628
  • 4
  • 13
  • "*You do not need to do the async stuff in the map function*" - but the `async` is how the `map` callback returns a promise. – Bergi Aug 04 '20 at 19:32