CODEWITHSUNDEEP
javascriptpromise
angry kiwi
angry kiwi

Reputation: 11445

Map data from array into Promises and execute code on Promise.all() not working

I have an array like this

let array =[ {message:'hello'}, {message:'http://about.com'}, {message:'http://facebook.com'}]

I want to loop through it and at each item, I make a request to server to get open graph data, then save the fetched data back to array. Expected result

array =[ 
    {message:'hello'}, 
    {message: {
            url:'http://about.com', title:'about'
        }
    }, 
    {message:{
            url:'http://facebook.com', title:'facebook'
        }
    }
]

And I need to execute something else after the async fully complete. Below code is what I think it would be

let requests = array.map( (item) => {
    return new Promise( (resolve) => {
        if (item.message.is('link')) {
            axios.get(getOpenGraphOfThisLink + item.message)
            .then( result => {
                item.message =result.data
                // console.log(item.message)
                // outputs were
                //{url:'http://about.com', title:'about'}
                //{url:'http://facebook.com', title:'facebook'}
                resolve()
            })

        }
    })
})

Promise.all(requests).then( (array) => {
    // console.log (array)
    // nothing output here
})

The promise.all() won't run. console.log(array) does not output anything.

Upvotes: 1

Views: 336

Answers (1)

T.J. Crowder
T.J. Crowder

Reputation: 1074276

I see three main issues with that code:

  1. Critically, you're only sometimes resolving the promise you create in the map callback; if item.message.is('link') is false, you never do anything to resolve. Thus, the Promise.all promise will never resolve.

  2. You're accepting array as an argument to your Promise.all then callback, but it won't be one (or rather, not a useful one).

  3. You're not handling the possibility of a failure from the axios call.

And if we resolve #2 by pre-filtering the array, then there's a fourth issue that you're creating a promise when you already have one to work with.

Instead:

let array = /*...*/;
let requests = array.filter(item => item.message.is('link'))
    .map(item => axios.get(getOpenGraphicOfThisLink + item.message)
        .then(result => {
            item.message = result.data;
            return result;
        })
    );

Promise.all(requests).then(
    () => {
        // Handle success here, using `array`
    },
    error => {
        // Handle error
    }
);

Note how reusing the axios promise automatically propagates the error up the chain (because we don't provide a second callback to then or a catch callback).

Example (doesn't demonstrate errors from axios.get, but...):

// Apparently you add an "is" function to strings, so:
Object.defineProperty(String.prototype, "is", {
  value(type) {
    return type != "link" ? true :  this.startsWith("http://");
  }
});

// And something to stand in for axios.get
const axios = {
  get(url) {
    return new Promise(resolve => {
      setTimeout(() => {
        resolve({data: "Data for " + url});
      }, 10);
    });
  }
};

// The code:
let array =[ {message:'hello'}, {message:'http://about.com'}, {message:'http://facebook.com'}]
let requests = array.filter(item => item.message.is('link'))
    .map(item => axios.get(/*getOpenGraphicOfThisLink + */item.message)
        .then(result => {
            item.message = result.data;
            return result;
        })
    );

Promise.all(requests).then(
    () => {
        // Handle success here, using `array`
        console.log(array);
    },
    error => {
        // Handle error
        console.log(error);
    }
);

Upvotes: 3

Related Questions