[react-packager] Cleanup option passing and validation

packager/react-packager/src/DependencyResolver/haste/DependencyGraph/index.js

@@ -8,15 +8,35 @@ var path = require('path');
 var isAbsolutePath = require('absolute-path');
 var debug = require('debug')('DependecyGraph');
 var util = require('util');
+var declareOpts = require('../../../lib/declareOpts');
 
 var readFile = q.nfbind(fs.readFile);
 var readDir = q.nfbind(fs.readdir);
 var lstat = q.nfbind(fs.lstat);
 var realpath = q.nfbind(fs.realpath);
 
+var validateOpts = declareOpts({
+  roots: {
+    type: 'array',
+    required: true,
+  },
+  ignoreFilePath: {
+    type: 'function',
+    default: function(){}
+  },
+  fileWatcher: {
+    type: 'object',
+    required: true,
+  },
+});
+
 function DependecyGraph(options) {
-  this._roots = options.roots;
-  this._ignoreFilePath = options.ignoreFilePath || function(){};
+  var opts = validateOpts(options);
+
+  this._roots = opts.roots;
+  this._ignoreFilePath = opts.ignoreFilePath;
+  this._fileWatcher = options.fileWatcher;
+
   this._loaded = false;
   this._queue = this._roots.slice();
   this._graph = Object.create(null);
@@ -24,7 +44,6 @@ function DependecyGraph(options) {
   this._packagesById = Object.create(null);
   this._moduleById = Object.create(null);
   this._debugUpdateEvents = [];
-  this._fileWatcher = options.fileWatcher;
 
   // Kick off the search process to precompute the dependency graph.
   this._init();

packager/react-packager/src/DependencyResolver/haste/__tests__/HasteDependencyResolver-test.js

@@ -24,7 +24,8 @@ describe('HasteDependencyResolver', function() {
       var deps = [module];
 
       var depResolver = new HasteDependencyResolver({
-        projectRoot: '/root'
+        projectRoot: '/root',
+        dev: false,
       });
 
       // Is there a better way? How can I mock the prototype instead?
@@ -79,13 +80,75 @@ describe('HasteDependencyResolver', function() {
         });
     });
 
+    pit('should get dependencies with polyfills', function() {
+      var module = {id: 'index', path: '/root/index.js', dependencies: ['a']};
+      var deps = [module];
+
+      var depResolver = new HasteDependencyResolver({
+        projectRoot: '/root',
+        dev: true,
+      });
+
+      // Is there a better way? How can I mock the prototype instead?
+      var depGraph = depResolver._depGraph;
+      depGraph.getOrderedDependencies.mockImpl(function() {
+        return deps;
+      });
+      depGraph.load.mockImpl(function() {
+        return q();
+      });
+
+      return depResolver.getDependencies('/root/index.js')
+        .then(function(result) {
+          expect(result.mainModuleId).toEqual('index');
+          expect(result.dependencies).toEqual([
+            { path: 'polyfills/prelude_dev.js',
+              id: 'polyfills/prelude_dev.js',
+              isPolyfill: true,
+              dependencies: []
+            },
+            { path: 'polyfills/require.js',
+              id: 'polyfills/require.js',
+              isPolyfill: true,
+              dependencies: ['polyfills/prelude_dev.js']
+            },
+            { path: 'polyfills/polyfills.js',
+              id: 'polyfills/polyfills.js',
+              isPolyfill: true,
+              dependencies: ['polyfills/prelude_dev.js', 'polyfills/require.js']
+            },
+            { id: 'polyfills/console.js',
+              isPolyfill: true,
+              path: 'polyfills/console.js',
+              dependencies: [
+                'polyfills/prelude_dev.js',
+                'polyfills/require.js',
+                'polyfills/polyfills.js'
+              ],
+            },
+            { id: 'polyfills/error-guard.js',
+              isPolyfill: true,
+              path: 'polyfills/error-guard.js',
+              dependencies: [
+                'polyfills/prelude_dev.js',
+                'polyfills/require.js',
+                'polyfills/polyfills.js',
+                'polyfills/console.js'
+              ],
+            },
+            module
+          ]);
+        });
+    });
+
     pit('should pass in more polyfills', function() {
       var module = {id: 'index', path: '/root/index.js', dependencies: ['a']};
       var deps = [module];
 
       var depResolver = new HasteDependencyResolver({
         projectRoot: '/root',
-        polyfillModuleNames: ['some module']
+        polyfillModuleNames: ['some module'],
+        dev: false,
       });
 
       // Is there a better way? How can I mock the prototype instead?
@@ -155,7 +218,8 @@ describe('HasteDependencyResolver', function() {
   describe('wrapModule', function() {
     it('should ', function() {
       var depResolver = new HasteDependencyResolver({
-        projectRoot: '/root'
+        projectRoot: '/root',
+        dev: false,
       });
 
       var depGraph = depResolver._depGraph;

packager/react-packager/src/DependencyResolver/haste/index.js

@@ -4,6 +4,7 @@ var path = require('path');
 var FileWatcher = require('../../FileWatcher');
 var DependencyGraph = require('./DependencyGraph');
 var ModuleDescriptor = require('../ModuleDescriptor');
+var declareOpts = require('../../lib/declareOpts');
 
 var DEFINE_MODULE_CODE =
   '__d(' +
@@ -18,22 +19,50 @@ var DEFINE_MODULE_REPLACE_RE = /_moduleName_|_code_|_deps_/g;
 
 var REL_REQUIRE_STMT = /require\(['"]([\.\/0-9A-Z_$\-]*)['"]\)/gi;
 
-function HasteDependencyResolver(config) {
-  this._fileWatcher = config.nonPersistent
+var validateOpts = declareOpts({
+  projectRoots: {
+    type: 'array',
+    required: true,
+  },
+  blacklistRE: {
+    type: 'object', // typeof regex is object
+  },
+  polyfillModuleNames: {
+    type: 'array',
+    default: [],
+  },
+  dev: {
+    type: 'boolean',
+    default: true,
+  },
+  nonPersistent: {
+    type: 'boolean',
+    default: false,
+  },
+  moduleFormat: {
+    type: 'string',
+    default: 'haste',
+  },
+});
+
+function HasteDependencyResolver(options) {
+  var opts = validateOpts(options);
+
+  this._fileWatcher = opts.nonPersistent
     ? FileWatcher.createDummyWatcher()
-    : new FileWatcher(config.projectRoots);
+    : new FileWatcher(opts.projectRoots);
 
   this._depGraph = new DependencyGraph({
-    roots: config.projectRoots,
+    roots: opts.projectRoots,
     ignoreFilePath: function(filepath) {
       return filepath.indexOf('__tests__') !== -1 ||
-        (config.blacklistRE && config.blacklistRE.test(filepath));
+        (opts.blacklistRE && opts.blacklistRE.test(filepath));
     },
     fileWatcher: this._fileWatcher
   });
 
   this._polyfillModuleNames = [
-    config.dev
+    opts.dev
       ? path.join(__dirname, 'polyfills/prelude_dev.js')
       : path.join(__dirname, 'polyfills/prelude.js'),
     path.join(__dirname, 'polyfills/require.js'),
@@ -41,7 +70,7 @@ function HasteDependencyResolver(config) {
     path.join(__dirname, 'polyfills/console.js'),
     path.join(__dirname, 'polyfills/error-guard.js'),
   ].concat(
-    config.polyfillModuleNames || []
+    opts.polyfillModuleNames || []
   );
 }
 

packager/react-packager/src/JSTransformer/Cache.js

@@ -4,19 +4,36 @@ var path = require('path');
 var version = require('../../package.json').version;
 var tmpdir = require('os').tmpDir();
 var pathUtils = require('../fb-path-utils');
+var declareOpts = require('../lib/declareOpts');
 var fs = require('fs');
 var _ = require('underscore');
 var q = require('q');
 
 var Promise = q.Promise;
 
+var validateOpts = declareOpts({
+  resetCache: {
+    type: 'boolean',
+    default: false,
+  },
+  cacheVersion: {
+    type: 'string',
+    default: '1.0',
+  },
+  projectRoots: {
+    type: 'array',
+    required: true,
+  },
+});
 module.exports = Cache;
 
-function Cache(projectConfig) {
-  this._cacheFilePath = cacheFilePath(projectConfig);
+function Cache(options) {
+  var opts = validateOpts(options);
+
+  this._cacheFilePath = cacheFilePath(opts);
 
   var data;
-  if (!projectConfig.resetCache) {
+  if (!opts.resetCache) {
     data = loadCacheSync(this._cacheFilePath);
   } else {
     data = Object.create(null);
@@ -63,7 +80,7 @@ Cache.prototype.invalidate = function(filepath){
   if(this._has(filepath)) {
     delete this._data[filepath];
   }
-}
+};
 
 Cache.prototype.end = function() {
   return this._persistCache();
@@ -114,9 +131,9 @@ function loadCacheSync(cacheFilepath) {
   return ret;
 }
 
-function cacheFilePath(projectConfig) {
-  var roots = projectConfig.projectRoots.join(',').split(path.sep).join('-');
-  var cacheVersion = projectConfig.cacheVersion || '0';
+function cacheFilePath(options) {
+  var roots = options.projectRoots.join(',').split(path.sep).join('-');
+  var cacheVersion = options.cacheVersion || '0';
   return path.join(
     tmpdir,
     [

packager/react-packager/src/JSTransformer/index.js

@@ -1,28 +1,69 @@
 
 'use strict';
 
-var os = require('os');
 var fs = require('fs');
 var q = require('q');
 var Cache = require('./Cache');
 var _ = require('underscore');
 var workerFarm = require('worker-farm');
+var declareOpts = require('../lib/declareOpts');
 
 var readFile = q.nfbind(fs.readFile);
 
 module.exports = Transformer;
 Transformer.TransformError = TransformError;
 
-function Transformer(projectConfig) {
-  this._cache = projectConfig.nonPersistent
-    ? new DummyCache() : new Cache(projectConfig);
+var validateOpts = declareOpts({
+  projectRoots: {
+    type: 'array',
+    required: true,
+  },
+  blacklistRE: {
+    type: 'object', // typeof regex is object
+  },
+  polyfillModuleNames: {
+    type: 'array',
+    default: [],
+  },
+  cacheVersion: {
+    type: 'string',
+    default: '1.0',
+  },
+  resetCache: {
+    type: 'boolean',
+    default: false,
+  },
+  dev: {
+    type: 'boolean',
+    default: true,
+  },
+  transformModulePath: {
+    type:'string',
+    required: true,
+  },
+  nonPersistent: {
+    type: 'boolean',
+    default: false,
+  },
+});
 
-  if (projectConfig.transformModulePath == null) {
+function Transformer(options) {
+  var opts = validateOpts(options);
+
+  this._cache = opts.nonPersistent
+    ? new DummyCache()
+    : new Cache({
+      resetCache: options.resetCache,
+      cacheVersion: options.cacheVersion,
+      projectRoots: options.projectRoots,
+    });
+
+  if (options.transformModulePath == null) {
     this._failedToStart = q.Promise.reject(new Error('No transfrom module'));
   } else {
     this._workers = workerFarm(
       {autoStart: true},
-      projectConfig.transformModulePath
+      options.transformModulePath
     );
   }
 }

packager/react-packager/src/Packager/index.js

@@ -10,44 +10,69 @@ var DependencyResolver = require('../DependencyResolver');
 var _ = require('underscore');
 var Package = require('./Package');
 var Activity = require('../Activity');
+var declareOpts = require('../lib/declareOpts');
 
-var DEFAULT_CONFIG = {
-  /**
-   * RegExp used to ignore paths when scanning the filesystem to calculate the
-   * dependency graph.
-   */
-  blacklistRE: null,
+var validateOpts = declareOpts({
+  projectRoots: {
+    type: 'array',
+    required: true,
+  },
+  blacklistRE: {
+    type: 'object', // typeof regex is object
+  },
+  moduleFormat: {
+    type: 'string',
+    default: 'haste',
+  },
+  polyfillModuleNames: {
+    type: 'array',
+    default: [],
+  },
+  cacheVersion: {
+    type: 'string',
+    default: '1.0',
+  },
+  resetCache: {
+    type: 'boolean',
+    default: false,
+  },
+  dev: {
+    type: 'boolean',
+    default: true,
+  },
+  transformModulePath: {
+    type:'string',
+    required: true,
+  },
+  nonPersistent: {
+    type: 'boolean',
+    default: false,
+  },
+});
 
-  /**
-   * The kind of module system/transport wrapper to use for the modules bundled
-   * in the package.
-   */
-  moduleFormat: 'haste',
+function Packager(options) {
+  var opts = this._opts = validateOpts(options);
 
-  /**
-   * An ordered list of module names that should be considered as dependencies
-   * of every module in the system. The list is ordered because every item in
-   * the list will have an implicit dependency on all items before it.
-   *
-   * (This ordering is necessary to build, for example, polyfills that build on
-   *  each other)
-   */
-  polyfillModuleNames: [],
+  opts.projectRoots.forEach(verifyRootExists);
 
-  nonPersistent: false,
-};
+  this._resolver = new DependencyResolver({
+    projectRoots: opts.projectRoots,
+    blacklistRE: opts.blacklistRE,
+    polyfillModuleNames: opts.polyfillModuleNames,
+    dev: opts.dev,
+    nonPersistent: opts.nonPersistent,
+    moduleFormat: opts.moduleFormat
+  });
 
-function Packager(projectConfig) {
-  projectConfig.projectRoots.forEach(verifyRootExists);
-
-  this._config = Object.create(DEFAULT_CONFIG);
-  for (var key in projectConfig) {
-    this._config[key] = projectConfig[key];
-  }
-
-  this._resolver = new DependencyResolver(this._config);
-
-  this._transformer = new Transformer(projectConfig);
+  this._transformer = new Transformer({
+    projectRoots: opts.projectRoots,
+    blacklistRE: opts.blacklistRE,
+    cacheVersion: opts.cacheVersion,
+    resetCache: opts.resetCache,
+    dev: opts.dev,
+    transformModulePath: opts.transformModulePath,
+    nonPersistent: opts.nonPersistent,
+  });
 }
 
 Packager.prototype.kill = function() {
@@ -92,7 +117,7 @@ Packager.prototype.package = function(main, runModule, sourceMapUrl) {
 
 Packager.prototype.invalidateFile = function(filePath) {
   this._transformer.invalidateFile(filePath);
-}
+};
 
 Packager.prototype.getDependencies = function(main) {
   return this._resolver.getDependencies(main);
@@ -103,7 +128,7 @@ Packager.prototype._transformModule = function(module) {
   return this._transformer.loadFileAndTransform(
     ['es6'],
     path.resolve(module.path),
-    this._config.transformer || {}
+    this._opts.transformer || {}
   ).then(function(transformed) {
     return _.extend(
       {},

packager/react-packager/src/Server/__tests__/Server-test.js

@@ -6,8 +6,6 @@ jest.setMock('worker-farm', function(){ return function(){}; })
     .dontMock('url')
     .dontMock('../');
 
-
-var server = require('../');
 var q = require('q');
 
 describe('processRequest', function(){
@@ -45,17 +43,17 @@ describe('processRequest', function(){
   beforeEach(function(){
     Activity = require('../../Activity');
     Packager = require('../../Packager');
-    FileWatcher = require('../../FileWatcher')
+    FileWatcher = require('../../FileWatcher');
 
-    Packager.prototype.package = function(main, runModule, sourceMapUrl) {
+    Packager.prototype.package = function() {
       return q({
-        getSource: function(){
-          return "this is the source"
+        getSource: function() {
+          return 'this is the source';
         },
         getSourceMap: function(){
-          return "this is the source map"
-        }
-      })
+          return 'this is the source map';
+        },
+      });
     };
 
     FileWatcher.prototype.on = watcherFunc;

packager/react-packager/src/Server/index.js

@@ -1,26 +1,56 @@
 var url = require('url');
 var path = require('path');
-var FileWatcher = require('../FileWatcher')
+var declareOpts = require('../lib/declareOpts');
+var FileWatcher = require('../FileWatcher');
 var Packager = require('../Packager');
 var Activity = require('../Activity');
 var q = require('q');
 
 module.exports = Server;
 
+var validateOpts = declareOpts({
+  projectRoots: {
+    type: 'array',
+    required: true,
+  },
+  blacklistRE: {
+    type: 'object', // typeof regex is object
+  },
+  moduleFormat: {
+    type: 'string',
+    default: 'haste',
+  },
+  polyfillModuleNames: {
+    type: 'array',
+    default: [],
+  },
+  cacheVersion: {
+    type: 'string',
+    default: '1.0',
+  },
+  resetCache: {
+    type: 'boolean',
+    default: false,
+  },
+  dev: {
+    type: 'boolean',
+    default: true,
+  },
+  transformModulePath: {
+    type:'string',
+    required: true,
+  },
+  nonPersistent: {
+    type: 'boolean',
+    default: false,
+  },
+});
+
 function Server(options) {
-  this._projectRoots = options.projectRoots;
+  var opts = validateOpts(options);
+  this._projectRoots = opts.projectRoots;
   this._packages = Object.create(null);
-  this._packager = new Packager({
-    projectRoots: options.projectRoots,
-    blacklistRE: options.blacklistRE,
-    polyfillModuleNames: options.polyfillModuleNames || [],
-    runtimeCode: options.runtimeCode,
-    cacheVersion: options.cacheVersion,
-    resetCache: options.resetCache,
-    dev: options.dev,
-    transformModulePath: options.transformModulePath,
-    nonPersistent: options.nonPersistent,
-  });
+  this._packager = new Packager(opts);
 
   this._fileWatcher = options.nonPersistent
     ? FileWatcher.createDummyWatcher()
@@ -35,10 +65,10 @@ Server.prototype._onFileChange = function(type, filepath, root) {
   this._packager.invalidateFile(absPath);
   // Make sure the file watcher event runs through the system before
   // we rebuild the packages.
-  setImmediate(this._rebuildPackages.bind(this, absPath))
+  setImmediate(this._rebuildPackages.bind(this, absPath));
 };
 
-Server.prototype._rebuildPackages = function(filepath) {
+Server.prototype._rebuildPackages = function() {
   var buildPackage = this._buildPackage.bind(this);
   var packages = this._packages;
   Object.keys(packages).forEach(function(key) {

packager/react-packager/src/lib/__mocks__/declareOpts.js

@@ -0,0 +1,10 @@
+module.exports = function(declared) {
+  return function(opts) {
+    for (var p in declared) {
+      if (opts[p] == null && declared[p].default != null){
+        opts[p] = declared[p].default;
+      }
+    }
+    return opts;
+  };
+};

packager/react-packager/src/lib/__tests__/declareOpts-test.js

@@ -0,0 +1,82 @@
+jest.autoMockOff();
+
+var declareOpts = require('../declareOpts');
+
+describe('declareOpts', function() {
+  it('should declare and validate simple opts', function() {
+    var validate = declareOpts({
+      name: {
+        required: true,
+        type: 'string',
+      },
+      age: {
+        type: 'number',
+        default: 21,
+      }
+    });
+    var opts = validate({ name: 'fooer' });
+
+    expect(opts).toEqual({
+      name: 'fooer',
+      age: 21
+    });
+  });
+
+  it('should work with complex types', function() {
+    var validate = declareOpts({
+      things: {
+        required: true,
+        type: 'array',
+      },
+      stuff: {
+        type: 'object',
+        required: true,
+      }
+    });
+
+    var opts = validate({ things: [1, 2, 3], stuff: {hai: 1} });
+    expect(opts).toEqual({
+      things: [1,2,3],
+      stuff: {hai: 1},
+    });
+  });
+
+  it('should throw when a required option is not present', function() {
+    var validate = declareOpts({
+      foo: {
+        required: true,
+        type: 'number',
+      }
+    });
+
+    expect(function() {
+      validate({});
+    }).toThrow('Error validating module options: foo is required');
+  });
+
+  it('should throw on invalid type', function() {
+    var validate = declareOpts({
+      foo: {
+        required: true,
+        type: 'number'
+      }
+    });
+
+    expect(function() {
+      validate({foo: 'lol'});
+    }).toThrow('Error validating module options: foo must be a number');
+  });
+
+  it('should throw on extra options', function() {
+    var validate = declareOpts({
+      foo: {
+        required: true,
+        type: 'number',
+      }
+    });
+
+    expect(function() {
+      validate({foo: 1, lol: 1});
+    }).toThrow('Error validating module options: lol is not allowed');
+  });
+});

packager/react-packager/src/lib/declareOpts.js

@@ -0,0 +1,53 @@
+/**
+ * Declares, validates and defaults options.
+ * var validate = declareOpts({
+ *   foo: {
+ *     type: 'bool',
+ *     required: true,
+ *   }
+ * });
+ *
+ * var myOptions = validate(someOptions);
+ */
+
+var Joi = require('joi');
+
+module.exports = function(descriptor) {
+  var joiKeys = {};
+  Object.keys(descriptor).forEach(function(prop) {
+    var record = descriptor[prop];
+    if (record.type == null) {
+      throw new Error('Type is required');
+    }
+
+    if (record.type === 'function') {
+      record.type = 'func';
+    }
+
+    var propValidator = Joi[record.type]();
+
+    if (record.required) {
+      propValidator = propValidator.required();
+    }
+
+    if (record.default) {
+      propValidator = propValidator.default(record.default);
+    }
+
+    joiKeys[prop] = propValidator;
+  });
+
+  var schema = Joi.object().keys(joiKeys);
+
+  return function(opts) {
+    var res = Joi.validate(opts, schema, {
+      abortEarly: true,
+      allowUnknown: false,
+    });
+
+    if (res.error) {
+      throw new Error('Error validating module options: ' + res.error.message);
+    }
+    return res.value;
+  };
+};