From 1b0e21aeec91a97789fb3106ae834cdc4715e2b0 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Thu, 25 Feb 2021 10:31:22 -0500 Subject: [PATCH] [DEPRECATE] `registerPlugin` / `unregisterPlugin` and legacy class based AST plugins The usage of `registerPlugin`/`unregisterPlugin` **requires** global state mutation which leaks AST plugins between addon's and applications. Tools like Embroider and ember-cli-htmlbars have avoided this by ensuring that we purge the Node module caching system between each attempt at using the template compiler. This results in _massive_ memory growth (the entire `ember-template-compiler.js` source string is included in memory for each addon in the build, as well as any JIT artificats). This is **very _bad_**. This PR deprecates using `registerPlugin`/`unregisterPlugin` along with the legacy AST transform API. All of these APIs are private APIs (as is anything to do with the template compilation AST), but they are used fairly often so a proper deprecation cycle is useful. --- .../lib/system/compile-options.ts | 41 ++++++++++++++++++- .../tests/system/compile_options_test.js | 38 ++++++++++++++--- 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/packages/ember-template-compiler/lib/system/compile-options.ts b/packages/ember-template-compiler/lib/system/compile-options.ts index 36059842997..8a803b3aaaa 100644 --- a/packages/ember-template-compiler/lib/system/compile-options.ts +++ b/packages/ember-template-compiler/lib/system/compile-options.ts @@ -1,5 +1,5 @@ import { EMBER_STRICT_MODE } from '@ember/canary-features'; -import { assert } from '@ember/debug'; +import { assert, deprecate } from '@ember/debug'; import { assign } from '@ember/polyfills'; import { PrecompileOptions } from '@glimmer/compiler'; import { AST, ASTPlugin, ASTPluginEnvironment, Syntax } from '@glimmer/syntax'; @@ -68,6 +68,19 @@ type LegacyPluginClass = new (env: ASTPluginEnvironment) => LegacyPlugin; function wrapLegacyPluginIfNeeded(_plugin: PluginFunc | LegacyPluginClass): PluginFunc { let plugin = _plugin; if (_plugin.prototype && _plugin.prototype.transform) { + deprecate( + 'Using class based template compilation plugins is deprecated, please update to the functional style', + false, + { + id: 'template-compiler.registerPlugin', + until: '4.0.0', + for: 'ember-source', + since: { + enabled: '3.27.0', + }, + } + ); + const pluginFunc: PluginFunc = (env: ASTPluginEnvironment): ASTPlugin => { let pluginInstantiated = false; @@ -97,6 +110,19 @@ function wrapLegacyPluginIfNeeded(_plugin: PluginFunc | LegacyPluginClass): Plug } export function registerPlugin(type: string, _plugin: PluginFunc | LegacyPluginClass): void { + deprecate( + 'registerPlugin is deprecated, please pass plugins directly via `compile` and/or `precompile`.', + false, + { + id: 'template-compiler.registerPlugin', + until: '4.0.0', + for: 'ember-source', + since: { + enabled: '3.27.0', + }, + } + ); + if (type !== 'ast') { throw new Error( `Attempting to register ${_plugin} as "${type}" which is not a valid Glimmer plugin type.` @@ -116,6 +142,19 @@ export function registerPlugin(type: string, _plugin: PluginFunc | LegacyPluginC } export function unregisterPlugin(type: string, PluginClass: PluginFunc | LegacyPluginClass): void { + deprecate( + 'unregisterPlugin is deprecated, please pass plugins directly via `compile` and/or `precompile`.', + false, + { + id: 'template-compiler.registerPlugin', + until: '4.0.0', + for: 'ember-source', + since: { + enabled: '3.27.0', + }, + } + ); + if (type !== 'ast') { throw new Error( `Attempting to unregister ${PluginClass} as "${type}" which is not a valid Glimmer plugin type.` diff --git a/packages/ember-template-compiler/tests/system/compile_options_test.js b/packages/ember-template-compiler/tests/system/compile_options_test.js index e662f747183..921d3f2387a 100644 --- a/packages/ember-template-compiler/tests/system/compile_options_test.js +++ b/packages/ember-template-compiler/tests/system/compile_options_test.js @@ -120,19 +120,34 @@ class CustomPluginsTests extends RenderingTestCase { } moduleFor( - 'ember-template-compiler: registerPlugin with a custom plugins in legacy format', + 'ember-template-compiler: [DEPRECATED] registerPlugin with a custom plugins in legacy format', class extends CustomPluginsTests { beforeEach() { + expectDeprecation( + 'Using class based template compilation plugins is deprecated, please update to the functional style' + ); + expectDeprecation( + 'registerPlugin is deprecated, please pass plugins directly via `compile` and/or `precompile`.' + ); registerPlugin('ast', LegacyCustomTransform); } afterEach() { - unregisterPlugin('ast', LegacyCustomTransform); + expectDeprecation(() => { + unregisterPlugin('ast', LegacyCustomTransform); + }, /unregisterPlugin is deprecated, please pass plugins directly via `compile` and\/or `precompile`/); return super.afterEach(); } ['@test custom registered plugins are deduplicated'](assert) { + expectDeprecation( + 'Using class based template compilation plugins is deprecated, please update to the functional style' + ); + expectDeprecation( + 'registerPlugin is deprecated, please pass plugins directly via `compile` and/or `precompile`.' + ); registerPlugin('ast', LegacyCustomTransform); + this.registerTemplate( 'application', '
' @@ -143,19 +158,27 @@ moduleFor( ); moduleFor( - 'ember-template-compiler: registerPlugin with a custom plugins', + 'ember-template-compiler: [DEPRECATED] registerPlugin with a custom plugins', class extends CustomPluginsTests { beforeEach() { - registerPlugin('ast', customTransform); + expectDeprecation(() => { + registerPlugin('ast', customTransform); + }, /registerPlugin is deprecated, please pass plugins directly via `compile` and\/or `precompile`/); } afterEach() { - unregisterPlugin('ast', customTransform); + expectDeprecation(() => { + unregisterPlugin('ast', customTransform); + }, /unregisterPlugin is deprecated, please pass plugins directly via `compile` and\/or `precompile`/); + return super.afterEach(); } ['@test custom registered plugins are deduplicated'](assert) { - registerPlugin('ast', customTransform); + expectDeprecation(() => { + registerPlugin('ast', customTransform); + }, /registerPlugin is deprecated, please pass plugins directly via `compile` and\/or `precompile`/); + this.registerTemplate( 'application', '
' @@ -170,6 +193,9 @@ moduleFor( class extends RenderingTestCase { // override so that we can provide custom AST plugins to compile compile(templateString) { + expectDeprecation( + 'Using class based template compilation plugins is deprecated, please update to the functional style' + ); return compile(templateString, { plugins: { ast: [LegacyCustomTransform],