Skip to content

Commit 1c813dc

Browse files
rwjbluerwjblue
committed
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).
1 parent 7335a9f commit 1c813dc

File tree

4 files changed

+76
-116
lines changed

4 files changed

+76
-116
lines changed

‎lib/template-compiler-plugin.js‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,29 +39,15 @@ class TemplateCompiler extends Filter {
3939
// TODO: do we need this?
4040
this.precompile = this.options.templateCompiler.precompile;
4141

42-
let { templateCompiler, plugins, EmberENV } = options;
42+
let { templateCompiler, EmberENV } = options;
4343

44-
utils.registerPlugins(templateCompiler, plugins);
4544
utils.initializeEmberENV(templateCompiler, EmberENV);
4645
}
4746

4847
baseDir() {
4948
return __dirname;
5049
}
5150

52-
unregisterPlugins() {
53-
let { templateCompiler, plugins } = this.options;
54-
55-
utils.unregisterPlugins(templateCompiler, plugins);
56-
}
57-
58-
registeredASTPlugins() {
59-
// This is a super obtuse way to get access to the plugins we've registered
60-
// it also returns other plugins that are registered by ember itself.
61-
let options = this.options.templateCompiler.compileOptions();
62-
return (options.plugins && options.plugins.ast) || [];
63-
}
64-
6551
processString(string, relativePath) {
6652
let srcDir = this.inputPaths[0];
6753
let srcName = path.join(srcDir, relativePath);
@@ -75,10 +61,18 @@ class TemplateCompiler extends Filter {
7561
parseOptions: {
7662
srcName: srcName,
7763
},
64+
65+
// intentionally not using `plugins: this.options.plugins` here
66+
// because if we do, Ember will mutate the shared plugins object (adding
67+
// all of the built in AST transforms into plugins.ast, which breaks
68+
// persistent caching)
69+
plugins: {
70+
ast: this.options.plugins ? this.options.plugins.ast : [],
71+
},
7872
}) +
7973
';';
8074
if (this.options.dependencyInvalidation) {
81-
let plugins = pluginsWithDependencies(this.registeredASTPlugins());
75+
let plugins = pluginsWithDependencies(this.options.plugins.ast);
8276
let dependencies = [];
8377
for (let i = 0; i < plugins.length; i++) {
8478
let pluginDeps = plugins[i].getDependencies(relativePath);

‎lib/utils.js‎

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -154,26 +154,6 @@ function getTemplateCompiler(templateCompilerPath, EmberENV = {}) {
154154
return context.module.exports;
155155
}
156156

157-
function registerPlugins(templateCompiler, plugins) {
158-
if (plugins) {
159-
for (let type in plugins) {
160-
for (let i = 0, l = plugins[type].length; i < l; i++) {
161-
templateCompiler.registerPlugin(type, plugins[type][i]);
162-
}
163-
}
164-
}
165-
}
166-
167-
function unregisterPlugins(templateCompiler, plugins) {
168-
if (plugins) {
169-
for (let type in plugins) {
170-
for (let i = 0, l = plugins[type].length; i < l; i++) {
171-
templateCompiler.unregisterPlugin(type, plugins[type][i]);
172-
}
173-
}
174-
}
175-
}
176-
177157
function initializeEmberENV(templateCompiler, EmberENV) {
178158
if (!templateCompiler || !EmberENV) {
179159
return;
@@ -215,14 +195,14 @@ function setup(pluginInfo, options) {
215195
let htmlbarsOptions = buildOptions(projectConfig, templateCompilerPath, pluginInfo);
216196
let { templateCompiler } = htmlbarsOptions;
217197

218-
registerPlugins(templateCompiler, {
219-
ast: pluginInfo.plugins,
220-
});
221-
222-
let { precompile: templatePrecompile } = templateCompiler;
198+
let templatePrecompile = templateCompiler.precompile;
223199

224200
let precompile = (template, _options) => {
225-
let options = {};
201+
let options = {
202+
plugins: {
203+
ast: pluginInfo.plugins,
204+
},
205+
};
226206

227207
for (let option in _options) {
228208
if (option === 'scope') {
@@ -346,8 +326,6 @@ function setupPlugins(wrappers) {
346326

347327
module.exports = {
348328
buildOptions,
349-
registerPlugins,
350-
unregisterPlugins,
351329
initializeEmberENV,
352330
template,
353331
setup,

‎node-tests/ast_plugins_test.js‎

Lines changed: 56 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,19 @@ const { createTempDir, createBuilder } = require('broccoli-test-helper');
1010
const fixturify = require('fixturify');
1111
const addDependencyTracker = require('../lib/addDependencyTracker');
1212
const templateCompiler = require('ember-source/dist/ember-template-compiler.js');
13-
const CANNOT_UNREGISTER_PLUGINS = !templateCompiler.unregisterPlugin;
1413

1514
describe('AST plugins', function () {
1615
const they = it;
1716
this.timeout(10000);
1817

1918
let input, output, builder, tree, htmlbarsOptions;
2019

20+
async function clearTreeCache(tree) {
21+
if (tree && tree.processor.processor._cache) {
22+
await tree.processor.processor._cache.clear();
23+
}
24+
}
25+
2126
beforeEach(
2227
co.wrap(function* () {
2328
rewriterCallCount = 0;
@@ -33,12 +38,7 @@ describe('AST plugins', function () {
3338

3439
afterEach(
3540
co.wrap(function* () {
36-
if (tree) {
37-
tree.unregisterPlugins();
38-
if (tree.processor.processor._cache) {
39-
yield tree.processor.processor._cache.clear();
40-
}
41-
}
41+
yield clearTreeCache(tree);
4242

4343
if (builder) {
4444
builder.cleanup();
@@ -104,9 +104,6 @@ describe('AST plugins', function () {
104104
they(
105105
'are accepted and used.',
106106
co.wrap(function* () {
107-
if (CANNOT_UNREGISTER_PLUGINS) {
108-
this.skip();
109-
}
110107
htmlbarsOptions.plugins = {
111108
ast: [DivRewriter],
112109
};
@@ -126,9 +123,6 @@ describe('AST plugins', function () {
126123
they(
127124
'will bust the hot cache if the dependency changes.',
128125
co.wrap(function* () {
129-
if (CANNOT_UNREGISTER_PLUGINS) {
130-
this.skip();
131-
}
132126
Object.assign(htmlbarsOptions, {
133127
plugins: {
134128
ast: [DivRewriter],
@@ -185,65 +179,67 @@ describe('AST plugins', function () {
185179
they(
186180
'will bust the persistent cache if the template cache key changes.',
187181
co.wrap(function* () {
188-
if (CANNOT_UNREGISTER_PLUGINS) {
189-
this.skip();
190-
}
191182
Object.assign(htmlbarsOptions, {
192183
plugins: {
193184
ast: [DivRewriter],
194185
},
195186
dependencyInvalidation: true,
196187
});
197188

198-
let firstTree = new TemplateCompiler(input.path(), htmlbarsOptions);
189+
let firstTree, secondTree, thirdTree;
199190

200191
try {
201-
output = createBuilder(firstTree);
202-
yield output.build();
203-
204-
let templateOutput = output.readText('template.js');
205-
assert.ok(!templateOutput.match(/div/));
206-
assert.ok(templateOutput.match(/my-custom-element/));
207-
assert.strictEqual(rewriterCallCount, 1);
208-
} finally {
209-
yield output.dispose();
210-
firstTree.unregisterPlugins();
211-
}
212-
213-
// The state didn't change. the output should be cached
214-
// and the rewriter shouldn't be invoked.
215-
let secondTree = new TemplateCompiler(input.path(), htmlbarsOptions);
216-
try {
217-
let output = createBuilder(secondTree);
218-
yield output.build();
219-
assert.deepStrictEqual(output.changes()['template.js'], 'create');
220-
// the "new" file is read from cache.
221-
let templateOutput = output.readText('template.js');
222-
assert.ok(!templateOutput.match(/div/));
223-
assert.ok(templateOutput.match(/my-custom-element/));
224-
assert.strictEqual(rewriterCallCount, 1);
225-
} finally {
226-
yield output.dispose();
227-
secondTree.unregisterPlugins();
228-
}
192+
firstTree = new TemplateCompiler(input.path(), htmlbarsOptions);
193+
194+
try {
195+
output = createBuilder(firstTree);
196+
yield output.build();
197+
198+
let templateOutput = output.readText('template.js');
199+
assert.ok(!templateOutput.match(/div/));
200+
assert.ok(templateOutput.match(/my-custom-element/));
201+
assert.strictEqual(rewriterCallCount, 1);
202+
} finally {
203+
yield output.dispose();
204+
}
229205

230-
// The state changes. the cache key updates and the template
231-
// should be recompiled.
232-
input.write({
233-
'template.tagname': 'MyChangedElement',
234-
});
206+
// The state didn't change. the output should be cached
207+
// and the rewriter shouldn't be invoked.
208+
secondTree = new TemplateCompiler(input.path(), htmlbarsOptions);
209+
try {
210+
let output = createBuilder(secondTree);
211+
yield output.build();
212+
assert.deepStrictEqual(output.changes()['template.js'], 'create');
213+
// the "new" file is read from cache.
214+
let templateOutput = output.readText('template.js');
215+
assert.ok(!templateOutput.match(/div/));
216+
assert.ok(templateOutput.match(/my-custom-element/));
217+
assert.strictEqual(rewriterCallCount, 1);
218+
} finally {
219+
yield output.dispose();
220+
}
235221

236-
let thirdTree = new TemplateCompiler(input.path(), htmlbarsOptions);
237-
try {
238-
let output = createBuilder(thirdTree);
239-
yield output.build();
240-
let templateOutput = output.readText('template.js');
241-
assert.strictEqual(rewriterCallCount, 2);
242-
assert.ok(templateOutput.match(/my-changed-element/));
243-
assert.strictEqual(rewriterCallCount, 2);
222+
// The state changes. the cache key updates and the template
223+
// should be recompiled.
224+
input.write({
225+
'template.tagname': 'MyChangedElement',
226+
});
227+
228+
thirdTree = new TemplateCompiler(input.path(), htmlbarsOptions);
229+
try {
230+
let output = createBuilder(thirdTree);
231+
yield output.build();
232+
let templateOutput = output.readText('template.js');
233+
assert.strictEqual(rewriterCallCount, 2);
234+
assert.ok(templateOutput.match(/my-changed-element/));
235+
assert.strictEqual(rewriterCallCount, 2);
236+
} finally {
237+
yield output.dispose();
238+
}
244239
} finally {
245-
yield output.dispose();
246-
thirdTree.unregisterPlugins();
240+
clearTreeCache(firstTree);
241+
clearTreeCache(secondTree);
242+
clearTreeCache(thirdTree);
247243
}
248244
})
249245
);

‎node-tests/template_compiler_test.js‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,8 @@ describe('TemplateCompiler', function () {
8484

8585
let tree = new TemplateCompiler(input.path(), htmlbarsOptions);
8686

87-
try {
88-
output = createBuilder(tree);
89-
await output.build();
90-
} finally {
91-
tree.unregisterPlugins();
92-
}
87+
output = createBuilder(tree);
88+
await output.build();
9389

9490
let expected = `export default Ember.HTMLBars.template(${htmlbarsPrecompile(source, {
9591
moduleName: 'template.hbs',
@@ -125,12 +121,8 @@ describe('TemplateCompiler', function () {
125121

126122
let tree = new TemplateCompiler(input.path(), htmlbarsOptions);
127123

128-
try {
129-
output = createBuilder(tree);
130-
await output.build();
131-
} finally {
132-
tree.unregisterPlugins();
133-
}
124+
output = createBuilder(tree);
125+
await output.build();
134126

135127
assert.ok(wasProduction);
136128
});

0 commit comments

Comments
 (0)