fix(cli): cleanup build process for addons

denali-js · Oct 25, 2016 · 5fda03d · 5fda03d
1 parent 16d2a2a
commit 5fda03d
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 51 deletions.
diff --git a/cli/builder.js b/cli/builder.js
@@ -14,13 +14,22 @@ export default class BuildOrigin {
     this.dir = dir;
     this.project = project;
     this.lint = options.lint;
-    this.isDummy = options.isDummy;
+    this.environment = options.environment;
+
     this.pkg = require(path.join(this.dir, 'package.json'));
   }
 
+  get isAddon() {
+    return this.pkg.keywords && this.pkg.keywords.includes('denali-addon');
+  }
+
+  get isProjectRoot() {
+    return this.project.dir === this.dir;
+  }
+
   sourceDirs() {
     let dirs = [ 'app', 'config', 'lib' ];
-    if (this.project.environment === 'test') {
+    if (this.environment === 'test') {
       dirs.push('test');
     }
     return dirs;
@@ -48,13 +57,6 @@ export default class BuildOrigin {
     };
   }
 
-  treeForTest() {
-    if (this.isDummy) {
-      return '..';
-    }
-    return 'test';
-  }
-
   toTree() {
     let dirs = this.sourceDirs();
 
@@ -90,9 +92,22 @@ export default class BuildOrigin {
     sourceTrees.push(new PackageTree(this.pkg));
     let mergedSource = new MergeTree(sourceTrees, { overwrite: true });
 
-    // If this is an addon builder, move the source into a node_modules folder
-    if (this.project.appBuilder !== this) {
-      mergedSource = new Funnel(mergedSource, { destDir: `node_modules/${ this.pkg.name }` });
+    if (this.isAddon) {
+      // If this is an addon, the project root, and we are building
+      // for "test", we want to move the tests from the addon up to the dummy
+      // app so they are actually run, but move everything else into
+      // node_modules like normal.
+      if (this.isProjectRoot && this.environment === 'test') {
+        let addonTests = new Funnel(mergedSource, { include: [ 'test/**/*' ] });
+        let addonWithoutTests = new Funnel(mergedSource, {
+          exclude: [ 'test/**/*' ],
+          destDir: `node_modules/${ this.pkg.name }`
+        });
+        mergedSource = new MergeTree([ addonWithoutTests, addonTests ]);
+      // If it's just any old addon, move it into the local node_modules folder
+      } else {
+        mergedSource = new Funnel(mergedSource, { destDir: `node_modules/${ this.pkg.name }` });
+      }
     }
 
     return mergedSource;

diff --git a/cli/project.js b/cli/project.js
@@ -6,6 +6,7 @@ import broccoli from 'broccoli';
 import { Watcher } from 'broccoli/lib';
 import rimraf from 'rimraf';
 import MergeTree from 'broccoli-merge-trees';
+import printSlowNodes from 'broccoli-slow-trees';
 import { sync as copyDereferenceSync } from 'copy-dereference';
 import chalk from 'chalk';
 import ui from './ui';
@@ -20,33 +21,27 @@ export default class Project {
   constructor(options = {}) {
     this.environment = options.environment || 'development';
     this.dir = options.dir || process.cwd();
-    this.projectDir = this.dir;
     this.lint = options.lint;
     this.audit = options.audit;
 
-    let pkg = require(path.join(this.dir, 'package.json'));
-    if (pkg.keywords && pkg.keywords.includes('denali-addon')) {
-      this.addons = discoverAddons(this.dir, {
-        environment: this.environment,
-        preseededAddons: [ this.dir ]
-      });
-      this.isAddon = true;
-      this.dir = path.join(this.dir, 'test/dummy');
-    } else {
-      this.addons = discoverAddons(this.dir, {
-        environment: this.environment
-      });
-    }
-
+    this.pkg = require(path.join(this.dir, 'package.json'));
+    this.addons = discoverAddons(this.dir, {
+      environment: this.environment,
+      preseededAddons: this.isAddon ? [ this.dir ] : null
+    });
     this.buildTree = this._createBuildTree();
   }
 
+  get isAddon() {
+    return this.pkg.keywords && this.pkg.keywords.includes('denali-addon');
+  }
+
   build(outputDir = 'dist') {
     this.startTime = process.hrtime();
     let broccoliBuilder = new broccoli.Builder(this.buildTree);
     return broccoliBuilder.build()
       .then((results) => {
-        this._finishBuild(results.directory, outputDir);
+        this._finishBuild(results, outputDir);
       }).finally(() => {
         return broccoliBuilder.cleanup();
       }).catch((err) => {
@@ -75,7 +70,7 @@ export default class Project {
     process.on('SIGTERM', onExit);
 
     watcher.on('change', (results) => {
-      this._finishBuild(results.directory, outputDir);
+      this._finishBuild(results, outputDir);
       onBuild();
     });
 
@@ -140,14 +135,34 @@ export default class Project {
   }
 
   _createBuildTree() {
-    // Create a builder for the app itself first, so that addons can modify it
-    this.appBuilder = this._builderForDir(this.dir, { isDummy: this.isAddon });
-    // Create builders for each addon
-    let builders = this.addons.map((dir) => this._builderForDir(dir));
-    // Add the app builder last to ensure it overwrites any addon trees
-    builders.push(this.appBuilder);
-    let buildTrees = builders.map((builder) => builder.toTree());
-    return new MergeTree(buildTrees, { overwrite: true });
+    let appPath = this.isAddon ? path.join(this.dir, 'test', 'dummy') : this.dir;
+    let app = this._builderForDir(appPath, {
+      lint: this.lint,
+      environment: this.environment
+    });
+    let addons = this.addons.map((addonDir) => {
+      // By default, addons are built with the same environment as the consuming
+      // app, but with two exceptions:
+      // 1. If the consuming app env is "test", the addons are built with an env
+      //    of "development", since we only want to test the consuming app.
+      let environment = this.environment === 'test' ? 'development' : this.environment;
+      // 2. If this is an addon & dummy app, then we make a special case
+      //    exception for the addon itself. So if we run `denali test` in an
+      //    addon folder, it will build the dummy app and the addon under test
+      //    with an env of "test" (but all *other* addons will be "development")
+      if (addonDir === this.dir) {
+        environment = this.environment;
+      }
+      return this._builderForDir(addonDir, {
+        lint: addonDir === this.dir ? this.lint : false,
+        environment,
+        app
+      });
+    });
+
+    let trees = addons.map((addon) => addon.toTree());
+    trees.push(app.toTree());
+    return new MergeTree(trees, { overwrite: true });
   }
 
   _builderForDir(dir, options = {}) {
@@ -158,16 +173,16 @@ export default class Project {
     } else {
       BuilderClass = Builder;
     }
-    return new BuilderClass(dir, this, Object.assign({ lint: this.lint }, options));
+    return new BuilderClass(dir, this, options);
   }
 
   _finishBuild(results, outputDir) {
     rimraf.sync(outputDir);
-    copyDereferenceSync(results, outputDir);
-    this._linkDependencies(path.join(this.projectDir, 'node_modules'), path.join(outputDir, 'node_modules'));
-    ui.info('Build successful');
+    copyDereferenceSync(results.directory, outputDir);
+    this._linkDependencies(path.join(this.dir, 'node_modules'), path.join(outputDir, 'node_modules'));
+    printSlowNodes(results.graph);
     if (this.audit) {
-      let pkg = path.join(this.projectDir, 'package.json');
+      let pkg = path.join(this.dir, 'package.json');
       nsp.check({ package: pkg }, (err, vulnerabilities) => {
         if (err && [ 'ENOTFOUND', 'ECONNRESET' ].includes(err.code)) {
           ui.warn('Error trying to scan package dependencies for vulnerabilities with nsp, unable to reach server. Skipping scan ...');

diff --git a/package.json b/package.json
@@ -69,6 +69,7 @@
     "broccoli-lint-eslint": "^2.7.0",
     "broccoli-merge-trees": "^1.1.4",
     "broccoli-plugin": "^1.2.2",
+    "broccoli-slow-trees": "1.1.0",
     "broccoli-stew": "^1.4.0",
     "chalk": "^1.1.1",
     "cli-table2": "^0.2.0",

diff --git a/yarn.lock b/yarn.lock
@@ -1202,7 +1202,7 @@ broccoli-plugin, broccoli-plugin@^1.0.0:
     rimraf "^2.3.4"
     symlink-or-copy "^1.0.1"
 
-broccoli-slow-trees@^1.0.0:
+broccoli-slow-trees@^1.0.0, [email protected]:
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/broccoli-slow-trees/-/broccoli-slow-trees-1.1.0.tgz#426c5724e008107e4573f73e8a9ca702916b78f7"
 
@@ -1569,7 +1569,7 @@ command-exists:
   version "1.0.2"
   resolved "https://registry.yarnpkg.com/command-exists/-/command-exists-1.0.2.tgz#1f0e104ed5f252d2b857cdd69b47cfedc95756e9"
 
-commander@^2.0.0, commander@^2.5.0, commander@^2.8.1, commander@^2.9.0:
+commander@^2.0.0, commander@^2.5.0, commander@^2.8.1, commander@^2.9.0, [email protected]:
   version "2.9.0"
   resolved "https://registry.yarnpkg.com/commander/-/commander-2.9.0.tgz#9c99094176e12240cb22d6c5146098400fe0f7d4"
   dependencies:
@@ -1857,7 +1857,13 @@ cors@^2.7.1:
   dependencies:
     vary "^1"
 
-cross-spawn@^4:
+create-error-class@^3.0.1:
+  version "3.0.2"
+  resolved "https://registry.yarnpkg.com/create-error-class/-/create-error-class-3.0.2.tgz#06be7abef947a3f14a30fd610671d401bca8b7b6"
+  dependencies:
+    capture-stack-trace "^1.0.0"
+
+cross-spawn@^4, cross-spawn@^4.0.0:
   version "4.0.2"
   resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-4.0.2.tgz#7b9247621c23adfdd3856004a823cbe397424d41"
   dependencies:
@@ -3746,7 +3752,7 @@ [email protected]:
     lru-cache "2"
     sigmund "~1.0.0"
 
-minimist@^1.1.0, minimist@^1.1.3, minimist@^1.2.0, [email protected]:
+minimist@^1.1.0, minimist@^1.1.3, minimist@^1.2.0:
   version "1.2.0"
   resolved "https://registry.yarnpkg.com/minimist/-/minimist-1.2.0.tgz#a35008b20f41383eec1fb914f4cd5df79a264284"
 
@@ -4552,11 +4558,7 @@ repeating@^2.0.0:
   dependencies:
     is-finite "^1.0.0"
 
-replaceall@^0.1.6:
-  version "0.1.6"
-  resolved "https://registry.yarnpkg.com/replaceall/-/replaceall-0.1.6.tgz#81d81ac7aeb72d7f5c4942adf2697a3220688d8e"
-
-request@^2.65.0, [email protected], [email protected]:
+request@^2.65.0, [email protected]:
   version "2.75.0"
   resolved "https://registry.yarnpkg.com/request/-/request-2.75.0.tgz#d2b8268a286da13eaa5d01adf5d18cc90f657d93"
   dependencies: