Remove usage of registerPlugin / unregisterPlugin

These APIs force Ember to use global mutable state (the list of plugins)
and require some pretty gnarly cache busting techniques to avoid having
addons break each other (due to the global mutable state leaking from
one addon to another).

In order to discourage this mutable state issue, Ember has deprecated
usage of `Ember.HTMLBars.registerPlugin` and
`Ember.HTMLBars.unregisterPlugin` (as of Ember 3.27).

This PR changes all invocations to pass the required AST transforms
directly in to the compiler invocation (instead of calling
`registerPlugin` before hand), and allows us to continue working
properly while avoiding the deprecation (and that evil mutable state).

(cherry picked from commit 1c813dc)

Author: rwjblue

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

@@ -1,5 +1,6 @@
 'use strict';
 
+const crypto = require('crypto');
 const fs = require('fs');
 const path = require('path');
 const hashForDep = require('hash-for-dep');
@@ -140,26 +141,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;
@@ -201,11 +182,17 @@ function setup(pluginInfo, options) {
   let htmlbarsOptions = buildOptions(projectConfig, templateCompilerPath, pluginInfo);
   let { templateCompiler } = htmlbarsOptions;
 
-  registerPlugins(templateCompiler, {
-    ast: pluginInfo.plugins,
-  });
+  let templatePrecompile = templateCompiler.precompile;
+
+  let precompile = (template, options) => {
+    options = options || {};
+    options.plugins = {
+      ast: pluginInfo.plugins,
+    };
+
+    return templatePrecompile(template, options);
+  };
 
-  let { precompile } = templateCompiler;
   precompile.baseDir = () => path.resolve(__dirname, '..');
 
   let cacheKey;
@@ -305,8 +292,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;
 
+  let clearTreeCache = co.wrap(function* clearTreeCache(tree) {
+    if (tree && tree.processor.processor._cache) {
+      yield 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

@@ -83,12 +83,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',
@@ -126,12 +122,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);
   });