Repository: ember-cli/ember-cli
Pull request #6908 (2017-03-27 06:17:25)

Several fixes for the module unification feature flag
Several fixes here for the module unification flag, written mostly by @rwjblue with my very watchful supervision.

* Avoid depending on the `app` tree being present at all times
* Create a more realistic `src` build pipeline
* Property add `src` files to the linting system
* When the `src` tree is present, defer to `src/main.js` to declare an application instead of `app/app.js`.

/cc @rwjblue @Turbo87 

homu: :umbrella: The latest upstream changes (presumably #7118) made this pull request unmergeable. Please resolve the merge conflicts.
mixonic: This code is some of that last stuff we need to see landed in order to support MU behind feature flags.

Patch:
diff --git a/lib/broccoli/ember-app.js b/lib/broccoli/ember-app.js
index c27456af6..22b6a4271 100644
--- a/lib/broccoli/ember-app.js
+++ b/lib/broccoli/ember-app.js
@@ -196,7 +196,13 @@ class EmberApp {
     let srcPath = this._resolveLocal('src');
     let srcTree = existsSync(srcPath) ? new WatchedDir(srcPath) : null;
 
-    let appTree = new WatchedDir(this._resolveLocal('app'));
+    let appPath = this._resolveLocal('app');
+    let appTree;
+    if (experiments.MODULE_UNIFICATION) {
+      appTree = existsSync(appPath) ? new WatchedDir(appPath) : null;
+    } else {
+      appTree = new WatchedDir(appPath);
+    }
 
     let testsPath = this._resolveLocal('tests');
     let testsTree = existsSync(testsPath) ? new WatchedDir(testsPath) : null;
@@ -692,7 +698,6 @@ class EmberApp {
     let index;
     if (!experiments.MODULE_UNIFICATION || !this.trees.src) {
       index = this._rawAppIndex(htmlName);
-
     } else {
       let appIndex = this._rawAppIndex(htmlName, true);
       let srcIndex = this._rawSrcIndex(htmlName);
@@ -707,6 +712,8 @@ class EmberApp {
   }
 
   _rawAppIndex(outputPath, optional) {
+    if (!this.trees.app) { return; }
+
     return new Funnel(this.trees.app, {
       [optional ? 'include' : 'files']: ['index.html'],
       getDestinationPath: () => outputPath,
@@ -730,6 +737,10 @@ class EmberApp {
     @return {Tree}
   */
   _filterAppTree() {
+    if (!this.trees.app) {
+      return;
+    }
+
     if (!this._cachedFilterAppTree) {
       let podPatterns = this._podTemplatePatterns();
       let excludePatterns = podPatterns.concat([
@@ -853,8 +864,12 @@ class EmberApp {
     @return
   */
   _processedAppTree() {
-    let addonTrees = this.addonTreesFor('app');
-    let mergedApp = this._mergeTrees(addonTrees.concat(this._filterAppTree()), {
+    let appTrees = [].concat(
+      this.addonTreesFor('app'),
+      this._filterAppTree()
+    ).filter(Boolean);
+
+    let mergedApp = this._mergeTrees(appTrees, {
       overwrite: true,
       annotation: 'TreeMerger (app)',
     });
@@ -866,6 +881,50 @@ class EmberApp {
     });
   }
 
+  /**
+    @private
+    @method _processedSrcTree
+    @return
+  */
+  _processedSrcTree() {
+    if (!experiments.MODULE_UNIFICATION) {
+      return null;
+    }
+    // styles
+    // templates
+    let rawSrcTree = this.trees.src;
+
+    if (!rawSrcTree) { return; }
+
+    let srcNamespacedTree = 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',
+    });
+  }
+
   /**
     @private
     @method _processedTemplatesTree
@@ -1127,8 +1186,11 @@ class EmberApp {
     let config = this._configTree();
     let templates = this._processedTemplatesTree();
 
+    let srcTree = this._processedSrcTree();
+    let trees = [this._processedAppTree(), srcTree, templates].filter(Boolean);
+
     let app = this.addonPreprocessTree('js', this._mergeTrees(
-      [this._processedAppTree(), templates],
+      trees,
       {
         annotation: 'TreeMerger (preprocessedApp & templates)',
         overwrite: true,
@@ -1206,16 +1268,34 @@ class EmberApp {
     @return {Array}
    */
   lintTestTrees() {
-    let lintedApp = this.addonLintTree('app', this._filterAppTree());
+    let lintTrees = [];
+
+    let appTree = this._filterAppTree();
+    if (appTree) {
+      let lintedApp = this.addonLintTree('app', appTree);
+      lintedApp = processModulesOnly(new Funnel(lintedApp, {
+        srcDir: '/',
+        destDir: `${this.name}/tests/`,
+        annotation: 'Funnel (lint app)',
+      }), 'Babel: lintTree(app)');
+
+      lintTrees.push(lintedApp);
+    }
+
+    if (experiments.MODULE_UNIFICATION && this.trees.src) {
+      let lintedSrc = this.addonLintTree('src', this.trees.src);
+      lintedSrc = processModulesOnly(new Funnel(lintedSrc, {
+        srcDir: '/',
+        destDir: `${this.name}/tests/src/`,
+        annotation: 'Funnel (lint src)',
+      }), 'Babel: lintTree(src)');
+
+      lintTrees.push(lintedSrc);
+    }
+
     let lintedTests = this.addonLintTree('tests', this.trees.tests);
     let lintedTemplates = this.addonLintTree('templates', this._templatesTree());
 
-    lintedApp = processModulesOnly(new Funnel(lintedApp, {
-      srcDir: '/',
-      destDir: `${this.name}/tests/`,
-      annotation: 'Funnel (lint app)',
-    }), 'Babel: lintTree(app)');
-
     lintedTests = processModulesOnly(new Funnel(lintedTests, {
       srcDir: '/',
       destDir: `${this.name}/tests/`,
@@ -1228,7 +1308,7 @@ class EmberApp {
       annotation: 'Funnel (lint templates)',
     }), 'Babel: lintTree(templates)');
 
-    return [lintedApp, lintedTests, lintedTemplates];
+    return [lintedTests, lintedTemplates].concat(lintTrees);
   }
 
   /**
@@ -1338,7 +1418,8 @@ class EmberApp {
         annotation: 'Funnel (styles)',
       });
 
-      let trees = [external].concat(addonTrees, styles);
+      let trees = [external].concat(addonTrees);
+      trees.push(styles);
 
       let options = { outputPaths: this.options.outputPaths.app.css };
       options.registry = this.registry;
@@ -1352,7 +1433,9 @@ class EmberApp {
 
       if (this.vendorStaticStyles.length > 0) {
         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(filename => this.import(filename));
+        this.vendorStaticStyles.forEach(filename => {
+          this.import(filename);
+        });
       }
 
       let vendorStyles = [];
@@ -1548,7 +1631,7 @@ class EmberApp {
     });
 
     let match = assetPath.match(/^node_modules\/((@[^/]+\/)?[^/]+)\//);
-    if (match !== null) {
+    if (match) {
       let basedir = options.resolveFrom || this.project.root;
       let name = match[1];
       let _path = path.dirname(resolve.sync(`${name}/package.json`, { basedir }));
@@ -1693,9 +1776,11 @@ class EmberApp {
       this.index(),
       this.javascript(),
       this.styles(),
+      // undefined when `experiments.MODULE_UNIFICATION` is not available
+      this._srcAfterStylePreprocessing,
       this.otherAssets(),
       this.publicTree(),
-    ];
+    ].filter(Boolean);
 
     if (this.tests && this.trees.tests) {
       sourceTrees = sourceTrees.concat(this.testIndex(), this.test());
@@ -1821,8 +1906,10 @@ class EmberApp {
   */
   _contentForAppBoot(content, config) {
     if (this.options.autoRun) {
+      let shouldUseSrc = experiments.MODULE_UNIFICATION && !!this.trees.src;
+      let moduleToRequire = `${config.modulePrefix}/${shouldUseSrc ? 'src/main' : 'app'}`;
       content.push('if (!runningTests) {');
-      content.push(`  require("${config.modulePrefix}/app")["default"].create(${calculateAppConfig(config)});`);
+      content.push(`  require("${moduleToRequire}")["default"].create(${calculateAppConfig(config)});`);
       content.push('}');
     }
   }


Test patch:
diff --git a/tests/unit/broccoli/ember-app-test.js b/tests/unit/broccoli/ember-app-test.js
index 74d1dd790..9ff9cddee 100644
--- a/tests/unit/broccoli/ember-app-test.js
+++ b/tests/unit/broccoli/ember-app-test.js
@@ -632,10 +632,12 @@ describe('EmberApp', function() {
       });
 
       it('calls each addon postprocessTree hook', function() {
+        app.index = td.function();
         app._processedTemplatesTree = td.function();
 
         td.when(app._processedTemplatesTree(), { ignoreExtraArgs: true }).thenReturn('x');
         td.when(addon.postprocessTree(), { ignoreExtraArgs: true }).thenReturn('blap');
+        td.when(app.index(), { ignoreExtraArgs: true }).thenReturn(null);
 
         expect(app.toTree()).to.equal('blap');
 
@@ -1062,23 +1064,6 @@ describe('EmberApp', function() {
       expect(project.ui.output).to.not.contain('EmberApp.concatFiles() is deprecated');
     });
 
-    describe('podTemplates', function() {
-      it('works', function() {
-        let app = new EmberApp({
-          project,
-        });
-
-        let wasCalledCount = 0;
-        app.podTemplates = function() {
-          wasCalledCount++;
-        };
-
-        expect(wasCalledCount).to.eql(0);
-        app._templatesTree();
-        expect(wasCalledCount).to.eql(1);
-      });
-    });
-
     describe('concat order', function() {
       let count = 0;
       let args = [];
diff --git a/tests/unit/broccoli/ember-app/app-and-dependencies-test.js b/tests/unit/broccoli/ember-app/app-and-dependencies-test.js
new file mode 100644
index 000000000..fe7bf236a
--- /dev/null
+++ b/tests/unit/broccoli/ember-app/app-and-dependencies-test.js
@@ -0,0 +1,165 @@
+'use strict';
+
+const co = require('co');
+const broccoliTestHelper = require('broccoli-test-helper');
+const expect = require('chai').expect;
+
+const EmberApp = require('../../../../lib/broccoli/ember-app');
+const MockCLI = require('../../../helpers/mock-cli');
+const Project = require('../../../../lib/models/project');
+
+const buildOutput = broccoliTestHelper.buildOutput;
+const createTempDir = broccoliTestHelper.createTempDir;
+const walkSync = require('walk-sync');
+const experiments = require("../../../../lib/experiments/index.js");
+
+describe('EmberApp#appAndDependencies', function() {
+  let input;
+
+  beforeEach(co.wrap(function *() {
+    process.env.EMBER_ENV = 'development';
+
+    input = yield createTempDir();
+
+    input.write({
+      'node_modules': {
+        'fake-template-preprocessor': {
+          'package.json': JSON.stringify({
+            name: 'fake-template-preprocessor',
+            main: 'index.js',
+            keywords: ['ember-addon'],
+          }),
+          'index.js': `
+             module.exports = {
+               name: 'fake-template-preprocessor',
+               setupPreprocessorRegistry(type, registry) {
+                 registry.add('template', { ext: 'hbs', toTree(tree) { return tree; } }  )
+               }
+             }
+          `,
+        },
+      },
+      'config': {
+        'environment.js': `module.exports = function() { return { modulePrefix: 'test-app' }; };`,
+      },
+    });
+  }));
+
+  afterEach(function() {
+    delete process.env.EMBER_ENV;
+    return input.dispose();
+  });
+
+  function createApp(options) {
+    options = options || {};
+
+    let pkg = { name: 'ember-app-test', dependencies: { 'fake-template-preprocessor': '*' } };
+
+    let cli = new MockCLI();
+    let project = new Project(input.path(), pkg, cli.ui, cli);
+
+    return new EmberApp({
+      project,
+      name: pkg.name,
+      _ignoreMissingLoader: true,
+      sourcemaps: { enabled: false },
+    }, options);
+  }
+
+  function getFiles(path) {
+    return walkSync(path, {
+      ignore: ['vendor/ember-cli/**/*'],
+      directories: false,
+    });
+  }
+
+  it('results in a tree containing final loose modules', co.wrap(function *() {
+    input.write({
+      'app': {
+        'index.html': 'foobar',
+        'routes': {
+          'application.js': 'export default class { }',
+        },
+        'templates': {
+          'application.hbs': 'hi hi',
+        },
+      },
+    });
+
+    let app = createApp();
+    let output = yield buildOutput(app.appAndDependencies());
+    let actualFiles = getFiles(output.path());
+
+    expect(actualFiles).to.deep.equal([
+      'ember-app-test/config/environments/development.json',
+      'ember-app-test/config/environments/test.json',
+      'ember-app-test/index.html',
+      'ember-app-test/routes/application.js',
+      'ember-app-test/templates/application.hbs',
+    ]);
+  }));
+
+  if (experiments.MODULE_UNIFICATION) {
+    it('works properly without an app directory', co.wrap(function *() {
+      input.write({
+        'src': {
+          'ui': {
+            'index.html': 'foobar',
+            'routes': {
+              'application': {
+                'route.js': 'export default class { }',
+                'template.hbs': 'hi hi',
+              },
+            },
+          },
+        },
+      });
+
+      let app = createApp();
+      let output = yield buildOutput(app.appAndDependencies());
+      let actualFiles = getFiles(output.path());
+
+      expect(actualFiles).to.deep.equal([
+        'ember-app-test/config/environments/development.json',
+        'ember-app-test/config/environments/test.json',
+        'ember-app-test/src/ui/index.html',
+        'ember-app-test/src/ui/routes/application/route.js',
+        'ember-app-test/src/ui/routes/application/template.hbs',
+      ]);
+    }));
+
+    it('merges src with with app', co.wrap(function *() {
+      input.write({
+        'app': {
+          'routes': {
+            'index.js': 'export default class {}',
+          },
+        },
+        'src': {
+          'ui': {
+            'index.html': 'foobar',
+            'routes': {
+              'application': {
+                'route.js': 'export default class { }',
+                'template.hbs': 'hi hi',
+              },
+            },
+          },
+        },
+      });
+
+      let app = createApp();
+      let output = yield buildOutput(app.appAndDependencies());
+      let actualFiles = getFiles(output.path());
+
+      expect(actualFiles).to.deep.equal([
+        'ember-app-test/config/environments/development.json',
+        'ember-app-test/config/environments/test.json',
+        'ember-app-test/routes/index.js',
+        'ember-app-test/src/ui/index.html',
+        'ember-app-test/src/ui/routes/application/route.js',
+        'ember-app-test/src/ui/routes/application/template.hbs',
+      ]);
+    }));
+  }
+});
diff --git a/tests/unit/broccoli/ember-app/index-test.js b/tests/unit/broccoli/ember-app/index-test.js
index cede657fa..b7f60ba8f 100644
--- a/tests/unit/broccoli/ember-app/index-test.js
+++ b/tests/unit/broccoli/ember-app/index-test.js
@@ -151,5 +151,22 @@ describe('EmberApp.index()', function() {
         'index.html': 'src',
       });
     }));
+
+    it('works without an `app` directory', co.wrap(function *() {
+      input.write({
+        'src': {
+          'ui': {
+            'index.html': 'src',
+          },
+        },
+        'config': {},
+      });
+
+      let app = createApp();
+      let output = yield buildOutput(app.index());
+      expect(output.read()).to.deep.equal({
+        'index.html': 'src',
+      });
+    }));
   }
 });