3

I know I'm returning early in the following function, how can I chain the recursive promises to my result?

My goal is to get an array of list of files in the directory and all of it's subdirectories. Array is single dimension, I'm using concat in this example.

function iterate(body) {
    return new Promise(function(resolve, reject){
        var list = [];
        fs.readdir(body.path, function(error, list){
            list.forEach(function(file){
                file = path.resolve(body.path, file);
                fs.stat(file, function(error, stat){
                    console.log(file, stat.isDirectory());
                    if(stat.isDirectory()) {
                        return iterate({path: file})
                            .then(function(result){
                                list.concat(result);
                            })
                            .catch(reject);
                    } else {
                        list.push(file);
                    }
                })
            });
            resolve(list);
        });
    });
};
user2727195
  • 7,122
  • 17
  • 70
  • 118

1 Answers1

12

There are numerous mistakes in your code. A partial list:

  1. .concat() returns a new array, so list.concat(result) by itself doesn't actually do anything.

  2. You're calling resolve() synchronously and not waiting for all async operations to be completed.

  3. You're trying to recursively return from deep inside several nested async callbacks. You can't do that. That won't get the results back anywhere.

I find this a ton easier to use by using a promisified version of the fs module. I use Bluebird to create that and then you can do this:

const path = require('path');
var Promise = require('bluebird');
const fs = Promise.promisifyAll(require('fs'));

function iterate(dir) {
    return fs.readdirAsync(dir).map(function(file) {
        file = path.resolve(dir, file);
        return fs.statAsync(file).then(function(stat) {
            if (stat.isDirectory()) {
                return iterate(file);
            } else {
                return file;
            }
        })
    }).then(function(results) {
        // flatten the array of arrays
        return Array.prototype.concat.apply([], results);
    });
}

Note: I changed iterate() to just take the initial path so it's more generically useful. You can just pass body.path to it initially to adapt.

Here's a version using generic ES6 promises:

const path = require('path');
const fs = require('fs');

fs.readdirAsync = function(dir) {
    return new Promise(function(resolve, reject) {
        fs.readdir(dir, function(err, list) {
            if (err) {
                reject(err);
            } else {
                resolve(list);
            }
        });
    });
}

fs.statAsync = function(file) {
    return new Promise(function(resolve, reject) {
        fs.stat(file, function(err, stat) {
            if (err) {
                reject(err);
            } else {
                resolve(stat);
            }
        });
    });
}


function iterate2(dir) {
    return fs.readdirAsync(dir).then(function(list) {
        return Promise.all(list.map(function(file) {
            file = path.resolve(dir, file);
            return fs.statAsync(file).then(function(stat) {
                if (stat.isDirectory()) {
                    return iterate2(file);
                } else {
                    return file;
                }
            });
        }));
    }).then(function(results) {
        // flatten the array of arrays
        return Array.prototype.concat.apply([], results);
    });
}

iterate2(".").then(function(results) {
    console.log(results);
});

Here's a version that adds a customizable filter function:

function iterate2(dir, filterFn) {
    // default filter function accepts all files
    filterFn = filterFn || function() {return true;}
    return fs.readdirAsync(dir).then(function(list) {
        return Promise.all(list.map(function(file) {
            file = path.resolve(dir, file);
            return fs.statAsync(file).then(function(stat) {
                if (stat.isDirectory()) {
                    return iterate2(file, filterFn);
                } else {
                    return filterFn(file)? file : "";
                }
            });
        })).then(function(results) {
            return results.filter(function(f) {
                return !!f;
            });
        });
    }).then(function(results) {
        // flatten the array of arrays
        return Array.prototype.concat.apply([], results);
    });
}

// example usage
iterate2(".", function(f) {
    // filter out 
    return !(/(^|\/)\.[^\/\.]/g).test(f);
}).then(function(results) {
    console.log(results);
});
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • I can't use bluebird, this project is dependency free and using native promises. – user2727195 Jul 11 '16 at 19:14
  • @user2727195 - Well, I'd suggest you manually promisify the `fs` operations you are doing and use those because promises are THE way to coordinate lots of async operations. It's kind of silly to use node.js and not use any external modules. That's the main strength of node.js so you can use pre-built modules that solve problems you have rather than re-inventing the wheel. Unless this is homework... I can work on a generic version, but it's a LOT more work. – jfriend00 Jul 11 '16 at 19:15
  • @user2727195 - I added a version using generic node.js and no add-on libraries. – jfriend00 Jul 11 '16 at 19:25
  • :) at least can you give me some directions to work on, and I'll try to make it happen. you can see even for zip, it's dependency free, I've used system process zip. http://stackoverflow.com/questions/38298466/zip-the-folder-using-built-in-modules. – user2727195 Jul 11 '16 at 19:26
  • @user2727195 - I've already included it in my answer. With the advent of NPM and the ease of specifying depencies on other modules, "dependency free" is a silly notion in my opinion. That just requires you to reinvent what others have already done which is 2/3 of the reason to use node.js in the first place (in my opinion). – jfriend00 Jul 11 '16 at 19:27
  • our pm is very careful with dependencies and all of their versions, upgrades, continuously change in the API's and forcing us to adapt and change our code. the goal is always to get the code and specs right and freeze the code there. by using native language features as much as you can. – user2727195 Jul 11 '16 at 19:29
  • @user2727195 - With NPM, you can specify a dependency on a specific version of an external module and only upgrade that module when you choose to. It's frozen until you change the version number you want to include yourself. IMO, it's ridiculously counter-productive to ban the use of external modules in node.js programming. There are tens of thousands of pre-built modules that have already solved zillions of problems. Why rebuild all that from scratch? Never mind. This is off-topic. I've given you the verbose version that rewrites stuff other people have already done for us. – jfriend00 Jul 11 '16 at 19:32
  • true, you're right but it always throws us whenever we see a list of issues of the dependency on a github repo, and they are continuously reinventing who they are. dragging developers from one side of the hill to the other. – user2727195 Jul 11 '16 at 19:35
  • I've this regex to check for hidden files like .DS_Store, where do you recommend to put this? `if(!(/(^|\/)\.[^\/\.]/g).test(file))` – user2727195 Jul 11 '16 at 20:44
  • or maybe I should do filtering for hidden files at the end of the total result? – user2727195 Jul 11 '16 at 20:52
  • @user2727195 - I think we've long since answered your original question here, but I added one more version with a custom filter function you can pass in. But, yes you could always filter the array at the end of the whole process too. – jfriend00 Jul 11 '16 at 21:43
  • `return Array.prototype.concat.apply([], results);` is the mose brilliant to collect results! wonderful code, but hard to understand – jiajianrong Jun 28 '19 at 08:33