0

Here is a working version of my code which returns everything as expected:

***.then(r => r.json()).then(async r => {

                    for (let i = 0; i < r.length; i++) {
                        let pipeline = r[i];
                        pipeline.collapsed = true;
                        pipeline.levels = await this.getPipelineLevels(pipeline.id);
                    }

                    this.project.pipelines.items = r;
                })

Here is the "broken" version that returns strange results:

****.then(r => r.json()).then(r => {
                    let pipelines = r.map(async (value) => {
                        let levels = await this.getPipelineLevels(value.id);
                        return {...value, collapsed: true, levels: levels};
                    });

                    this.project.pipelines.levels = pipelines;

Strange output in console with console.log(JSON.stringify(pipelines)) after *.map():

[{"_c":[],"_s":0,"_d":false,"_h":0,"_n":false},{"_c":[],"_s":0,"_d":false,"_h":0,"_n":false}]

What's happening here?

nicholaswmin
  • 21,686
  • 15
  • 91
  • 167
AndrewShmig
  • 4,843
  • 6
  • 39
  • 68

2 Answers2

3

Because Array.map doesn't actually await it's passed callback. It doesn't care that you marked it's callback as async.

Just Array.map your Promises then pass them to Promise.all and let that await everything (in-parallel) for you.

const getPipelineLevels = id => new Promise(resolve => {
  setTimeout(() => resolve({ id: id, foo: 'bar' }), 500)
})

const idx = [1,2,3,4,5]

const tasks = idx.map(id => {
  return getPipelineLevels(id)
    .then(value => ({ ...value, bar: 'baz' }))
})

Promise.all(tasks)
  .then(results => {
    console.log(results)
  })
nicholaswmin
  • 21,686
  • 15
  • 91
  • 167
  • Thank you, but a lot of unnecessary code lines :) What @gleam suggested fits better for me. – AndrewShmig Feb 05 '19 at 17:09
  • @AndrewShmig Err, what's an "unnecessary code line"? I can put this through WinZip so it becomes 0 and 1's if that's what floats your boat. – nicholaswmin Feb 05 '19 at 17:10
  • I understood everything ok, don't worry, but that's just not my coding style. That's all. Thanks again! – AndrewShmig Feb 05 '19 at 17:13
  • @AndrewShmig I don't think you understand. gleam's previous answer was mostly OK but not entirely correct. This might not be "your style" but it's correct. In any case, gleam has adjusted his so it's correct. – nicholaswmin Feb 05 '19 at 17:14
  • Thought about this a little bit more... you are abs right. Thank you! – AndrewShmig Feb 05 '19 at 17:34
2

Try something like this:

.then(async r => {
    let pipelines = await Promise.all(r.map(async (value) => {
        let levels = await this.getPipelineLevels(value.id);
        return {...value, collapsed: true, levels: levels};
    }));
    this.project.pipelines.levels = pipelines;
});

Array.map(async (value) => {...}) returns an array of Promises.

This solution would also be faster than what the OP was trying to achieve anyway since it awaits in-parallel.

And notice that you should to avoid awaiting a .then(…) chain.

gleam
  • 861
  • 6
  • 12
  • 2
    FWIW this solution would also be faster than what the OP was trying to achieve anyway since it awaits in-parallel. But really there's no need for `async` in `.map`. Just map the promises and let `Promise.all` await them. If you need to modify the value of the Promise, just attach a `.then` to each Promise inside `.map` and do your work there. – nicholaswmin Feb 05 '19 at 16:57
  • 1
    @NikKyriakides - May I use your comment to extend an answer? – gleam Feb 05 '19 at 17:05
  • @NikKyriakides, thank you for more exact explanation! @gleam, doh... haven't seen that `map` with `async` returns promises... thanks! – AndrewShmig Feb 05 '19 at 17:07
  • @gleam Sure you can, I've written up an answer as well which I think is more correct. If you implement what I've done there I'll delete mine. – nicholaswmin Feb 05 '19 at 17:09
  • 1
    That's not implementing the same suggestion. That's whole-sale cut-and-paste. – Scott Sauyet Feb 05 '19 at 17:14
  • @gleam, you have entirely changed the answer I was accepting. Why? – AndrewShmig Feb 05 '19 at 17:14
  • 1
    @AndrewShmig Because this one is correct, the previous wasn't. – nicholaswmin Feb 05 '19 at 17:15
  • I am trying to put all clever-things in one answer :) – gleam Feb 05 '19 at 17:16
  • @NikKyriakides, why the prev wasn't? Can you explain? Is it a working solution? yes, it works. Then why it's not correct? – AndrewShmig Feb 05 '19 at 17:16
  • @AndrewShmig Because using `async` in a `.map` served no purpose. Also posts here are meant to help others as well so the OP generalising his example like I did is likely to help that purpose. – nicholaswmin Feb 05 '19 at 17:17
  • @NikKyriakides async in map has effect. You can use `await` so you don't need to use Promises inside callback ;) – gleam Feb 05 '19 at 17:19
  • Err no. `.map` doesn't await each promise in each iteration. In the context of this question it has no effect. That's the point of the question. – nicholaswmin Feb 05 '19 at 17:20
  • I am agree. In the context of this question it has no effect. But this case is still possible. – gleam Feb 05 '19 at 17:21
  • 1
    If you believe your previous answer has merits of it's own, rollback your answer to what it was and I'll undelete my answer. Don't let me influence you into what your answer should be. – nicholaswmin Feb 05 '19 at 17:22