0

I have a function responsible for uploading images to remote storage.

I also have another function that creates a user in a DB. During the user creation, I should upload the image, get back the URL of the uploaded image, and add it to the user object before adding it to the database. However, I am stuck at getting back the urls:

My function that uploads images:

export const writePicture = (picture) => {
    if (picture !== undefined) {

       //code to upload image to database (...)

                .then((url) => {
                    console.log(`finished executing writePicture, url: ${url}`) //all good here, url is console logged.
                    return url;
                });
        });
    } else {
        console.log("finished executing writePicture")
        return ""
    };
}

The above function is exported to another file where I use promises. My promises are currently set as follows:

//pictureLogo and pictureImage are existing image objects

const waitForLogoUrl = new Promise((resolve, reject) => {
  resolve(writePicture(pictureLogo));
});

const waitForImageUrl = new Promise((resolve, reject) => {
  resolve(writePicture(pictureImage));
});

Promise.all([waitForLogoUrl, waitForImageUrl]).then((urls) => {
  //urls should be an array containing the urls from first and second promise
  console.log("the urls are:");
  console.log(urls);
});

The above is not working as expected. Inside urls, I am getting an array of two undefined entries instead of the two obtained urls.

writePicture() is working fine, it is uploading the pictures and console logging the urls. However the urls are being console logged after the Promise.all has finished:

https://i.stack.imgur.com/RenRs.jpg

I am new to using promises so I am probably missing something about how to call a function inside a resolve. Any help is appreciated.

Ghadir
  • 507
  • 10
  • 21
  • 1
    Does this answer your question? [How do I return the response from an asynchronous call?](https://stackoverflow.com/questions/14220321/how-do-i-return-the-response-from-an-asynchronous-call) – evolutionxbox Jan 27 '21 at 19:04
  • I guess it does to some extent but there are many examples there and I wouldn't have found my way around it myself. Thank you for the suggestion though, it is a good resource – Ghadir Jan 27 '21 at 19:31

1 Answers1

1

Usually when new Promise appears in code, it's a red flag, and this is no exception. Below I assume you cannot use async/await, and it only corrects 2 things:

  1. Return promises from writePicture
  2. Don't unnecessarily wrap its result in another promise.
export const writePicture = (picture) => {
    if (picture !== undefined) {
      return codeToUploadImage(...)   // Return is critical here
         .then((url) => {
            console.log(`finished executing writePicture, url: ${url}`) //all good here, url is console logged.
            return url;
         });
    } else {
        console.log("finished executing writePicture")
        return ""
    };
}


//pictureLogo and pictureImage are existing image objects

const waitForLogoUrl = writePicture(pictureLogo);
const waitForImageUrl = writePicture(pictureImage);

Promise.all([waitForLogoUrl, waitForImageUrl]).then((urls) => {
  //urls should be an array containing the urls from first and second promise
  console.log("the urls are:");
  console.log(urls);
})
Evert
  • 93,428
  • 18
  • 118
  • 189
  • Since I am new to Promises (literaly just learned it today to write the above), why is it a red flag? Is there another way to use them without using the `new Promise` constructor? Thank you – Ghadir Jan 27 '21 at 19:10
  • 1
    @Ghadir [What is the explicit promise construction antipattern and how do I avoid it?](https://stackoverflow.com/q/23803743) `writePicture` already *should* return a promise. Wrapping it into another promise is not very useful. The only issue here seems to be that `writePicture` returns a non-promise on failure (when the input is `undefined`). If the function returns a promise either way, then there is no use for the Promise constructor. – VLAZ Jan 27 '21 at 19:25