Repository: ember-cli/ember-cli
Pull request #6518 (2016-12-03 18:55:19)

Avoid pod template processing for addons without pod templates.
This turned out to be a significant performance issue, and the majority of addons do not have pod templates. This lets us opt-out of the work when possible...

TODO:

- [x] Properly detect `addon/templates` and ensure `shouldCompilePodTemplates` does not falsely return true when only `addon/templates` templates exist.
- [x] Tests
  - [x] Test with addon/templates templates, but no pods based templates
  - [x] Test with addon pods based templates but no addon/templates
  - [x] Test with both addon pods based templates and addon/templates templates
- [x] Rebase on top of https://github.com/ember-cli/ember-cli/pull/6515

homu: :umbrella: The latest upstream changes (presumably #6515) made this pull request unmergeable. Please resolve the merge conflicts.

Patch:
diff --git a/lib/models/addon.js b/lib/models/addon.js
index 0a9e45f20..9ff0601ce 100644
--- a/lib/models/addon.js
+++ b/lib/models/addon.js
@@ -632,6 +632,28 @@ var Addon = CoreObject.extend({
     @return {Boolean} indicates if templates need to be compiled for this addon
   */
   shouldCompileTemplates: function() {
+    return this._detectTemplateInfo().hasTemplates;
+  },
+
+  /**
+     Looks in the addon/ and addon/templates trees to determine if template files
+     exists in the pods format that need to be precompiled.
+
+     This is executed once when building, but not on rebuilds.
+
+     @private
+     @method _shouldCompilePodTemplates
+     @return {Boolean} indicates if pod based templates need to be compiled for this addon
+  */
+  _shouldCompilePodTemplates: function() {
+    return this._detectTemplateInfo().hasPodTemplates;
+  },
+
+  _detectTemplateInfo: function() {
+    if (this._cachedTemplateInfo) {
+      return this._cachedTemplateInfo;
+    }
+
     var templateExtensions = this.registry.extensionsForType('template');
     var addonTreePath = this._treePathFor('addon');
     var addonTemplatesTreePath = this._treePathFor('addon-templates');
@@ -639,6 +661,17 @@ var Addon = CoreObject.extend({
 
     var files = this._getAddonTreeFiles();
 
+    var addonTemplatesRelativeToAddonPath = addonTemplatesTreeInAddonTree && addonTemplatesTreePath.replace(addonTreePath + '/', '');
+    var podTemplateMatcher = new RegExp('template\.(' + templateExtensions.join('|') + ')$');
+    var hasPodTemplates = files.some(function(file) {
+      // short curcuit if this is actually a `addon/templates` file
+      if (addonTemplatesTreeInAddonTree && file.indexOf(addonTemplatesRelativeToAddonPath) === 0) {
+        return false;
+      }
+
+      return file.match(podTemplateMatcher);
+    });
+
     if (!addonTemplatesTreeInAddonTree) {
       files = files.concat(this._getAddonTemplatesTreeFiles());
     }
@@ -646,10 +679,16 @@ var Addon = CoreObject.extend({
     files = files.filter(Boolean);
 
     var extensionMatcher = new RegExp('(' + templateExtensions.join('|') + ')$');
-
-    return files.some(function(file) {
+    var hasTemplates = files.some(function(file) {
       return file.match(extensionMatcher);
     });
+
+    this._cachedTemplateInfo = {
+      hasTemplates: hasTemplates,
+      hasPodTemplates: hasPodTemplates
+    };
+
+    return this._cachedTemplateInfo;
   },
 
   _getAddonTreeFiles: function() {
@@ -692,17 +731,19 @@ var Addon = CoreObject.extend({
       trees.push(standardTemplates);
     }
 
-    var includePatterns = this.registry.extensionsForType('template').map(function(extension) {
-      return '**/*/template.' + extension;
-    });
+    if (this._shouldCompilePodTemplates()) {
+      var includePatterns = this.registry.extensionsForType('template').map(function(extension) {
+        return '**/*/template.' + extension;
+      });
 
-    var podTemplates = new Funnel(tree, {
-      include: includePatterns,
-      destDir: 'modules/' + this.name + '/',
-      annotation: 'Funnel: Addon Pod Templates'
-    });
+      var podTemplates = new Funnel(tree, {
+        include: includePatterns,
+        destDir: 'modules/' + this.name + '/',
+        annotation: 'Funnel: Addon Pod Templates'
+      });
 
-    trees.push(podTemplates);
+      trees.push(podTemplates);
+    }
 
     this._cachedAddonTemplateFiles = mergeTrees(trees, {
       annotation: 'TreeMerge (' + this.name + ' templates)'


Test patch:
diff --git a/tests/unit/models/addon-test.js b/tests/unit/models/addon-test.js
index 658ba0ec5..fd45b1b8d 100644
--- a/tests/unit/models/addon-test.js
+++ b/tests/unit/models/addon-test.js
@@ -619,22 +619,33 @@ describe('models/addon.js', function() {
         addon.compileTemplates();
       }).not.to.throw();
     });
+  });
+
+  describe('_detectTemplateInfo', function() {
+    beforeEach(function() {
+      projectPath = path.resolve(fixturePath, 'simple');
+      var packageContents = require(path.join(projectPath, 'package.json'));
+
+      project = new Project(projectPath, packageContents);
+
+      project.initializeAddons();
+
+      addon = findWhere(project.addons, { name: 'Ember CLI Generated with export' });
+    });
 
     it('should not call _getAddonTemplatesTreeFiles when default treePath is used', function() {
-      addon.root = path.join(fixturePath, 'with-empty-addon-templates');
       var wasCalled = false;
       addon._getAddonTemplatesTreeFiles = function() {
         wasCalled = true;
         return [];
       };
 
-      addon.compileTemplates();
+      addon._detectTemplateInfo();
 
       expect(wasCalled).to.not.be.ok;
     });
 
     it('should call _getAddonTemplatesTreeFiles when custom treePaths[\'addon-templates\'] is used', function() {
-      addon.root = path.join(fixturePath, 'with-empty-addon-templates');
       addon.treePaths['addon-templates'] = 'foo';
       var wasCalled = false;
       addon._getAddonTemplatesTreeFiles = function() {
@@ -642,10 +653,72 @@ describe('models/addon.js', function() {
         return [];
       };
 
-      addon.compileTemplates();
+      addon._detectTemplateInfo();
 
       expect(wasCalled).to.be.ok;
     });
+
+    it('hasPodTemplates when pod templates found', function() {
+      addon._getAddonTreeFiles = function() {
+        return [
+          'foo-bar/',
+          'foo-bar/component.js',
+          'foo-bar/template.hbs'
+        ];
+      };
+
+      expect(addon._detectTemplateInfo()).to.deep.equal({
+        hasTemplates: true,
+        hasPodTemplates: true
+      });
+    });
+
+    it('does not hasPodTemplates when no pod templates found', function() {
+      addon._getAddonTreeFiles = function() {
+        return [
+          'templates/',
+          'templates/components/',
+          'templates/components/foo-bar.hbs'
+        ];
+      };
+
+      expect(addon._detectTemplateInfo()).to.deep.equal({
+        hasTemplates: true,
+        hasPodTemplates: false
+      });
+    });
+
+    it('does not hasPodTemplates when no pod templates found (pod-like structure in `addon/templates/`)', function() {
+      addon._getAddonTreeFiles = function() {
+        return [
+          'templates/',
+          // this doesn't need "pod template handling" because
+          // it is actually in the addon-templates tree
+          'templates/foo-bar/template.hbs'
+        ];
+      };
+
+      expect(addon._detectTemplateInfo()).to.deep.equal({
+        hasTemplates: true,
+        hasPodTemplates: false
+      });
+    });
+
+    it('does not hasTemplates when no templates found', function() {
+      addon._getAddonTreeFiles = function() {
+        return [
+          'components/',
+          'components/foo-bar.js',
+          'templates/',
+          'templates/components/'
+        ];
+      };
+
+      expect(addon._detectTemplateInfo()).to.deep.equal({
+        hasTemplates: false,
+        hasPodTemplates: false
+      });
+    });
   });
 
   describe('addonDiscovery', function() {