Repository: ember-cli/ember-cli
Pull request #7737 (2018-04-05 14:33:56)

Fix issues identified with "delayed transpilation" system.
This PR includes two specific fixes (discovered during pairing session with @twokul this morning).

---

Ensure `treeForSrc` does nothing when `isModuleUnification` is falsey.

An earlier change made `isModuleUnification` a manual opt-in (due to the breaking change of detecting `src/` and assuming it was module unification), however the `treeForSrc` implementation was not properly leveraging `this.isModuleUnification()` to determine if it should even do any work.

This adds an additional conditional to the short curcuit, and ensures that `treeForSrc` does _nothing_ when a given addon is not opting in to module unification.

---

Cleanup the mechanism to determine the default plugin for a given type.

Instead of using the previous heuristics we require that a plugin declare itself as the default for type. This would look like:

```js
registry.add('js', {
 name: 'ember-cli-babel',
 ext: 'js',
 isDefaultForType: true,
 toTree: (tree) => this.transpileTree(tree)
});
```

If no plugins declare that they are the default for a given type, all plugins will be used (since choosing the default arbitrarily is not a valid option).


Patch:
diff --git a/lib/broccoli/ember-app.js b/lib/broccoli/ember-app.js
index ec49beda0..bd01b58c2 100644
--- a/lib/broccoli/ember-app.js
+++ b/lib/broccoli/ember-app.js
@@ -682,34 +682,46 @@ class EmberApp {
     return this._addonTreesFor(type).map(addonBundle => addonBundle.tree);
   }
 
-  _getDefaultCompiler(type) {
-    // since we are now using the app's registry to compile addon code,
-    // we could get into the case where the app has multiple compilers (coffeescript, emblem, etc.)
-    // operating on addon code where there weren't previously.
-    // This prevents that by using the default app compiler only.
+  _getDefaultPluginForType(type) {
     let plugins = this.registry.load(type);
-    return plugins[plugins.length - 1];
+    let defaultsForType = plugins.filter(plugin => plugin.isDefaultForType);
+
+    if (defaultsForType.length > 1) {
+      throw new Error(
+        `There are multiple preprocessor plugins marked as default for '${type}': ${defaultsForType.map(p => p.name).join(', ')}`
+      );
+    }
+
+    return defaultsForType[0];
   }
 
   _compileAddonTemplates(tree) {
-    let plugin = this._getDefaultCompiler('template');
-    if (plugin) {
-      tree = plugin.toTree(tree, {
-        annotation: `_compileAddonTemplates`,
-        registry: this.registry,
-      });
+    let defaultPluginForType = this._getDefaultPluginForType('template');
+    let options = {
+      annotation: `_compileAddonTemplates`,
+      registry: this.registry,
+    };
+
+    if (defaultPluginForType) {
+      tree = defaultPluginForType.toTree(tree, options);
+    } else {
+      tree = preprocessTemplates(tree, options);
     }
 
     return tree;
   }
 
   _compileAddonJs(tree) {
-    let plugin = this._getDefaultCompiler('js');
-    if (plugin) {
-      tree = plugin.toTree(tree, '/', '/', {
-        annotation: `_compileAddonJs`,
-        registry: this.registry,
-      });
+    let defaultPluginForType = this._getDefaultPluginForType('js');
+    let options = {
+      annotation: '_compileAddonJs',
+      registry: this.registry,
+    };
+
+    if (defaultPluginForType) {
+      tree = defaultPluginForType.toTree(tree, options);
+    } else {
+      tree = preprocessJs(tree, '/', '/', options);
     }
 
     return tree;


Test patch:
diff --git a/tests/unit/broccoli/ember-app/app-and-dependencies-test.js b/tests/unit/broccoli/ember-app/app-and-dependencies-test.js
index d0c0ffa35..399596759 100644
--- a/tests/unit/broccoli/ember-app/app-and-dependencies-test.js
+++ b/tests/unit/broccoli/ember-app/app-and-dependencies-test.js
@@ -33,7 +33,12 @@ describe('EmberApp#appAndDependencies', function() {
             module.exports = {
               name: 'fake-template-preprocessor',
               setupPreprocessorRegistry(type, registry) {
-                registry.add('template', { ext: 'hbs', toTree(tree) { return tree; } });
+                registry.add('template', {
+                  isDefaultForType: true,
+                  name: 'fake-template-preprocessor',
+                  ext: 'hbs',
+                  toTree(tree) { return tree; }
+                });
               }
             };
           `,
@@ -48,7 +53,10 @@ describe('EmberApp#appAndDependencies', function() {
   afterEach(co.wrap(function *() {
     delete process.env.EMBER_ENV;
     yield input.dispose();
-    yield output.dispose();
+
+    if (output) {
+      yield output.dispose();
+    }
   }));
 
   function createApp(options) {
@@ -264,4 +272,79 @@ describe('EmberApp#appAndDependencies', function() {
       'my-addon/index.js',
     ]);
   }));
+
+  it('uses preprocessor that is marked by default', co.wrap(function *() {
+    let app = createApp();
+
+    app.registry.add('js', {
+      ext: 'js',
+      name: 'addon1',
+      isDefaultForType: true,
+      toTree(tree) {
+        return tree;
+      },
+    });
+
+    app.registry.add('js', {
+      ext: 'js',
+      name: 'addon2',
+      toTree(tree) {
+        expect(true).to.equal(false);
+        return tree;
+      },
+    });
+
+    yield buildOutput(app.addonTree());
+  }));
+
+  it('uses all registered preprocessors if none is marked by default', co.wrap(function *() {
+    let count = 0;
+    let app = createApp();
+
+    app.registry.add('js', {
+      ext: 'js',
+      name: 'addon1',
+      toTree(tree) {
+        count++;
+        return tree;
+      },
+    });
+
+    app.registry.add('js', {
+      ext: 'js',
+      name: 'addon2',
+      toTree(tree) {
+        count++;
+        return tree;
+      },
+    });
+
+    yield buildOutput(app.addonTree());
+
+    expect(count).to.equal(2);
+  }));
+
+  it('throws an exception if more than one preprocessor is marked as default', co.wrap(function *() {
+    let exceptionMessage;
+    let app = createApp();
+
+    app.registry.add('template', {
+      ext: 'hbs',
+      name: 'faulty-addon',
+      isDefaultForType: true,
+      toTree(tree) {
+        return tree;
+      },
+    });
+
+    yield co(function *() {
+      output = yield buildOutput(app.addonTree());
+    }).catch(e => {
+      exceptionMessage = e.message;
+    }).then(() => {
+      expect(exceptionMessage).to.equal(
+        `There are multiple preprocessor plugins marked as default for 'template': fake-template-preprocessor, faulty-addon`
+      );
+    });
+  }));
 });