Merge pull request #661 from ember-cli/avoid-register-plugin

Remove usage of registerPlugin / unregisterPlugin

Author: Robert Jackson

lib/template-compiler-plugin.js

@@ -39,29 +39,15 @@ class TemplateCompiler extends Filter {
     // TODO: do we need this?
     this.precompile = this.options.templateCompiler.precompile;
 
-    let { templateCompiler, plugins, EmberENV } = options;
+    let { templateCompiler, EmberENV } = options;
 
-    utils.registerPlugins(templateCompiler, plugins);
     utils.initializeEmberENV(templateCompiler, EmberENV);
   }
 
   baseDir() {
     return __dirname;
   }
 
-  unregisterPlugins() {
-    let { templateCompiler, plugins } = this.options;
-
-    utils.unregisterPlugins(templateCompiler, plugins);
-  }
-
-  registeredASTPlugins() {
-    // This is a super obtuse way to get access to the plugins we've registered
-    // it also returns other plugins that are registered by ember itself.
-    let options = this.options.templateCompiler.compileOptions();
-    return (options.plugins && options.plugins.ast) || [];
-  }
-
   processString(string, relativePath) {
     let srcDir = this.inputPaths[0];
     let srcName = path.join(srcDir, relativePath);
@@ -75,10 +61,18 @@ class TemplateCompiler extends Filter {
           parseOptions: {
             srcName: srcName,
           },
+
+          // intentionally not using `plugins: this.options.plugins` here
+          // because if we do, Ember will mutate the shared plugins object (adding
+          // all of the built in AST transforms into plugins.ast, which breaks
+          // persistent caching)
+          plugins: {
+            ast: this.options.plugins ? this.options.plugins.ast : [],
+          },
         }) +
         ';';
       if (this.options.dependencyInvalidation) {
-        let plugins = pluginsWithDependencies(this.registeredASTPlugins());
+        let plugins = pluginsWithDependencies(this.options.plugins.ast);
         let dependencies = [];
         for (let i = 0; i < plugins.length; i++) {
           let pluginDeps = plugins[i].getDependencies(relativePath);

lib/utils.js

@@ -154,26 +154,6 @@ function getTemplateCompiler(templateCompilerPath, EmberENV = {}) {
   return context.module.exports;
 }
 
-function registerPlugins(templateCompiler, plugins) {
-  if (plugins) {
-    for (let type in plugins) {
-      for (let i = 0, l = plugins[type].length; i < l; i++) {
-        templateCompiler.registerPlugin(type, plugins[type][i]);
-      }
-    }
-  }
-}
-
-function unregisterPlugins(templateCompiler, plugins) {
-  if (plugins) {
-    for (let type in plugins) {
-      for (let i = 0, l = plugins[type].length; i < l; i++) {
-        templateCompiler.unregisterPlugin(type, plugins[type][i]);
-      }
-    }
-  }
-}
-
 function initializeEmberENV(templateCompiler, EmberENV) {
   if (!templateCompiler || !EmberENV) {
     return;
@@ -215,14 +195,14 @@ function setup(pluginInfo, options) {
   let htmlbarsOptions = buildOptions(projectConfig, templateCompilerPath, pluginInfo);
   let { templateCompiler } = htmlbarsOptions;
 
-  registerPlugins(templateCompiler, {
-    ast: pluginInfo.plugins,
-  });
-
-  let { precompile: templatePrecompile } = templateCompiler;
+  let templatePrecompile = templateCompiler.precompile;
 
   let precompile = (template, _options) => {
-    let options = {};
+    let options = {
+      plugins: {
+        ast: pluginInfo.plugins,
+      },
+    };
 
     for (let option in _options) {
       if (option === 'scope') {
@@ -346,8 +326,6 @@ function setupPlugins(wrappers) {
 
 module.exports = {
   buildOptions,
-  registerPlugins,
-  unregisterPlugins,
   initializeEmberENV,
   template,
   setup,

node-tests/ast_plugins_test.js

@@ -10,14 +10,19 @@ const { createTempDir, createBuilder } = require('broccoli-test-helper');
 const fixturify = require('fixturify');
 const addDependencyTracker = require('../lib/addDependencyTracker');
 const templateCompiler = require('ember-source/dist/ember-template-compiler.js');
-const CANNOT_UNREGISTER_PLUGINS = !templateCompiler.unregisterPlugin;
 
 describe('AST plugins', function () {
   const they = it;
   this.timeout(10000);
 
   let input, output, builder, tree, htmlbarsOptions;
 
+  async function clearTreeCache(tree) {
+    if (tree && tree.processor.processor._cache) {
+      await tree.processor.processor._cache.clear();
+    }
+  }
+
   beforeEach(
     co.wrap(function* () {
       rewriterCallCount = 0;
@@ -33,12 +38,7 @@ describe('AST plugins', function () {
 
   afterEach(
     co.wrap(function* () {
-      if (tree) {
-        tree.unregisterPlugins();
-        if (tree.processor.processor._cache) {
-          yield tree.processor.processor._cache.clear();
-        }
-      }
+      yield clearTreeCache(tree);
 
       if (builder) {
         builder.cleanup();
@@ -104,9 +104,6 @@ describe('AST plugins', function () {
   they(
     'are accepted and used.',
     co.wrap(function* () {
-      if (CANNOT_UNREGISTER_PLUGINS) {
-        this.skip();
-      }
       htmlbarsOptions.plugins = {
         ast: [DivRewriter],
       };
@@ -126,9 +123,6 @@ describe('AST plugins', function () {
   they(
     'will bust the hot cache if the dependency changes.',
     co.wrap(function* () {
-      if (CANNOT_UNREGISTER_PLUGINS) {
-        this.skip();
-      }
       Object.assign(htmlbarsOptions, {
         plugins: {
           ast: [DivRewriter],
@@ -185,65 +179,67 @@ describe('AST plugins', function () {
     they(
       'will bust the persistent cache if the template cache key changes.',
       co.wrap(function* () {
-        if (CANNOT_UNREGISTER_PLUGINS) {
-          this.skip();
-        }
         Object.assign(htmlbarsOptions, {
           plugins: {
             ast: [DivRewriter],
           },
           dependencyInvalidation: true,
         });
 
-        let firstTree = new TemplateCompiler(input.path(), htmlbarsOptions);
+        let firstTree, secondTree, thirdTree;
 
         try {
-          output = createBuilder(firstTree);
-          yield output.build();
-
-          let templateOutput = output.readText('template.js');
-          assert.ok(!templateOutput.match(/div/));
-          assert.ok(templateOutput.match(/my-custom-element/));
-          assert.strictEqual(rewriterCallCount, 1);
-        } finally {
-          yield output.dispose();
-          firstTree.unregisterPlugins();
-        }
-
-        // The state didn't change. the output should be cached
-        // and the rewriter shouldn't be invoked.
-        let secondTree = new TemplateCompiler(input.path(), htmlbarsOptions);
-        try {
-          let output = createBuilder(secondTree);
-          yield output.build();
-          assert.deepStrictEqual(output.changes()['template.js'], 'create');
-          // the "new" file is read from cache.
-          let templateOutput = output.readText('template.js');
-          assert.ok(!templateOutput.match(/div/));
-          assert.ok(templateOutput.match(/my-custom-element/));
-          assert.strictEqual(rewriterCallCount, 1);
-        } finally {
-          yield output.dispose();
-          secondTree.unregisterPlugins();
-        }
+          firstTree = new TemplateCompiler(input.path(), htmlbarsOptions);
+
+          try {
+            output = createBuilder(firstTree);
+            yield output.build();
+
+            let templateOutput = output.readText('template.js');
+            assert.ok(!templateOutput.match(/div/));
+            assert.ok(templateOutput.match(/my-custom-element/));
+            assert.strictEqual(rewriterCallCount, 1);
+          } finally {
+            yield output.dispose();
+          }
 
-        // The state changes. the cache key updates and the template
-        // should be recompiled.
-        input.write({
-          'template.tagname': 'MyChangedElement',
-        });
+          // The state didn't change. the output should be cached
+          // and the rewriter shouldn't be invoked.
+          secondTree = new TemplateCompiler(input.path(), htmlbarsOptions);
+          try {
+            let output = createBuilder(secondTree);
+            yield output.build();
+            assert.deepStrictEqual(output.changes()['template.js'], 'create');
+            // the "new" file is read from cache.
+            let templateOutput = output.readText('template.js');
+            assert.ok(!templateOutput.match(/div/));
+            assert.ok(templateOutput.match(/my-custom-element/));
+            assert.strictEqual(rewriterCallCount, 1);
+          } finally {
+            yield output.dispose();
+          }
 
-        let thirdTree = new TemplateCompiler(input.path(), htmlbarsOptions);
-        try {
-          let output = createBuilder(thirdTree);
-          yield output.build();
-          let templateOutput = output.readText('template.js');
-          assert.strictEqual(rewriterCallCount, 2);
-          assert.ok(templateOutput.match(/my-changed-element/));
-          assert.strictEqual(rewriterCallCount, 2);
+          // The state changes. the cache key updates and the template
+          // should be recompiled.
+          input.write({
+            'template.tagname': 'MyChangedElement',
+          });
+
+          thirdTree = new TemplateCompiler(input.path(), htmlbarsOptions);
+          try {
+            let output = createBuilder(thirdTree);
+            yield output.build();
+            let templateOutput = output.readText('template.js');
+            assert.strictEqual(rewriterCallCount, 2);
+            assert.ok(templateOutput.match(/my-changed-element/));
+            assert.strictEqual(rewriterCallCount, 2);
+          } finally {
+            yield output.dispose();
+          }
         } finally {
-          yield output.dispose();
-          thirdTree.unregisterPlugins();
+          clearTreeCache(firstTree);
+          clearTreeCache(secondTree);
+          clearTreeCache(thirdTree);
         }
       })
     );

node-tests/template_compiler_test.js

@@ -84,12 +84,8 @@ describe('TemplateCompiler', function () {
 
     let tree = new TemplateCompiler(input.path(), htmlbarsOptions);
 
-    try {
-      output = createBuilder(tree);
-      await output.build();
-    } finally {
-      tree.unregisterPlugins();
-    }
+    output = createBuilder(tree);
+    await output.build();
 
     let expected = `export default Ember.HTMLBars.template(${htmlbarsPrecompile(source, {
       moduleName: 'template.hbs',
@@ -125,12 +121,8 @@ describe('TemplateCompiler', function () {
 
     let tree = new TemplateCompiler(input.path(), htmlbarsOptions);
 
-    try {
-      output = createBuilder(tree);
-      await output.build();
-    } finally {
-      tree.unregisterPlugins();
-    }
+    output = createBuilder(tree);
+    await output.build();
 
     assert.ok(wasProduction);
   });