Support promises

[TwitchBronBron]

It would be great if node-glob-all returned a promise for those of us who are moving away from the node callback model. Since nobody is expecting a return value from globAll, I would imagine this would be a pretty safe change to implement.

I'm willing to do the work if you're interested in accepting the pull request.

I noticed that globAll returns the GlobAll instance, but this is undocumented. So with the assumption that nobody should actually be using the returned instance of GlobAll, this code would do the trick (untested):

var globAll = function (array, opts, callback) {
    return new Promise(function (resolve, reject) {
        var g = new GlobAll(false, array, opts, function (error, results) {
            if (error) {
                reject(error);
                return (typeof callback === "function") ? callback(error) : null;
            } else {
                resolve(results);
                return (typeof callback === "function") ? callback(null, results) : null;
            }
        });
        g.run(); //async, so results are empty
    });
};

If you must continue to return that GlobAll instance, then perhaps something like this would work, where it keeps the existing functionality, and only returns a promise when you do not provide a callback.

var globAll = function (array, opts, callback) {
    if (callback) {
        var g = new GlobAll(false, array, opts, callback);
        g.run(); //async, so results are empty
        return g;
    } else {
        return new Promise(function (resolve, reject) {
            var g = new GlobAll(false, array, opts, function (err, results) {
                if (err) {
                    reject(error);
                } else {
                    resolve(results);
                }
            });
            g.run(); //async, so results are empty
        });
    }
};

Usage

//since we didn't provide the callback parameter, globAll returns a promise
globAll('someGlob').then(function (results) {
    //do something on success
}, function () { 
    //do something on error
});

[TwitchBronBron]

@jpillora any thoughts on this?

[jpillora]

Hey Bronley, 9 months! Very patient :)

I wonder if the GlobAll instance could extend Promise, and maintain existing functionality (emitting match and end events)?

Otherwise I’m happy to change the return value to a Promise and bump the major version of this package👍

PR away!

[TwitchBronBron]

So, another 7 months have passed. I ended up switching glob libraries for a different reason for that project, but I'm back now because I need it for a new project :D

I'm going to put together a PR for this with the GlobAll instance extending promise like you suggested. But before I do that, I wanted to verify with you are ok with requiring that glob-all depends on Promise existing? Because once we use Promise as the inheriting prototype, GlobAll will throw exceptions if Promise doesn't exist (i.e. I think NodeJS started supporting promises around 0.12). This is probably not an issue, but just wanted to verify.

If that concerns you, we could probably do something like this instead, which defers the promise usage to only once actually requested:

Object.defineProperty(GlobAll.prototype, 'promise', {
    get(){
       if('undefined' === typeof Promise){
            throw new Error('Promise is not supported in your environment');
        } else{
              //return the promise instance somehow
        }
    }
});

By doing this, we could allow glob-all to still work the same in all environments, and upgrade to using promises only when explicitly requested.

var g = globAll(...);
var result = await g.promise;

Thoughts?

[TwitchBronBron]

@jpillora what do you think of this approach?

[TwitchBronBron]

@jpillora thoughts on my proposal?

[GeorgeTaveras1231]

glob@^9 supports promises and array of globs. Since this library is based on glob, maybe it makes sense to deprecate this library? 🤔

[TwitchBronBron]

Yeah, I think that makes sense.