ESLint reports a promise misuse for code I consider ok

Question

In a symbol table implementation I have this method:

    public getAllSymbols(type?: typeof Symbol, localOnly = false): Promise<Set<Symbol>> {
        const promise = super.getAllSymbols(type ?? Symbol, localOnly);

        return new Promise(async (resolve, reject) => {
            try {
                let result = await promise;

                if (!localOnly) {
                    this.dependencies.forEach(async (dependency) => {
                        result = new Set([...result, ...await dependency.getAllSymbols(type, localOnly)]);
                    });
                }

                resolve(result);
            } catch (reason) {
                reject(reason);
            }
        });
    }

which works fine, however ESLint reports 2 promise misuses:

Promise returned in function argument where a void return was expected. no-misused-promises

What's wrong with this code and how would I have to write it to get rid of the linter error?

Would imagine that the issue is that the executor function now returns a promise (as it is declared async) and so does the forEach argument. Neither of these functions are expected to return anything. — Woody, Sep 07 '21 at 09:00
I don't see a return, except for the one which the function returns. Looking at the [eslint error description](https://github.com/typescript-eslint/typescript-eslint/blob/v4.31.0/packages/eslint-plugin/docs/rules/no-misused-promises.md) uses as one example pretty much what I have done myself. — Mike Lischke, Sep 07 '21 at 09:04
The eslint error description tells you that your code will be reported as an error - it is under examples of incorrect code. making a function async automatically wraps the return value in a Promise - even if you have no return statement in your function - in which case it will return Promise......you can always await an async function. — Woody, Sep 07 '21 at 09:08

Yousaf · Accepted Answer · 2021-09-12T16:07:09.703

Problems in your code:

Executor function shouldn't be async - it's an anti-pattern
As super.getAllSymbol(...) already returns a promise, so no need to wrap it in a promise constructor - it's another anti-pattern. Call the then() method directly on the promise returned by super.getAllSymbol(...)
Using async-await with forEach() won't give you the expected result because the callback function of forEach() won't wait for the awaited promise to settle - it will just continue to the next iteration.

You can use Promise.all() along with the map() method to get the expected output.

You could also use for-of loop but using Promise.all() is better if you don't want all the promises to settle in a sequential manner.

Your code could be re-written as (types removed for simplicity):

public getAllSymbols(type, localOnly = false) {

     const promise = super.getAllSymbols(type ?? Symbol, localOnly);

     return promise
         .then(result => {
             if (!localOnly) {
                 return Promise.all(this.dependencies.map(dep => (
                    dep.getAllSymbols(type, localOnly))
                 )))
                 .then(resultArr => {
                     return new Set([...result, ...resultArr]);
                 });
             }
             else {
                 return result;
             }                       
         });
}

or you could use the async-await syntax:

public async getAllSymbols(type, localOnly = false) {

    const result = await super.getAllSymbols(type ?? Symbol, localOnly);

    if (!localOnly) {
        const resultArr = await Promise.all(this.dependencies.map(dep => ( 
            dep.getAllSymbols(type, localOnly)
        )));

        return new Set([...result, ...resultArr]);                         
    }

    return result;                   
}

I have removed the catch block because, in my opinion, calling code should handle the errors, if any.

Above function can be called as:

getAllSymbols(...)
   .then(result => { ... })
   .catch(error => { ... });

or you can use async-await syntax:

try {
    const result = await getAllSymbols(...);
    ...
   
} catch (error) {
    // handle error
}

Note: you may want to fix the code for the set merges. `resultArr` is not an array of symbols but a an array of sets of symbols. So you need a loop over the array to construct the final set. — Mike Lischke, Sep 07 '21 at 11:28

ESLint reports a promise misuse for code I consider ok

1 Answers1