Promise not returned from recursive function

Question

Background

I have a problem where my promise is not being returned to its calling function. I know this would usually work, but it's not when inside a recursive function. The resolve is being called and I can see the function FindElementById is finding the element correctly, however the call in the thenable UpdateMasterMenuItem(result, queuedItem) is not being called.

What am I doing in my code snippets, what am I trying to do?

Loop over a collection queuedItems and find those items in another collection menuItems. I do this by passing in the Id of the item I'm trying to find, as well as the collection it's in, into a recursive function FindElementById. When the item is found, I'm returning the found item and doing some other stuff to it in the function UpdateMasterMenuItem.

Code

// Calling Loop
        queuedItems.forEach(function (queuedItem) {
            FindElementById(queuedItem.dataset.id, menuItems).then(function(result) {
                UpdateMasterMenuItem(result, queuedItem);
            });
        });


// Recursive Function
function FindElementById(id, menuItems) {
    return new Promise((resolve) => {
        menuItems.forEach(function (menuItem) {
            if (menuItem.Id === id) {
                return resolve(menuItem);

            } else if (menuItem.Child.length > 0) {
                FindElementById(id, menuItem.Child);
            }
        });
    });
}

Is there a reason why FindElementById returns a promise? There's nothing asynchronous going on in the method. — Terry
– Terry, Commented Apr 19, 2020 at 10:59

Terry · Accepted Answer · 2020-04-19 11:08:49Z

2

The issues comes from the else if block: when you recursively invoke the method by itself, you also need to invoke resolve() once nested promise is resolved: otherwise the .then() callback will never fire.

function FindElementById(id, menuItems) {
    return new Promise((resolve) => {
        menuItems.forEach(function (menuItem) {
            if (menuItem.Id === id) {
                return resolve(menuItem);

            } else if (menuItem.Child.length > 0) {
                // You also need to remember to resolve here!
                FindElementById(id, menuItem.Child).then(nestedMenuitem => resolve(nestedMenuitem));
            }
        });
    });
}

However, there is an issue: the promise can only be resolved once. So, your promise is actually resolved at the first iteration of the forEach loop if the condition is satisfied. Is that what you want? If the ID is indeed truly unique, or that in the event of duplicate IDs you only want to return the first instance, then this is okay.

This brings us back to my comment: your function is not doing any asynchronous operation, so it is probably easier to make do without promises altogether:

function FindElementById(id, menuItems) {
    let el;
    for (let menuItem of menuItems) {
        if (menuItem.Id === id) {
            el = menuItem;
            break;
        } else {
            el = FindElementById(id, menuItem.Child)
        }
    }

    return el;
}

edited Apr 19, 2020 at 11:08

answered Apr 19, 2020 at 11:01

Terry

66.7k16 gold badges107 silver badges127 bronze badges

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

1 Comment

Dan Cundy Over a year ago

Great points. Upon reflection it seems I decided to try using a promise as a solution to a problem that I didn't properly understand.

Collectives™ on Stack Overflow

Promise not returned from recursive function

Background

Code

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

Background

Code

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related