Repository: ember-cli/ember-cli
Pull request #7796 (2018-04-30 02:40:26)

Split javascript processing from grouping
With this change, application is now only responsible for assembling
javascript files in one tree, whereas packager know how to properly process
them.

Packager also learned how to deal with MU `src` directory.


Patch:
diff --git a/lib/broccoli/default-packager.js b/lib/broccoli/default-packager.js
index c59fa09bc..634c5596f 100644
--- a/lib/broccoli/default-packager.js
+++ b/lib/broccoli/default-packager.js
@@ -8,6 +8,7 @@ const mergeTrees = require('./merge-trees');
 const ConfigLoader = require('broccoli-config-loader');
 const addonProcessTree = require('../utilities/addon-process-tree');
 
+const preprocessCss = p.preprocessCss;
 const preprocessJs = p.preprocessJs;
 const preprocessTemplates = p.preprocessTemplates;
 
@@ -88,7 +89,9 @@ module.exports = class DefaultPackager {
     this._cachedPublic = null;
     this._cachedConfig = null;
     this._cachedJavascript = null;
+    this._cachedProcessedSrc = null;
     this._cachedProcessedTemplates = null;
+    this._cachedProcessedJavascript = null;
 
     this.options = options || {};
 
@@ -149,6 +152,114 @@ module.exports = class DefaultPackager {
     return this._cachedProcessedTemplates;
   }
 
+  /*
+   * Runs pre/post-processors hooks on the javascript files and returns a single
+   * tree with the processed javascript.
+   *
+   * Given a tree:
+   *
+   * ```
+   * the-best-app-ever/
+   * ├── adapters
+   * │   └── application.js
+   * ├── app.js
+   * ├── components
+   * ├── controllers
+   * ├── helpers
+   * │   ├── and.js
+   * │   ├── app-version.js
+   * │   ├── await.js
+   * │   ├── camelize.js
+   * │   ├── cancel-all.js
+   * │   ├── dasherize.js
+   * │   ├── dec.js
+   * │   ├── drop.js
+   * │   └── eq.js
+   * ...
+   * ```
+   *
+   * Returns the tree with the same structure, but with processed files.
+   *
+   * @private
+   * @method processJavascript
+   * @param {BroccoliTree} tree
+   * @return {BroccoliTree}
+  */
+  processJavascript(tree) {
+    if (this._cachedProcessedJavascript === null) {
+      let app = callAddonsPreprocessTreeHook(this.project, 'js', tree);
+
+      let preprocessedApp = preprocessJs(app, '/', this.name, {
+        registry: this.registry,
+      });
+
+      this._cachedProcessedJavascript = callAddonsPostprocessTreeHook(this.project, 'js', preprocessedApp);
+    }
+
+    return this._cachedProcessedJavascript;
+  }
+
+  /*
+   * Runs pre/post-processors hooks on the application files (javascript,
+   * styles, templates) and returns a single tree with the processed ones.
+   *
+   * Used only when `MODULE_UNIFICATION` experiment is enabled.
+   *
+   * Given a tree:
+   *
+   * ```
+   * /
+   * └── src
+   *     ├── main.js
+   *     ├── resolver.js
+   *     ├── router.js
+   *     └── ui
+   *         ├── components
+   *         ├── index.html
+   *         ├── routes
+   *         │   └── application
+   *         │   └── template.hbs
+   *         └── styles
+   *             └── app.css
+   * ```
+   *
+   * Returns the tree with the same structure, but with processed files.
+   *
+   * @private
+   * @method processSrc
+   * @param {BroccoliTree} tree
+   * @return {BroccoliTree}
+  */
+  processSrc(tree) {
+    if (this._cachedProcessedSrc === null && tree !== null) {
+      let srcAfterPreprocessTreeHook = callAddonsPreprocessTreeHook(this.project, 'src', tree);
+
+      let options = {
+        outputPaths: this.distPaths.appCssFile,
+        registry: this.registry,
+      };
+
+      // TODO: This isn't quite correct (but it does function properly in most cases),
+      // and should be re-evaluated before enabling the `MODULE_UNIFICATION` feature
+      this._srcAfterStylePreprocessing = preprocessCss(srcAfterPreprocessTreeHook, '/src/ui/styles', '/assets', options);
+
+      let srcAfterTemplatePreprocessing = preprocessTemplates(srcAfterPreprocessTreeHook, {
+        registry: this.registry,
+        annotation: 'Process Templates: src',
+      });
+
+      let srcAfterPostprocessTreeHook = callAddonsPostprocessTreeHook(this.project, 'src', srcAfterTemplatePreprocessing);
+
+      return new Funnel(srcAfterPostprocessTreeHook, {
+        srcDir: '/',
+        destDir: `${this.name}`,
+        annotation: 'Funnel: src',
+      });
+    }
+
+    return this._cachedProcessedSrc;
+  }
+
   /*
    * Given an input tree, returns a properly assembled Broccoli tree with bower
    * components.
diff --git a/lib/broccoli/ember-app.js b/lib/broccoli/ember-app.js
index a471335c1..8af17c83a 100644
--- a/lib/broccoli/ember-app.js
+++ b/lib/broccoli/ember-app.js
@@ -165,6 +165,7 @@ class EmberApp {
       scriptOutputFiles: this._scriptOutputFiles,
       distPaths: {
         appJsFile: this.options.outputPaths.app.js,
+        appCssFile: this.options.outputPaths.app.css,
         vendorJsFile: this.options.outputPaths.vendor.js,
       },
     });
@@ -984,12 +985,40 @@ class EmberApp {
     });
   }
 
-  /**
-    @private
-    @method _processedAppTree
-    @return
+  /*
+   * Gather application and add-ons javascript files and return them in a single
+   * tree.
+   *
+   * Resulting tree:
+   *
+   * ```
+   * the-best-app-ever/
+   * ├── adapters
+   * │   └── application.js
+   * ├── app.js
+   * ├── components
+   * ├── controllers
+   * ├── helpers
+   * │   ├── and.js
+   * │   ├── app-version.js
+   * │   ├── await.js
+   * │   ├── camelize.js
+   * │   ├── cancel-all.js
+   * │   ├── dasherize.js
+   * │   ├── dec.js
+   * │   ├── drop.js
+   * │   └── eq.js
+   * ...
+   * ```
+   *
+   * Note, files in the example are "made up" and will differ from the real
+   * application.
+   *
+   * @private
+   * @method getAppJavascript
+   * @return {BroccoliTree}
   */
-  _processedAppTree() {
+  getAppJavascript() {
     let appTrees = [].concat(
       this.addonTreesFor('app'),
       this.trees.app
@@ -1007,48 +1036,22 @@ class EmberApp {
     });
   }
 
-  /**
-    @private
-    @method _processedSrcTree
-    @return
+  /*
+   * Returns a `src/` directory of the application or `null` if it is not
+   * present.
+   *
+   * @private
+   * @method getSrc
+   * @return {BroccoliTree}
   */
-  _processedSrcTree() {
-    if (!experiments.MODULE_UNIFICATION) {
-      return null;
-    }
-    // styles
-    // templates
+  getSrc() {
     let rawSrcTree = this.trees.src;
 
-    if (!rawSrcTree) { return; }
+    if (!rawSrcTree) { return null; }
 
-    let srcNamespacedTree = new Funnel(rawSrcTree, {
+    return new Funnel(rawSrcTree, {
       destDir: 'src',
     });
-
-    let srcAfterPreprocessTreeHook = this.addonPreprocessTree('src', srcNamespacedTree);
-
-    let options = {
-      outputPaths: this.options.outputPaths.app.css,
-      registry: this.registry,
-    };
-
-    // TODO: This isn't quite correct (but it does function properly in most cases),
-    // and should be re-evaluated before enabling the `MODULE_UNIFICATION` feature
-    this._srcAfterStylePreprocessing = preprocessCss(srcAfterPreprocessTreeHook, '/src/ui/styles', '/assets', options);
-
-    let srcAfterTemplatePreprocessing = preprocessTemplates(srcAfterPreprocessTreeHook, {
-      registry: this.registry,
-      annotation: 'Process Templates: src',
-    });
-
-    let srcAfterPostprocessTreeHook = this.addonPostprocessTree('src', srcAfterTemplatePreprocessing);
-
-    return new Funnel(srcAfterPostprocessTreeHook, {
-      srcDir: '/',
-      destDir: `${this.name}`,
-      annotation: 'Funnel: src',
-    });
   }
 
   /*
@@ -1368,24 +1371,21 @@ class EmberApp {
     let config = this._defaultPackager.packageConfig(this.tests);
     let templates = this._defaultPackager.processTemplates(this.getTemplates());
 
-    let srcTree = this._processedSrcTree();
-    let trees = [this._processedAppTree(), srcTree, templates].filter(Boolean);
+    let trees = [this.getAppJavascript(), templates];
 
-    let app = this.addonPreprocessTree('js', mergeTrees(
-      trees,
-      {
-        annotation: 'TreeMerger (preprocessedApp & templates)',
-        overwrite: true,
-      }
-    ));
+    if (experiments.MODULE_UNIFICATION) {
+      trees.push(this._defaultPackager.processSrc(this.getSrc()));
+    }
 
-    let external = this._processedExternalTree();
-    let preprocessedApp = preprocessJs(app, '/', this.name, {
-      registry: this.registry,
+    let mergedTree = mergeTrees(trees, {
+      annotation: 'TreeMerger (preprocessedApp & templates)',
+      overwrite: true,
     });
 
-    let postprocessedApp = this.addonPostprocessTree('js', preprocessedApp);
+    let external = this._processedExternalTree();
+
     let emberCLITree = this._processedEmberCLITree();
+    let postprocessedApp = this._defaultPackager.processJavascript(mergedTree);
 
     let sourceTrees = [
       external,


Test patch:
diff --git a/tests/helpers/default-packager.js b/tests/helpers/default-packager.js
index ff15c7f1f..293c0bbc8 100644
--- a/tests/helpers/default-packager.js
+++ b/tests/helpers/default-packager.js
@@ -258,6 +258,38 @@ function setupRegistryFor(registryType, fn) {
   };
 }
 
+/*
+ * Generates the object that represents an application's registry where all
+ * file processors are stored.
+ *
+ * It takes one argument: an object with the mapping from file type to a
+ * "process" function. For example:
+ *
+ * ```
+ * {
+ *   js: tree => tree
+ * }
+ * ```
+ *
+ * @param {Object} registryMap
+ * @return {Object}
+*/
+function setupRegistry(registryMap) {
+  return {
+    load(type) {
+      if (registryMap[type]) {
+        return [{
+          toTree(tree) {
+            return registryMap[type](tree);
+          },
+        }];
+      }
+
+      return [];
+    },
+  };
+}
+
 function setupProject(rootPath) {
   const path = require('path');
   const Project = require('../../lib/models/project');
@@ -271,6 +303,7 @@ function setupProject(rootPath) {
 
 module.exports = {
   setupProject,
+  setupRegistry,
   setupRegistryFor,
   validateDefaultPackagedDist,
   getDefaultUnpackagedDist,
diff --git a/tests/unit/broccoli/default-packager/javascript-test.js b/tests/unit/broccoli/default-packager/javascript-test.js
index be2bf5f79..eec2f00ec 100644
--- a/tests/unit/broccoli/default-packager/javascript-test.js
+++ b/tests/unit/broccoli/default-packager/javascript-test.js
@@ -2,11 +2,16 @@
 
 const co = require('co');
 const expect = require('chai').expect;
+const Funnel = require('broccoli-funnel');
 const DefaultPackager = require('../../../../lib/broccoli/default-packager');
 const broccoliTestHelper = require('broccoli-test-helper');
+const defaultPackagerHelpers = require('../../../helpers/default-packager');
+const experiments = require('../../../../lib/experiments');
 
 const buildOutput = broccoliTestHelper.buildOutput;
 const createTempDir = broccoliTestHelper.createTempDir;
+const setupRegistry = defaultPackagerHelpers.setupRegistry;
+const setupRegistryFor = defaultPackagerHelpers.setupRegistryFor;
 
 describe('Default Packager: Javascript', function() {
   let input, output;
@@ -150,4 +155,201 @@ describe('Default Packager: Javascript', function() {
       'vendor.js',
     ]);
   }));
+
+  it('processes javascript according to the registry', co.wrap(function *() {
+    let defaultPackager = new DefaultPackager({
+      name: 'the-best-app-ever',
+
+      registry: setupRegistryFor('js', function(tree) {
+        return new Funnel(tree, {
+          getDestinationPath(relativePath) {
+            return relativePath.replace(/js/g, 'jsx');
+          },
+        });
+      }),
+
+      project: { addons: [] },
+    });
+
+    expect(defaultPackager._cachedProcessedJavascript).to.equal(null);
+
+    output = yield buildOutput(defaultPackager.processJavascript(input.path()));
+
+    let outputFiles = output.read();
+
+    expect(outputFiles['the-best-app-ever']).to.deep.equal({
+      'app.jsx': 'app.js',
+      components: {
+        'x-foo.jsx': 'x-foo.js',
+      },
+      config: {
+        'environment.jsx': 'environment.js',
+      },
+      'router.jsx': 'router.js',
+    });
+  }));
+
+  it('runs pre/post-process add-on hooks', co.wrap(function *() {
+    let addonPreprocessTreeHookCalled = false;
+    let addonPostprocessTreeHookCalled = false;
+
+    let defaultPackager = new DefaultPackager({
+      name: 'the-best-app-ever',
+
+      registry: setupRegistryFor('js', tree => tree),
+
+      // avoid using `testdouble.js` here on purpose; it does not have a "proxy"
+      // option, where a function call would be registered and the original
+      // would be returned
+      project: {
+        addons: [{
+          preprocessTree(type, tree) {
+            addonPreprocessTreeHookCalled = true;
+
+            return tree;
+          },
+          postprocessTree(type, tree) {
+            addonPostprocessTreeHookCalled = true;
+
+            return tree;
+          },
+        }],
+      },
+    });
+
+    expect(defaultPackager._cachedProcessedJavascript).to.equal(null);
+
+    output = yield buildOutput(defaultPackager.processJavascript(input.path()));
+
+    expect(addonPreprocessTreeHookCalled).to.equal(true);
+    expect(addonPostprocessTreeHookCalled).to.equal(true);
+  }));
+
+  if (experiments.MODULE_UNIFICATION) {
+    // there is a little code duplication here (mainly the ceremony around
+    // setting up the folder structure and disposing of it after the tests are
+    // executed; once we enable MU flag by defaul, we should clean this up a tad
+    describe('with module unification layout', function() {
+      let input, output;
+      let addonPreprocessTreeHookCalled = false;
+      let addonPostprocessTreeHookCalled = false;
+
+      let MU_LAYOUT = {
+        src: {
+          'main.js': '',
+          'resolver.js': '',
+          'router.js': '',
+        },
+        ui: {
+          components: {
+            'login-form': {
+              'component.js': '',
+              'template.hbs': '',
+            },
+          },
+          'index.html': '',
+          routes: {
+            application: {
+              'template.hbs': '',
+            },
+          },
+          styles: {
+            'app.css': '',
+          },
+        },
+      };
+      let defaultPackager = new DefaultPackager({
+        name: 'the-best-app-ever',
+
+        distPaths: {
+          appJsFile: '/assets/the-best-app-ever.js',
+          appCssFile: '/assets/the-best-app-ever.css',
+          vendorJsFile: '/assets/vendor.js',
+        },
+
+        registry: setupRegistry({
+          js: tree => tree,
+          template: tree => new Funnel(tree, {
+            getDestinationPath(relativePath) {
+              return relativePath.replace(/hbs$/g, 'js');
+            },
+          }),
+        }),
+
+        // avoid using `testdouble.js` here on purpose; it does not have a "proxy"
+        // option, where a function call would be registered and the original
+        // would be returned
+        project: {
+          addons: [{
+            preprocessTree(type, tree) {
+              addonPreprocessTreeHookCalled = true;
+
+              return tree;
+            },
+            postprocessTree(type, tree) {
+              addonPostprocessTreeHookCalled = true;
+
+              return tree;
+            },
+          }],
+        },
+      });
+
+      before(co.wrap(function *() {
+        input = yield createTempDir();
+
+        input.write(MU_LAYOUT);
+      }));
+
+      after(co.wrap(function *() {
+        yield input.dispose();
+      }));
+
+      afterEach(co.wrap(function *() {
+        yield output.dispose();
+      }));
+
+      it('processes javascript according to the registry', co.wrap(function *() {
+        expect(defaultPackager._cachedProcessedSrc).to.equal(null);
+
+        output = yield buildOutput(defaultPackager.processSrc(input.path()));
+
+        let outputFiles = output.read();
+
+        expect(outputFiles['the-best-app-ever']).to.deep.equal({
+          src: {
+            'main.js': '',
+            'resolver.js': '',
+            'router.js': '',
+          },
+          ui: {
+            components: {
+              'login-form': {
+                'component.js': '',
+                'template.js': '',
+              },
+            },
+            'index.html': '',
+            routes: {
+              application: {
+                'template.js': '',
+              },
+            },
+            styles: {
+              'app.css': '',
+            },
+          },
+        });
+      }));
+
+      it('runs pre/post-process add-on hooks', co.wrap(function *() {
+        expect(defaultPackager._cachedProcessedSrc).to.equal(null);
+
+        output = yield buildOutput(defaultPackager.processSrc(input.path()));
+
+        expect(addonPreprocessTreeHookCalled).to.equal(true);
+        expect(addonPostprocessTreeHookCalled).to.equal(true);
+      }));
+    });
+  }
 });
diff --git a/tests/unit/broccoli/ember-app-test.js b/tests/unit/broccoli/ember-app-test.js
index d2498155b..61976ca03 100644
--- a/tests/unit/broccoli/ember-app-test.js
+++ b/tests/unit/broccoli/ember-app-test.js
@@ -421,8 +421,8 @@ describe('EmberApp', function() {
           td.when(app.addonTreesFor(), { ignoreExtraArgs: true }).thenReturn(['batman']);
         });
 
-        it('_processedAppTree calls addonTreesFor', function() {
-          app._processedAppTree();
+        it('getAppJavascript calls addonTreesFor', function() {
+          app.getAppJavascript();
 
           let args = td.explain(app.addonTreesFor).calls.map(function(call) { return call.args[0]; });