Skip to content

Commit

Permalink
Remove usage of registerPlugin / unregisterPlugin
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
rwjblue committed Feb 26, 2021
1 parent 72e6a6e commit 1774d0e
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 70 deletions.
19 changes: 3 additions & 16 deletions lib/template-compiler-plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,29 +40,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);
Expand All @@ -76,10 +62,11 @@ class TemplateCompiler extends Filter {
parseOptions: {
srcName: srcName,
},
plugins: this.options.plugins,
}) +
';';
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);
Expand Down
34 changes: 6 additions & 28 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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') {
Expand Down Expand Up @@ -336,8 +316,6 @@ function setupPlugins(wrappers) {

module.exports = {
buildOptions,
registerPlugins,
unregisterPlugins,
initializeEmberENV,
template,
setup,
Expand Down
14 changes: 0 additions & 14 deletions node-tests/ast_plugins_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ 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;
Expand All @@ -33,7 +32,6 @@ describe('AST plugins', function () {
afterEach(
co.wrap(function* () {
if (tree) {
tree.unregisterPlugins();
if (tree.processor.processor._cache) {
yield tree.processor.processor._cache.clear();
}
Expand Down Expand Up @@ -103,9 +101,6 @@ describe('AST plugins', function () {
they(
'are accepted and used.',
co.wrap(function* () {
if (CANNOT_UNREGISTER_PLUGINS) {
this.skip();
}
htmlbarsOptions.plugins = {
ast: [DivRewriter],
};
Expand All @@ -125,9 +120,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],
Expand Down Expand Up @@ -184,9 +176,6 @@ 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],
Expand All @@ -206,7 +195,6 @@ describe('AST plugins', function () {
assert.strictEqual(rewriterCallCount, 1);
} finally {
yield output.dispose();
firstTree.unregisterPlugins();
}

// The state didn't change. the output should be cached
Expand All @@ -223,7 +211,6 @@ describe('AST plugins', function () {
assert.strictEqual(rewriterCallCount, 1);
} finally {
yield output.dispose();
secondTree.unregisterPlugins();
}

// The state changes. the cache key updates and the template
Expand All @@ -242,7 +229,6 @@ describe('AST plugins', function () {
assert.strictEqual(rewriterCallCount, 2);
} finally {
yield output.dispose();
thirdTree.unregisterPlugins();
}
})
);
Expand Down
16 changes: 4 additions & 12 deletions node-tests/template_compiler_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -124,12 +120,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);
});
Expand Down

0 comments on commit 1774d0e

Please sign in to comment.