Repository: ember-cli/ember-cli
Pull request #5928 (2016-05-24 18:57:39)

EmberApp cleanup
This PR cleans up the `EmberApp` class in several small steps. Feedback highly appreciated!

/cc @stefanpenner


Patch:
diff --git a/lib/broccoli/ember-addon.js b/lib/broccoli/ember-addon.js
index ef2f90c83..07192b109 100644
--- a/lib/broccoli/ember-addon.js
+++ b/lib/broccoli/ember-addon.js
@@ -4,7 +4,7 @@
 /**
 @module ember-cli
 */
-var mergeDefaults = require('merge-defaults');
+var defaultsDeep = require('lodash/defaultsDeep');
 var Funnel   = require('broccoli-funnel');
 var EmberApp = require('./ember-app');
 
@@ -25,12 +25,12 @@ function EmberAddon(defaults, options) {
   } else if (arguments.length === 1) {
     options = defaults;
   } else {
-    mergeDefaults(options, defaults);
+    defaultsDeep(options, defaults);
   }
 
   process.env.EMBER_ADDON_ENV = process.env.EMBER_ADDON_ENV || 'development';
 
-  this.appConstructor(mergeDefaults(options, {
+  this.appConstructor(defaultsDeep(options, {
     name: 'dummy',
     configPath: './tests/dummy/config/environment',
     trees: {
diff --git a/lib/broccoli/ember-app.js b/lib/broccoli/ember-app.js
index bd1727f9c..14ee1b2c1 100644
--- a/lib/broccoli/ember-app.js
+++ b/lib/broccoli/ember-app.js
@@ -33,13 +33,46 @@ var mergeTrees    = require('./merge-trees');
 var WatchedDir    = require('broccoli-source').WatchedDir;
 var UnwatchedDir  = require('broccoli-source').UnwatchedDir;
 
-var mergeDefaults = require('merge-defaults');
 var merge         = require('lodash/merge');
+var defaultsDeep  = require('lodash/defaultsDeep');
 var omitBy        = require('lodash/omitBy');
 var isNull        = require('lodash/isNull');
 var Funnel        = require('broccoli-funnel');
 var funnelReducer = require('broccoli-funnel-reducer');
 
+
+var DEFAULT_CONFIG = {
+  es3Safe: true,
+  storeConfigInMeta: true,
+  autoRun: true,
+  outputPaths: {
+    app: {
+      html: 'index.html'
+    },
+    tests: {
+      js: '/assets/tests.js'
+    },
+    vendor: {
+      css: '/assets/vendor.css',
+      js: '/assets/vendor.js'
+    },
+    testSupport: {
+      css: '/assets/test-support.css',
+      js: {
+        testSupport: '/assets/test-support.js',
+        testLoader: '/assets/test-loader.js'
+      }
+    }
+  },
+  minifyCSS: {
+    options: { relativeTo: 'assets' }
+  },
+  sourcemaps: {},
+  trees: {},
+  jshintrc: {},
+  addons: {}
+};
+
 module.exports = EmberApp;
 
 /**
@@ -70,22 +103,21 @@ function EmberApp(defaults, options) {
   } else if (arguments.length === 1) {
     options = defaults;
   } else {
-    mergeDefaults(options, defaults);
+    defaultsDeep(options, defaults);
   }
 
   this._initProject(options);
+  this.name = options.name || this.project.name();
 
   this.env  = EmberApp.env();
-  this.name = options.name || this.project.name();
+  this.isProduction = (this.env === 'production');
 
   this.registry = options.registry || p.defaultRegistry(this);
 
   this.bowerDirectory = this.project.bowerDirectory;
 
-  var isProduction = this.env === 'production';
-
-  this._initTestsAndHinting(options, isProduction);
-  this._initOptions(options, isProduction);
+  this._initTestsAndHinting(options);
+  this._initOptions(options);
   this._initVendorFiles();
 
   this._styleOutputFiles       = {};
@@ -109,19 +141,25 @@ function EmberApp(defaults, options) {
 }
 
 /**
+  Initializes the `tests` and `hinting` properties.
+
+  Defaults to `false` unless `ember test` was used or this is *not* a production build.
+
   @private
   @method _initTestsAndHinting
   @param {Object} options
-  @param {Boolean} isProduction
 */
-EmberApp.prototype._initTestsAndHinting = function(options, isProduction) {
-  var testsEnabledDefault = process.env.EMBER_CLI_TEST_COMMAND || !isProduction;
+EmberApp.prototype._initTestsAndHinting = function(options) {
+  var testsEnabledDefault = process.env.EMBER_CLI_TEST_COMMAND || !this.isProduction;
 
   this.tests   = options.hasOwnProperty('tests')   ? options.tests   : testsEnabledDefault;
   this.hinting = options.hasOwnProperty('hinting') ? options.hinting : testsEnabledDefault;
 };
 
 /**
+  Initializes the `project` property from `options.project` or the
+  closest Ember CLI project from the current working directory.
+
   @private
   @method _initProject
   @param {Object} options
@@ -137,100 +175,81 @@ EmberApp.prototype._initProject = function(options) {
 };
 
 /**
+  Initializes the `options` property from the `options` parameter and
+  a set of default values from Ember CLI.
+
   @private
   @method _initOptions
   @param {Object} options
-  @param {Boolean} isProduction
 */
-EmberApp.prototype._initOptions = function(options, isProduction) {
-  var babelOptions = { babel: {} };
+EmberApp.prototype._initOptions = function(options) {
+  var babelOptions = {};
 
   if (this._addonInstalled('ember-cli-babel')) {
     var amdNameResolver = require('amd-name-resolver').moduleResolve;
     babelOptions = {
-      babel: {
-        compileModules: true,
-        modules: 'amdStrict',
-        moduleIds: true,
-        resolveModuleSource: amdNameResolver
-      }
+      compileModules: true,
+      modules: 'amdStrict',
+      moduleIds: true,
+      resolveModuleSource: amdNameResolver
     };
   }
 
-  var config = {
-    es3Safe: true,
-    storeConfigInMeta: true,
-    autoRun: true,
-    outputPaths: {},
-    minifyCSS: {
-      enabled: !!isProduction,
-      options: { relativeTo: 'assets' }
-    },
-    minifyJS: {
-      enabled: !!isProduction
-    },
-    sourcemaps: {},
-    trees: {},
-    jshintrc: {},
-    addons: {}
-  };
+  var appTree = new WatchedDir(this._resolveLocal('app'));
+  var testsTree = new WatchedDir(this._resolveLocal('tests'));
 
-  this.options = mergeDefaults(options, config, babelOptions);
+  // these are contained within app/ no need to watch again
+  // (we should probably have the builder or the watcher dedup though)
+  var stylesTree = new UnwatchedDir(this._resolveLocal('app/styles'));
+  var templatesPath = this._resolveLocal('app/templates');
+  var templatesTree = existsSync(templatesPath) ? new UnwatchedDir(templatesPath) : null;
 
-  // needs a deeper merge than is provided above
-  this.options.outputPaths = mergeDefaults(this.options.outputPaths, {
-    app: {
-      html: 'index.html',
-      css: {
-        'app': '/assets/' + this.name + '.css'
-      },
-      js: '/assets/' + this.name + '.js'
+  // do not watch vendor/ or bower's default directory by default
+  var bowerTree = this.project._watchmanInfo.enabled ? this.bowerDirectory : new UnwatchedDir(this.bowerDirectory);
+  var vendorPath = this._resolveLocal('vendor');
+  var vendorTree = existsSync(vendorPath) ? new UnwatchedDir(vendorPath) : null;
+
+  var publicPath = this._resolveLocal('public');
+  var publicTree = existsSync(publicPath) ? new WatchedDir(publicPath) : null;
+
+  this.options = defaultsDeep(options, {
+    babel: babelOptions,
+    jshintrc: {
+      app: this.project.root,
+      tests: this._resolveLocal('tests')
     },
-    tests: {
-      js: '/assets/tests.js'
+    minifyCSS: {
+      enabled: this.isProduction
     },
-    vendor: {
-      css: '/assets/vendor.css',
-      js: '/assets/vendor.js'
+    minifyJS: {
+      enabled: this.isProduction
     },
-    testSupport: {
-      css: '/assets/test-support.css',
-      js: {
-        testSupport: '/assets/test-support.js',
-        testLoader: '/assets/test-loader.js'
+    outputPaths: {
+      app: {
+        css: {
+          'app': '/assets/' + this.name + '.css'
+        },
+        js: '/assets/' + this.name + '.js'
       }
+    },
+    sourcemaps: {
+      enabled: !this.isProduction,
+      extensions: ['js']
+    },
+    trees: {
+      app: appTree,
+      tests: testsTree,
+      styles: stylesTree,
+      templates: templatesTree,
+      bower: bowerTree,
+      vendor: vendorTree,
+      public: publicTree
     }
-  });
-
-  this.options.sourcemaps = mergeDefaults(this.options.sourcemaps, {
-    enabled: !isProduction,
-    extensions: ['js']
-  });
+  }, DEFAULT_CONFIG);
 
   // For now we must disable Babel sourcemaps due to unforseen
   // performance regressions.
   this.options.babel.sourceMaps = false;
-
-  this.options.trees = mergeDefaults(this.options.trees, {
-    app:       new WatchedDir(this._resolveLocal('app')),
-    tests:     new WatchedDir(this._resolveLocal('tests')),
-
-    // these are contained within app/ no need to watch again
-    // (we should probably have the builder or the watcher dedup though)
-    styles:    new UnwatchedDir(this._resolveLocal('app/styles')),
-    templates: existsSync(this._resolveLocal('app/templates')) ? new UnwatchedDir(this._resolveLocal('app/templates')) : null,
-
-    // do not watch vendor/ or bower's default directory by default
-    bower: this.project._watchmanInfo.enabled ? this.bowerDirectory : new UnwatchedDir(this.bowerDirectory),
-    vendor: existsSync(this._resolveLocal('vendor')) ? new UnwatchedDir(this._resolveLocal('vendor')) : null,
-
-    public: existsSync(this._resolveLocal('public')) ? new WatchedDir(this._resolveLocal('public')) : null
-  });
-
-  this.options.jshintrc = mergeDefaults(this.options.jshintrc, {
-    app: this.project.root,
-    tests: this._resolveLocal('tests'),
-  });
 };
 
 /**
@@ -344,6 +363,8 @@ EmberApp.prototype.concatFiles = function(tree, options) {
 };
 
 /**
+  Checks the result of `addon.isEnabled()` if it exists, defaults to `true` otherwise.
+
   @private
   @method _addonEnabled
   @param {Addon} addon
@@ -384,13 +405,11 @@ EmberApp.prototype._addonDisabledByWhitelist = function(addon) {
   @return {Boolean}
 */
 EmberApp.prototype.shouldIncludeAddon = function(addon) {
-  var enabled = this._addonEnabled(addon);
-  var disabledByBlacklist = this._addonDisabledByBlacklist(addon);
-  var disabledByWhitelist = this._addonDisabledByWhitelist(addon);
-
-  var notDisabledByBlackOrWhitelist = !(disabledByBlacklist || disabledByWhitelist);
+  if (!this._addonEnabled(addon)) {
+    return false;
+  }
 
-  return notDisabledByBlackOrWhitelist && enabled;
+  return !this._addonDisabledByBlacklist(addon) && !this._addonDisabledByWhitelist(addon);
 };
 
 
@@ -542,15 +561,9 @@ EmberApp.prototype.addonPostprocessTree = function(type, tree) {
   @return {Tree}        Processed tree
  */
 EmberApp.prototype.addonPreprocessTree = function(type, tree) {
-  var workingTree = tree;
-
-  this.project.addons.forEach(function(addon) {
-    if (addon.preprocessTree) {
-      workingTree = addon.preprocessTree(type, workingTree);
-    }
-  });
-
-  return workingTree;
+  return this.project.addons.reduce(function(workingTree, addon) {
+    return addon.preprocessTree ? addon.preprocessTree(type, workingTree) : workingTree;
+  }, tree);
 };
 
 /**
@@ -572,7 +585,7 @@ EmberApp.prototype.addonLintTree = function(type, tree) {
 
   return mergeTrees(output, {
     overwrite: true,
-    annotation: 'TreeMerger (lint)'
+    annotation: 'TreeMerger (lint ' + type + ')'
   });
 };
 
@@ -625,9 +638,11 @@ EmberApp.prototype.index = function() {
 };
 
 /**
+  Filters styles and templates from the `app` tree.
+
   @private
   @method _filterAppTree
-  @return tree
+  @return {Tree}
 */
 EmberApp.prototype._filterAppTree = function() {
   if (this._cachedFilterAppTree) {
@@ -1023,10 +1038,9 @@ EmberApp.prototype._processedEmberCLITree = function() {
 EmberApp.prototype.appAndDependencies = function() {
   var sourceTrees = [];
   var config = this._configTree();
-  var app;
   var templates = this._processedTemplatesTree();
 
-  app = this.addonPreprocessTree('js', mergeTrees([
+  var app = this.addonPreprocessTree('js', mergeTrees([
     this._processedAppTree(),
     templates
   ].concat(sourceTrees), {
@@ -1077,31 +1091,7 @@ EmberApp.prototype.appTests = function(coreTestTree) {
   var appTestTrees = [coreTestTree];
 
   if (this.hinting) {
-    var lintedApp = this.addonLintTree('app', this._filterAppTree());
-    var lintedTests = this.addonLintTree('tests', this.trees.tests);
-    var lintedTemplates = this.addonLintTree('templates', this._templatesTree());
-
-    lintedApp = new Babel(new Funnel(lintedApp, {
-      srcDir: '/',
-      destDir: this.name + '/tests/',
-      annotation: 'Funnel (lint app)'
-    }), this._prunedBabelOptions());
-
-    lintedTests = new Babel(new Funnel(lintedTests, {
-      srcDir: '/',
-      destDir: this.name + '/tests/',
-      annotation: 'Funnel (lint tests)'
-    }), this._prunedBabelOptions());
-
-    lintedTemplates = new Babel(new Funnel(lintedTemplates, {
-      srcDir: '/',
-      destDir: this.name + '/tests/',
-      annotation: 'Funnel (lint templates)'
-    }), this._prunedBabelOptions());
-
-    appTestTrees.push(lintedApp);
-    appTestTrees.push(lintedTests);
-    appTestTrees.push(lintedTemplates);
+    Array.prototype.push.apply(appTestTrees, this.lintTestTrees());
   }
 
   appTestTrees.push(this._processedEmberCLITree());
@@ -1119,6 +1109,41 @@ EmberApp.prototype.appTests = function(coreTestTree) {
   });
 };
 
+/**
+  Runs the `app`, `tests` and `templates` trees through the chain of addons that produces lint trees.
+
+  Those lint trees are afterwards funneled into the `tests` folder, babel-ified and returned as an array.
+
+  @private
+  @method lintTestsTrees
+  @return {Array}
+ */
+EmberApp.prototype.lintTestTrees = function() {
+  var lintedApp = this.addonLintTree('app', this._filterAppTree());
+  var lintedTests = this.addonLintTree('tests', this.trees.tests);
+  var lintedTemplates = this.addonLintTree('templates', this._templatesTree());
+
+  lintedApp = new Babel(new Funnel(lintedApp, {
+    srcDir: '/',
+    destDir: this.name + '/tests/',
+    annotation: 'Funnel (lint app)'
+  }), this._prunedBabelOptions());
+
+  lintedTests = new Babel(new Funnel(lintedTests, {
+    srcDir: '/',
+    destDir: this.name + '/tests/',
+    annotation: 'Funnel (lint tests)'
+  }), this._prunedBabelOptions());
+
+  lintedTemplates = new Babel(new Funnel(lintedTemplates, {
+    srcDir: '/',
+    destDir: this.name + '/tests/',
+    annotation: 'Funnel (lint templates)'
+  }), this._prunedBabelOptions());
+
+  return [lintedApp, lintedTests, lintedTemplates];
+};
+
 /**
  * @private
  * @method _addonInstalled
@@ -1130,11 +1155,11 @@ EmberApp.prototype._addonInstalled = function(addonName) {
 };
 
 /**
- * Prunes ember-cli-babel options
+ * Returns `options.babel` without the `compileModules` and `includePolyfill` properties.
  *
  * @private
  * @method _prunedBabelOptions
- * @return {Object} The pruned babel-options
+ * @return {Object} The pruned babel options
  */
 EmberApp.prototype._prunedBabelOptions = function() {
   var babelOptions = merge({}, this.options.babel);
@@ -1227,15 +1252,14 @@ EmberApp.prototype.javascript = function() {
   @return {Tree} Merged tree for styles
 */
 EmberApp.prototype.styles = function() {
-  if (this._processedStylesTree) {
-    return this._processedStylesTree;
+  if (this._cachedStylesTree) {
+    return this._cachedStylesTree;
   }
 
   if (existsSync('app/styles/' + this.name + '.css')) {
     throw new SilentError('Style file cannot have the name of the application - ' + this.name);
   }
 
-  var deprecate = this.project.ui.writeDeprecateLine.bind(this.project.ui);
   var addonTrees = this.addonTreesFor('styles');
   var external = this._processedExternalTree();
   var styles = new Funnel(this.trees.styles, {
@@ -1258,7 +1282,7 @@ EmberApp.prototype.styles = function() {
   var preprocessedStyles = preprocessCss(stylesAndVendor, '/app/styles', '/assets', options);
 
   if (this.vendorStaticStyles.length > 0) {
-    deprecate('Usage of EmberApp.vendorStaticStyles is deprecated. Please use EmberApp.import instead for the following files: \'' + this.vendorStaticStyles.join('\', \'') + '\'');
+    this.project.ui.writeDeprecateLine('Usage of EmberApp.vendorStaticStyles is deprecated. Please use EmberApp.import instead for the following files: \'' + this.vendorStaticStyles.join('\', \'') + '\'');
     this.vendorStaticStyles.forEach(function(filename) {
       this.import(filename);
     }.bind(this));
@@ -1296,7 +1320,7 @@ EmberApp.prototype.styles = function() {
     annotation: 'styles'
   });
 
-  return this._processedStylesTree = this.addonPostprocessTree('css', mergedTrees);
+  return this._cachedStylesTree = this.addonPostprocessTree('css', mergedTrees);
 };
 
 /**
@@ -1431,15 +1455,13 @@ EmberApp.prototype.dependencies = function(pkg) {
 /**
   Imports an asset into the application.
 
-  Options:
-  - type - Either 'vendor' or 'test', defaults to 'vendor'
-  - prepend - Whether or not this asset should be prepended, defaults to false
-  - destDir - Destination directory, defaults to the name of the directory the asset is in
-
   @public
   @method import
-  @param  {(Object|String)}  asset   Either a path to the asset or an object with environment names and paths as key-value pairs.
-  @param  {Object=} options Options object
+  @param {Object|String} asset Either a path to the asset or an object with environment names and paths as key-value pairs.
+  @param {Object} [options] Options object
+  @param {String} [options.type='vendor'] Either 'vendor' or 'test'
+  @param {Boolean} [options.prepend=false] Whether or not this asset should be prepended
+  @param {String} [options.destDir] Destination directory, defaults to the name of the directory the asset is in
  */
 EmberApp.prototype.import = function(asset, options) {
   var assetPath = this._getAssetPath(asset);
@@ -1448,7 +1470,7 @@ EmberApp.prototype.import = function(asset, options) {
     return;
   }
 
-  options = mergeDefaults(options || {}, {
+  options = defaultsDeep(options || {}, {
     type: 'vendor',
     prepend: false
   });
@@ -1525,14 +1547,12 @@ EmberApp.prototype._getAssetPath = function(asset) {
   /* @type {String} */
   var assetPath;
 
-  if (typeof asset === 'object') {
-    if (this.env in asset) {
-      assetPath = asset[this.env];
-    } else {
-      assetPath = asset.development;
-    }
-  } else {
+  if (typeof asset !== 'object') {
     assetPath = asset;
+  } else if (this.env in asset) {
+    assetPath = asset[this.env];
+  } else {
+    assetPath = asset.development;
   }
 
   if (!assetPath) {
diff --git a/package.json b/package.json
index f0f1be023..5edc8914c 100644
--- a/package.json
+++ b/package.json
@@ -90,7 +90,6 @@
     "lodash": "^4.12.0",
     "markdown-it": "6.0.2",
     "markdown-it-terminal": "0.0.3",
-    "merge-defaults": "^0.2.1",
     "minimatch": "^3.0.0",
     "morgan": "^1.5.2",
     "node-modules-path": "^1.0.0",


Test patch:
diff --git a/tests/unit/broccoli/ember-app-test.js b/tests/unit/broccoli/ember-app-test.js
index 2d21b3314..14d6fe03c 100644
--- a/tests/unit/broccoli/ember-app-test.js
+++ b/tests/unit/broccoli/ember-app-test.js
@@ -688,7 +688,7 @@ describe('broccoli/ember-app', function() {
           expect(inputTree).to.deep.equal(['blazorz']);
           expect(options).to.deep.equal({
             overwrite: true,
-            annotation: 'TreeMerger (lint)'
+            annotation: 'TreeMerger (lint blah)'
           });
 
           assertionsWereRun = true;