0

I'm starting to work with raw promises and async/await in Node.js.

I have 2 "promis-ified" functions that I want to run in parallel, and then() perform some manipulation on the result and return the new data. The return value is always undefined, but inside the .then() the value is what I expect.

This is my function:

const fs = require('fs-promise-util').default;
/**
 * compares 2 directories to find hooks of the same name
 *
 * @return {Array} hooks that exist in remote and local directories
 */
function remoteVSlocal () {
    try {
        Promise.all([
                fs.readdir(REMOTE_HOOKS_PATH),
                fs.readdir(LOCAL_HOOKS_PATH)
        ]).then(function ([REMOTE_HOOKS, LOCAL_HOOKS]) {

            //filter out values that exist in both arrays
            //this returns a new array with the values I expect
            return LOCAL_HOOKS.filter(function (name) {
                return REMOTE_HOOKS.includes(name);
            });
        });
    } catch (err) {
        return err;
    }
}

When I call the function it returns undefined:

console.log(remoteVSlocal());

I expect a call to remoteVSlocal() to return the new array created by Array.filter().

Austin Meyers
  • 153
  • 2
  • 12

1 Answers1

2

Your function remoteVSlocal() doesn't actually return anything which is why the return value is undefined. You need to return the promise and use that returned promise when you call the function. Returning a value from an embedded .then() handler doesn't return from the function itself.

Here's a working version of your code assuming that fs.readdir() does actually return a promise (which btw is a horrible practice to take an existing standard API and change it's functionality - there are much better ways to promisify whole libraries).

Anyway, here's code that would work for you:

function remoteVSlocal () {
    return Promise.all([
            fs.readdir(REMOTE_HOOKS_PATH),
            fs.readdir(LOCAL_HOOKS_PATH)
    ]).then(function ([REMOTE_HOOKS, LOCAL_HOOKS]) {

        //filter out values that exist in both arrays
        //this returns a new array with the values I expect
        return LOCAL_HOOKS.filter(function (name) {
            return REMOTE_HOOKS.includes(name);
        });
    });
}

In addition, you need to return the promise from remoteVSlocal() and then use the returned promise:

remoteVSLocal().then(result => {
   // use result here
}).catch(err => {
   // process error here
});

Summary of changes:

  1. Return the promise from remoteVSlocal()
  2. When calling remoteVSlocal() use the returned promise with .then() and .catch().
  3. Remove try/catch since there are no synchronous exceptions here. Promises will propagate errors via a rejected promise.
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • Right, I didn't include it here but I'm using functions that wrap `fs.readdir` in promises. – Austin Meyers Jan 24 '18 at 19:15
  • @AustinMeyers - I don't know what that means. Your question shows `fs.readdir()` which does not return a promise. In addition, you need to return the promise from `remoteVSlocal()` and then use that returned promise when you call the function. – jfriend00 Jan 24 '18 at 19:18
  • I'm using a library to make `fs.readdir` a promise, this is at the top of my file: `const fs = require('fs-promise-util')` – Austin Meyers Jan 24 '18 at 19:22
  • 1
    @AustinMeyers - Well, then you also need to fix the other things I show in my answer like returning the promise from `Promise.all()` and using it. FYI, it's pretty bad form to rewrite an existing API to work differently (and then not tell anyone about that). There are much, much better ways to promisify a library (like the way Bluebird does it). I consider that horrible code to work on or maintain and just downright misleading to anyone who reads it. – jfriend00 Jan 24 '18 at 19:23
  • @AustinMeyers - OK, I rewrote my answer assuming that `fs.readdir()` returns a promise. Still think that's yuck. – jfriend00 Jan 24 '18 at 19:30
  • Thanks, thats totally correct,. I was hoping to avoid returning another promise but its clear to me now that `promise.all` returns a promise that I still have to wait for. – Austin Meyers Jan 24 '18 at 19:32
  • 1
    @AustinMeyers - Yeah once anything is async, all callers have to deal with an async result. There is no way in Javascript to change that. – jfriend00 Jan 24 '18 at 19:36
  • @AustinMeyers To _promisify_ the `fs library` in a _correct way_, you can do [this](https://stackoverflow.com/a/40681657/2954267) – robe007 Jan 24 '18 at 19:48