refactor parser to be less sketchy

lib/bundles/index.js

@@ -22,38 +22,33 @@ var _bundlesDir = fs.readdirSync('bundles/');
 var promises = [];
 _bundlesDir.forEach(function(bundleFolderName) {
     if (!fs.lstatSync('bundles/' + bundleFolderName).isDirectory()) return;
-    var parsePromise = parse(bundleFolderName);
 
-    // If key files are missing/incompatible, parsePromise is null, not a promise
-    // This might be a dumb antipattern, or it might be a genius optimization
-    // Could really go either way with me
-    // UPDATE: Jan 10, 2015: Yeah this is definitely a dumb antipattern.
-    // TODO: Make this not a dumb antipattern. Function "parse" should ALWAYS return a promise.
-    if (parsePromise) {
-        var postParsePromise = parsePromise.then(function(bundle) {
-            if (!bundle) return;
-            log.trace('Parsed bundle %s', bundle.name);
-            _bundles.push(bundle);
-        });
-        promises.push(postParsePromise);
-    }
+    // Begin parsing each folder. Push each parse promise onto an array.
+    promises.push(
+        parse(bundleFolderName)
+            .then(function(bundle) {
+                if (!bundle) return;
+                log.trace('Parsed bundle %s', bundle.name);
+                _bundles.push(bundle);
+            })
+    );
 });
 
+// Once all the initial parse promises have been resolved, start up the bundle watcher
 Q.all(promises)
     .then(function() {
         watcher.on('bundleChanged', function (name) {
-            var parsePromise = parse(name);
-
-            if (parsePromise) {
-                parsePromise.then(function(bundle) {
-                    log.info('%s was changed, and has been reloaded from disk', name);
-                    exports.add(bundle);
+            parse(name)
+                .then(function(bundle) {
+                    if (bundle) {
+                        log.info('%s was changed, and has been reloaded from disk', name);
+                        exports.add(bundle);
+                    } else {
+                        log.info('%s\'s nodecg.json can no longer be found on disk, ' +
+                        'assuming the bundle has been deleted or moved', name);
+                        exports.remove(name);
+                    }
                 });
-            } else {
-                log.info('%s\'s nodecg.json can no longer be found on disk, ' +
-                         'assuming the bundle has been deleted or moved', name);
-                exports.remove(name);
-            }
         });
 
         exports.emit('allLoaded', exports.all());

lib/bundles/parser.js

@@ -14,13 +14,16 @@ var exec = require('child_process').exec;
 var pjson = require('../../package.json');
 
 module.exports = function parse(bundleName) {
+    var deferred = Q.defer();
+
     // resolve the path to the bundle and its nodecg.json
     var dir = path.resolve(__dirname, '../../bundles/', bundleName);
     var manifestPath = path.join(dir, 'nodecg.json');
 
     // Return null if nodecg.json doesn't exist
     if (!fs.existsSync(manifestPath)) {
-        return null;
+        deferred.resolve();
+        return deferred.promise;
     }
 
     log.trace('Discovered bundle in folder bundles/%s', bundleName);
@@ -33,7 +36,8 @@ module.exports = function parse(bundleName) {
     if (!isCompatible(bundle.nodecgDependency)) {
         log.warn('Did not load %s as it requires NodeCG %s. Current version is %s',
             bundle.name, bundle.nodecgDependency, pjson.version);
-        return null;
+        deferred.resolve();
+        return deferred.promise;
     }
 
     // If there is a config file for this bundle, parse it