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

Question

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.

Can you put a runnable minimal reproducible example in the question (using Stack Snippets, the <> toolbar button), with something filling in for axios.get? Also, what do you see when you debug? — T.J. Crowder
– T.J. Crowder, Commented Nov 18, 2016 at 16:23

T.J. Crowder · Accepted Answer · 2016-11-18 16:41:32Z

3

I see three main issues with that code:

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.
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).
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);
    }
);

edited Nov 18, 2016 at 16:41

answered Nov 18, 2016 at 16:26

T.J. Crowder

1.1m201 gold badges2k silver badges2k bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

T.J. Crowder Over a year ago

@aw04: Heh, I'd twigged to that belatedly and was just adding that. :-)

angry kiwi Over a year ago

the // Handle error part is for handling error of axios?

T.J. Crowder Over a year ago

@angry_kiwi: Yes. The promise from Promise.all is resolved if all input promises resolve, or rejected if any input promise is rejected (and it doesn't wait for the others). If you instead want to handle those axios errors earlier, just do it as a second handler on the then inside the map callback (or as a catch there).

angry kiwi Over a year ago

When I use redux thunk function dispatch({type : constant.GET_POST,payload: array}) inside the // handle success. It will always dispatch the orginal array instead of the modified one. Any idea?

angry kiwi Over a year ago

if I do settimeOut( ()=> dispatch(...), 4000) then it will dispatch the modified array. I thought when we code in // handle success section, everything is synchronous from there. I thought wrong?

|

Collectives™ on Stack Overflow

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

1 Answer 1

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related