javascript promises recursion

Question

I have a async recursive function which returns promise if there is more work to do or returns result array otherwise. In case where no recursion is involved it correctly return the array but when recursion is there the array is undefined. The code is

function foo(filepath) {

  var resultArr = [];

  function doo(file) {
        return asyncOperation(file).then(resp => {
            resultArr.push(resp.data);
            if (resp.pages) {
                var pages = resp.pages.split(',');
                pages.forEach(page => {
                    return doo(page);
                });
            } else {
                return resultArr;
            }
        });
    }

    return doo(filepath);
}

And the way this is called

foo(abcfile).then(function(result){
    console.log(result);
});

If I pass abcfile which has no resp.pages, I get result array, but there are resp.pages, then the result array is undefined.

Answer 1

I think you're just missing a returned promise within the if (resp.pages) block

if (resp.pages) {
    return Promise.all(resp.pages.split(',').map(page => doo(page)))
        .then(pagesArr => {
            return resultArr.concat(...pagesArr)
        })
}

---

I'm thinking there may be an issue with scoping resultArr outside the doo function so maybe try this

function foo(filepath) {
    function doo(file) {
        return asyncOperation(file).then(resp => {
            const resultArr = [ resp.data ]
            if (resp.pages) {
                return Promise.all(resp.pages.split(',').map(page => doo(page)))
                    .then(pagesArr => resultArr.concat(...pagesArr))
            } else {
                return resultArr
            }
        })
    }

    return doo(filePath)
}

---

To explain the use of the spread-operator, look at it this way...

Say you have three pages for a file top; page1, page2 and page3 and each of those resolves with a couple of sub-pages each, the pagesArr would look like

[
  ['page1', 'page1a', 'page1b'],
  ['page2', 'page2a', 'page2b'],
  ['page3', 'page3a', 'page3b']
]

and resultArr so far looks like

['top']

If you use concat without the spread operator, you'd end up with

[
  "top",
  [
    "page1",
    "page1a",
    "page1b"
  ],
  [
    "page2",
    "page2a",
    "page2b"
  ],
  [
    "page3",
    "page3a",
    "page3b"
  ]
]

But with the spread, you get

[
  "top",
  "page1",
  "page1a",
  "page1b",
  "page2",
  "page2a",
  "page2b",
  "page3",
  "page3a",
  "page3b"
]

Answer 2

To verify this works, I'll make a fake dataset, and a fakeAsyncOperation which reads data from the dataset asynchronously. To model your data closely, each query from the fake dataset returns a response with data and pages fields.

let fake = new Map([
  ['root',    {data: 'root',    pages: ['a', 'b', 'c', 'd']}],
  ['a',       {data: 'a',       pages: ['a/a', 'a/a']}],
  ['a/a',     {data: 'a/a',     pages: []}],
  ['a/b',     {data: 'a/b',     pages: ['a/b/a']}],
  ['a/b/a',   {data: 'a/b/a',   pages: []}],
  ['b',       {data: 'b',       pages: ['b/a']}],
  ['b/a',     {data: 'b/a',     pages: ['b/a/a']}],
  ['b/a/a',   {data: 'b/a/a',   pages: ['b/a/a/a']}],
  ['b/a/a/a', {data: 'b/a/a/a', pages: []}],
  ['c',       {data: 'c',       pages: ['c/a', 'c/b', 'c/c', 'c/d']}],
  ['c/a',     {data: 'c/a',     pages: []}],
  ['c/b',     {data: 'c/b',     pages: []}],
  ['c/c',     {data: 'c/c',     pages: []}],
  ['c/d',     {data: 'c/d',     pages: []}],
  ['d',       {data: 'd',       pages: []}]
]);

let fakeAsyncOperation = (page) => {
  return new Promise(resolve => {
    setTimeout(resolve, 100, fake.get(page))
  })
}

Next we have your foo function. I've renamed doo to enqueue because it works like a queue. It has two parameters: acc for keeping track of the accumulated data, and xs (destructured) which is the items in the queue.

I've used the new async/await syntax that makes it particularly nice for dealing with this. We don't have to manually construct any Promises or deal with any manual .then chaining.

I made liberal use of the spread syntax in the recursive call because I its readability, but you could easily replace these for concat calls acc.concat([data]) and xs.concat(pages) if you like that more. – this is functional programming, so just pick an immutable operation you like and use that.

Lastly, unlike other answers that use Promise.all this will process each page in series. If a page were to have 50 subpages, Promise.all would attempt to make 50 requests in parallel and that may be undesired. Converting the program from parallel to serial is not necessarily straightforward, so that is the reason for providing this answer.

function foo (page) {
  async function enqueue (acc, [x,...xs]) {
    if (x === undefined)
      return acc
    else {
      let {data, pages} = await fakeAsyncOperation(x)
      return enqueue([...acc, data], [...xs, ...pages])
    }
  }
  return enqueue([], [page])
}

foo('root').then(pages => console.log(pages))

Output

[ 'root',
  'a',
  'b',
  'c',
  'd',
  'a/a',
  'a/a',
  'b/a',
  'c/a',
  'c/b',
  'c/c',
  'c/d',
  'b/a/a',
  'b/a/a/a' ]

---

Remarks

I'm happy that the foo function in my solution is not too far off from your original – I think you'll appreciate that. They both use an inner auxiliary function for looping and approach the problem in a similar way. async/await keeps the code nice an flat and highly readable (imo). Overall, I think this is an excellent solution for a somewhat complex problem.

Oh, and don't forget about circular references. There are no circular references in my dataset, but if page 'a' were to have pages: ['b'] and page 'b' had pages: ['a'], you can expect infinite recursion. Because this answer processes the pages serially, this would be a very easy to fix (by checking the accumulated value acc for an existing page identifier). This is much trickier (and out-of-scope of this answer) to handle when processing the pages in parallel.