From 290db755a1ac40e6f9a11ba00d1f97fbf2a5cbc8 Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Fri, 7 Aug 2020 07:09:33 -0700 Subject: [PATCH 01/13] Pass `isProduction` to Ember template compiler. This flag allows the template compiler to have different behavior in production vs development builds. Co-authored-by: Robert Jackson (cherry picked from commit 7da18b9c09d2d971f09de6e1f970d9a8d4a4f6da) --- lib/ember-addon-main.js | 17 +++++-- lib/template-compiler-plugin.js | 1 + lib/utils.js | 6 ++- node-tests/template_compiler_test.js | 76 ++++++++++++++++++++++++++++ package.json | 2 +- yarn.lock | 13 +++-- 6 files changed, 102 insertions(+), 13 deletions(-) diff --git a/lib/ember-addon-main.js b/lib/ember-addon-main.js index 4ba9c714..24a7ba44 100644 --- a/lib/ember-addon-main.js +++ b/lib/ember-addon-main.js @@ -60,6 +60,8 @@ module.exports = { // ensure that broccoli-ember-hbs-template-compiler is not processing hbs files registry.remove('template', 'broccoli-ember-hbs-template-compiler'); + let isProduction = process.env.EMBER_ENV === 'production'; + // when this.parent === this.project, `this.parent.name` is a function 😭 let parentName = typeof this.parent.name === 'function' ? this.parent.name() : this.parent.name; @@ -73,7 +75,7 @@ module.exports = { ); let shouldColocateTemplates = this._addon._shouldColocateTemplates(); - let htmlbarsOptions = this._addon.htmlbarsOptions(); + let htmlbarsOptions = Object.assign({ isProduction }, this._addon.htmlbarsOptions()); let inputTree = debugTree(tree, '01-input'); @@ -87,10 +89,15 @@ module.exports = { return debugTree(new TemplateCompiler(inputTree, htmlbarsOptions), '03-output'); }, - precompile(string, options) { + precompile(string, _options) { + let options = _options; let htmlbarsOptions = this._addon.htmlbarsOptions(); let templateCompiler = htmlbarsOptions.templateCompiler; + if (isProduction) { + options = Object.assign({ isProduction }, _options); + } + return utils.template(templateCompiler, string, options); }, }); @@ -145,6 +152,8 @@ module.exports = { addonOptions.babel.plugins = addonOptions.babel.plugins || []; let babelPlugins = addonOptions.babel.plugins; + let isProduction = process.env.EMBER_ENV === 'production'; + // add the babel-plugin-htmlbars-inline-precompile to the list of plugins // used by `ember-cli-babel` addon if (!utils.isInlinePrecompileBabelPluginRegistered(babelPlugins)) { @@ -156,7 +165,8 @@ module.exports = { let htmlbarsInlinePrecompilePlugin = utils.buildParalleizedBabelPlugin( pluginInfo, - templateCompilerPath + templateCompilerPath, + isProduction ); babelPlugins.push(htmlbarsInlinePrecompilePlugin); @@ -165,6 +175,7 @@ module.exports = { this.logger.debug('Prevented by these plugins: ' + pluginInfo.unparallelizableWrappers); let htmlBarsPlugin = utils.setup(pluginInfo, { + isProduction, projectConfig: this.projectConfig(), templateCompilerPath, }); diff --git a/lib/template-compiler-plugin.js b/lib/template-compiler-plugin.js index 2b395db7..4beb2d80 100644 --- a/lib/template-compiler-plugin.js +++ b/lib/template-compiler-plugin.js @@ -71,6 +71,7 @@ class TemplateCompiler extends Filter { 'export default ' + utils.template(this.options.templateCompiler, stripBom(string), { contents: string, + isProduction: this.options.isProduction, moduleName: relativePath, parseOptions: { srcName: srcName, diff --git a/lib/utils.js b/lib/utils.js index 4f9e5df6..46e97a2c 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -46,12 +46,13 @@ function isColocatedBabelPluginRegistered(plugins) { ); } -function buildParalleizedBabelPlugin(pluginInfo, templateCompilerPath) { +function buildParalleizedBabelPlugin(pluginInfo, templateCompilerPath, isProduction) { let parallelBabelInfo = { requireFile: require.resolve('./require-from-worker'), buildUsing: 'build', params: { templateCompilerPath, + isProduction, parallelConfigs: pluginInfo.parallelConfigs, modules: INLINE_PRECOMPILE_MODULES, }, @@ -176,6 +177,7 @@ function initializeEmberENV(templateCompiler, EmberENV) { function template(templateCompiler, string, options) { let precompiled = templateCompiler.precompile(string, options); + return 'Ember.HTMLBars.template(' + precompiled + ')'; } @@ -198,7 +200,7 @@ function setup(pluginInfo, options) { let plugin = [ require.resolve('babel-plugin-htmlbars-inline-precompile'), - { precompile, modules: INLINE_PRECOMPILE_MODULES }, + { precompile, isProduction: options.isProduction, modules: INLINE_PRECOMPILE_MODULES }, 'ember-cli-htmlbars:inline-precompile', ]; diff --git a/node-tests/template_compiler_test.js b/node-tests/template_compiler_test.js index 1f88bb78..d320d3de 100644 --- a/node-tests/template_compiler_test.js +++ b/node-tests/template_compiler_test.js @@ -59,6 +59,82 @@ describe('TemplateCompiler', function() { }) ); + it('invokes AST plugins', async function() { + let source = '{{foo-bar}}'; + input.write({ + 'template.hbs': source, + }); + let plugin = env => { + return { + name: 'fake-ast-plugin', + + visitor: { + MustacheStatement() { + return env.syntax.builders.text('Huzzah!'); + }, + }, + }; + }; + + htmlbarsOptions.plugins = { + ast: [plugin], + }; + + let tree = new TemplateCompiler(input.path(), htmlbarsOptions); + + try { + output = createBuilder(tree); + await output.build(); + } finally { + tree.unregisterPlugins(); + } + + let expected = `export default Ember.HTMLBars.template(${htmlbarsPrecompile(source, { + moduleName: 'template.hbs', + plugins: { + ast: [plugin], + }, + })});`; + + let outputString = output.readText('template.js'); + assert.strictEqual(outputString, expected); + assert.ok(outputString.includes('Huzzah!')); + }); + + it('AST Plugins have access to `isProduction` status', async function() { + let source = '{{foo-bar}}'; + input.write({ + 'template.hbs': source, + }); + + let wasProduction = false; + let plugin = env => { + wasProduction = env.isProduction; + + return { + name: 'fake-ast-plugin', + + visitor: {}, + }; + }; + + htmlbarsOptions.isProduction = true; + htmlbarsOptions.plugins = { + ast: [plugin], + }; + + let tree = new TemplateCompiler(input.path(), htmlbarsOptions); + + try { + output = createBuilder(tree); + await output.build(); + } finally { + tree.unregisterPlugins(); + } + + assert.ok(wasProduction); + }); + it( 'ignores utf-8 byte order marks', co.wrap(function*() { diff --git a/package.json b/package.json index ba45828e..7d3603df 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ }, "dependencies": { "@ember/edition-utils": "^1.2.0", - "babel-plugin-htmlbars-inline-precompile": "^3.0.1", + "babel-plugin-htmlbars-inline-precompile": "^3.2.0", "broccoli-debug": "^0.6.5", "broccoli-persistent-filter": "^2.3.1", "broccoli-plugin": "^3.1.0", diff --git a/yarn.lock b/yarn.lock index 31e54dac..2ff2963f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1645,10 +1645,10 @@ babel-plugin-htmlbars-inline-precompile@^1.0.0: resolved "https://registry.yarnpkg.com/babel-plugin-htmlbars-inline-precompile/-/babel-plugin-htmlbars-inline-precompile-1.0.0.tgz#a9d2f6eaad8a3f3d361602de593a8cbef8179c22" integrity sha512-4jvKEHR1bAX03hBDZ94IXsYCj3bwk9vYsn6ux6JZNL2U5pvzCWjqyrGahfsGNrhERyxw8IqcirOi9Q6WCo3dkQ== -babel-plugin-htmlbars-inline-precompile@^3.0.1: - version "3.0.1" - resolved "https://registry.yarnpkg.com/babel-plugin-htmlbars-inline-precompile/-/babel-plugin-htmlbars-inline-precompile-3.0.1.tgz#e1e38a4087f446578e419a21c112530c8df02345" - integrity sha512-ZiFY0nQjtdMPGIDwp/5LYOs6rCr54QfcSV5nPbrA7C++Fv4Vb2Q/qrKYx78t+dwmARJztnOBlObFk4z8veHxNA== +babel-plugin-htmlbars-inline-precompile@^3.2.0: + version "3.2.0" + resolved "https://registry.yarnpkg.com/babel-plugin-htmlbars-inline-precompile/-/babel-plugin-htmlbars-inline-precompile-3.2.0.tgz#c4882ea875d0f5683f0d91c1f72e29a4f14b5606" + integrity sha512-IUeZmgs9tMUGXYu1vfke5I18yYJFldFGdNFQOWslXTnDWXzpwPih7QFduUqvT+awDpDuNtXpdt5JAf43Q1Hhzg== babel-plugin-module-resolver@^3.1.1, babel-plugin-module-resolver@^3.2.0: version "3.2.0" @@ -7098,9 +7098,8 @@ mocha@^7.1.1: yargs-unparser "1.6.0" "module-name-inliner@link:./tests/dummy/lib/module-name-inliner": - version "0.1.0" - dependencies: - ember-cli-version-checker "*" + version "0.0.0" + uid "" morgan@^1.9.1: version "1.9.1" From 4ec05dd204cf2d04cf7527df546d5c2bf8d7e24e Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Tue, 11 Aug 2020 13:05:53 -0400 Subject: [PATCH 02/13] Release 4.4.0 --- CHANGELOG.md | 8 ++++++++ package.json | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee6c1ddc..76b630ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## v4.4.0 (2020-08-11) + +#### :rocket: Enhancement +* [#598](https://github.com/ember-cli/ember-cli-htmlbars/pull/598) Pass `isProduction` to Ember template compiler. ([@rwjblue](https://github.com/rwjblue)) + +#### Committers: 1 +- Robert Jackson ([@rwjblue](https://github.com/rwjblue)) + ## v4.3.1 (2020-04-09) #### :bug: Bug Fix diff --git a/package.json b/package.json index 7d3603df..1c10dc56 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ember-cli-htmlbars", - "version": "4.3.1", + "version": "4.4.0", "description": "A library for adding htmlbars to ember CLI", "keywords": [ "ember-addon", From 7a489a53cc5cbfedc2bc2d7de24328f673ec9bd1 Mon Sep 17 00:00:00 2001 From: Kris Selden Date: Fri, 5 Feb 2021 11:20:13 -0800 Subject: [PATCH 03/13] Make cacheKey lazy Right now cacheKey is eagerly made during the included hook. This is problematic if you have configuration and addons working together. This defers making the cacheKey until it is requested during build. (cherry picked from commit b5fb8fa128f3f1493bd150defdb7e17e5025b3f7) --- lib/utils.js | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 46e97a2c..e3d0fcf4 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -59,12 +59,20 @@ function buildParalleizedBabelPlugin(pluginInfo, templateCompilerPath, isProduct }; // parallelBabelInfo will not be used in the cache unless it is explicitly included - let cacheKey = makeCacheKey(templateCompilerPath, pluginInfo, JSON.stringify(parallelBabelInfo)); - + let cacheKey; return { _parallelBabel: parallelBabelInfo, baseDir: () => __dirname, - cacheKey: () => cacheKey, + cacheKey: () => { + if (cacheKey === undefined) { + cacheKey = makeCacheKey( + templateCompilerPath, + pluginInfo, + JSON.stringify(parallelBabelInfo) + ); + } + return cacheKey; + }, }; } @@ -188,15 +196,20 @@ function setup(pluginInfo, options) { let htmlbarsOptions = buildOptions(projectConfig, templateCompilerPath, pluginInfo); let { templateCompiler } = htmlbarsOptions; - let cacheKey = makeCacheKey(templateCompilerPath, pluginInfo); - registerPlugins(templateCompiler, { ast: pluginInfo.plugins, }); let { precompile } = templateCompiler; precompile.baseDir = () => path.resolve(__dirname, '..'); - precompile.cacheKey = () => cacheKey; + + let cacheKey; + precompile.cacheKey = () => { + if (cacheKey === undefined) { + cacheKey = makeCacheKey(templateCompilerPath, pluginInfo); + } + cacheKey; + }; let plugin = [ require.resolve('babel-plugin-htmlbars-inline-precompile'), From 1741f906579cf8b877b775836acbcffa1a782b8e Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 5 Feb 2021 17:13:03 -0500 Subject: [PATCH 04/13] Add v4.4.1 to CHANGELOG.md. --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76b630ee..7f3f362e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## v4.4.1 (2021-02-05) + +#### :rocket: Enhancement +* [#598](https://github.com/ember-cli/ember-cli-htmlbars/pull/657) Make `cacheKey` calculation lazy ([@krisselden](https://github.com/krisselden)) + +#### Committers: 1 +- Kris Selden ([@krisselden](https://github.com/krisselden)) + ## v4.4.0 (2020-08-11) #### :rocket: Enhancement From 3eb40a42a937b943d22a1d65508791a391f75f4c Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 5 Feb 2021 17:13:14 -0500 Subject: [PATCH 05/13] 4.4.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1c10dc56..05312cf5 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ember-cli-htmlbars", - "version": "4.4.0", + "version": "4.4.1", "description": "A library for adding htmlbars to ember CLI", "keywords": [ "ember-addon", From 2798c0be2da6be98033fe56ebb0c5d69eea48fde Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Thu, 25 Feb 2021 18:09:48 -0500 Subject: [PATCH 06/13] Replace `purgeModule` cache busting with `vm` based sandboxing The template compiler contents have to be evaluated separately for each addon in the build pipeline. If they are **not** the AST plugins from one addon leak through to other addons (or the app). This issue led us to attempt to purge the normal node require cache (the `purgeModule` code). This works (and has been in use for quite a while) but causes a non-trivial amount of memory overhead since each of the addons' ends up with a separate template compiler. This prevents JIT'ing and it causes the source code of the template compiler itself to be in memory many many many times (non-trivially increasing memory pressure). Migrating to `vm.Script` and sandboxed contexts (similar to what is done in FastBoot) resolves both of those issues. The script itself is cached and not reevaluated each time (removing the memory pressure issues) and the JIT information of the script context is also shared. Thanks to @krisselden for pointing out this improvement! (cherry picked from commit 8d5dbcf6791b424e770f6b43ba597dbb5ae5ce36) --- lib/utils.js | 81 ++++++++++++++++++--------------- node-tests/purge-module-test.js | 38 ---------------- 2 files changed, 44 insertions(+), 75 deletions(-) delete mode 100644 node-tests/purge-module-test.js diff --git a/lib/utils.js b/lib/utils.js index e3d0fcf4..b6a54428 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -6,6 +6,9 @@ const hashForDep = require('hash-for-dep'); const debugGenerator = require('heimdalljs-logger'); const logger = debugGenerator('ember-cli-htmlbars:utils'); const addDependencyTracker = require('./addDependencyTracker'); +const vm = require('vm'); + +const TemplateCompilerCache = new Map(); const INLINE_PRECOMPILE_MODULES = Object.freeze({ 'ember-cli-htmlbars': 'hbs', @@ -79,18 +82,10 @@ function buildParalleizedBabelPlugin(pluginInfo, templateCompilerPath, isProduct function buildOptions(projectConfig, templateCompilerPath, pluginInfo) { let EmberENV = projectConfig.EmberENV || {}; - purgeModule(templateCompilerPath); - - // do a full clone of the EmberENV (it is guaranteed to be structured - // cloneable) to prevent ember-template-compiler.js from mutating - // the shared global config - let clonedEmberENV = JSON.parse(JSON.stringify(EmberENV)); - global.EmberENV = clonedEmberENV; // Needed for eval time feature flag checks - let htmlbarsOptions = { isHTMLBars: true, EmberENV: EmberENV, - templateCompiler: require(templateCompilerPath), + templateCompiler: getTemplateCompiler(templateCompilerPath, EmberENV), templateCompilerPath: templateCompilerPath, plugins: { @@ -102,37 +97,47 @@ function buildOptions(projectConfig, templateCompilerPath, pluginInfo) { pluginCacheKey: pluginInfo.cacheKeys, }; - purgeModule(templateCompilerPath); - - delete global.Ember; - delete global.EmberENV; - return htmlbarsOptions; } -function purgeModule(templateCompilerPath) { - // ensure we get a fresh templateCompilerModuleInstance per ember-addon - // instance NOTE: this is a quick hack, and will only work as long as - // templateCompilerPath is a single file bundle - // - // (╯°□°）╯︵ ɹǝqɯǝ - // - // we will also fix this in ember for future releases - - // Module will be cached in .parent.children as well. So deleting from require.cache alone is not sufficient. - let mod = require.cache[templateCompilerPath]; - if (mod && mod.parent) { - let index = mod.parent.children.indexOf(mod); - if (index >= 0) { - mod.parent.children.splice(index, 1); - } else { - throw new TypeError( - `ember-cli-htmlbars attempted to purge '${templateCompilerPath}' but something went wrong.` - ); - } +function getTemplateCompiler(templateCompilerPath, EmberENV = {}) { + let templateCompilerFullPath = require.resolve(templateCompilerPath); + let cacheData = TemplateCompilerCache.get(templateCompilerFullPath); + + if (cacheData === undefined) { + let templateCompilerContents = fs.readFileSync(templateCompilerFullPath, { encoding: 'utf-8' }); + let templateCompilerCacheKey = crypto + .createHash('md5') + .update(templateCompilerContents) + .digest('hex'); + + cacheData = { + script: new vm.Script(templateCompilerContents, { + filename: templateCompilerPath, + }), + + templateCompilerCacheKey, + }; + + TemplateCompilerCache.set(templateCompilerFullPath, cacheData); } - delete require.cache[templateCompilerPath]; + let { script } = cacheData; + + // do a full clone of the EmberENV (it is guaranteed to be structured + // cloneable) to prevent ember-template-compiler.js from mutating + // the shared global config + let clonedEmberENV = JSON.parse(JSON.stringify(EmberENV)); + + let context = vm.createContext({ + EmberENV: clonedEmberENV, + module: { require, exports: {} }, + require, + }); + + script.runInContext(context); + + return context.module.exports; } function registerPlugins(templateCompiler, plugins) { @@ -222,8 +227,10 @@ function setup(pluginInfo, options) { function makeCacheKey(templateCompilerPath, pluginInfo, extra) { let templateCompilerFullPath = require.resolve(templateCompilerPath); - let templateCompilerCacheKey = fs.readFileSync(templateCompilerFullPath, { encoding: 'utf-8' }); + let { templateCompilerCacheKey } = TemplateCompilerCache.get(templateCompilerFullPath); + let cacheItems = [templateCompilerCacheKey, extra].concat(pluginInfo.cacheKeys.sort()); + // extra may be undefined return cacheItems.filter(Boolean).join('|'); } @@ -288,7 +295,6 @@ function setupPlugins(wrappers) { module.exports = { buildOptions, - purgeModule, registerPlugins, unregisterPlugins, initializeEmberENV, @@ -299,4 +305,5 @@ module.exports = { isColocatedBabelPluginRegistered, isInlinePrecompileBabelPluginRegistered, buildParalleizedBabelPlugin, + getTemplateCompiler, }; diff --git a/node-tests/purge-module-test.js b/node-tests/purge-module-test.js deleted file mode 100644 index 4d247cd5..00000000 --- a/node-tests/purge-module-test.js +++ /dev/null @@ -1,38 +0,0 @@ -'use strict'; - -const purgeModule = require('../lib/utils').purgeModule; -const expect = require('chai').expect; - -describe('purgeModule', function() { - const FIXTURE_COMPILER_PATH = require.resolve('./fixtures/compiler'); - - it('it works correctly', function() { - expect(purgeModule('asdfasdfasdfaf-unknown-file')).to.eql(undefined); - - expect(require.cache[FIXTURE_COMPILER_PATH]).to.eql(undefined); - - require(FIXTURE_COMPILER_PATH); - - const mod = require.cache[FIXTURE_COMPILER_PATH]; - - expect(mod.parent).to.eql(module); - expect(mod.parent.children).to.include(mod); - - purgeModule(FIXTURE_COMPILER_PATH); - - expect(require.cache[FIXTURE_COMPILER_PATH]).to.eql(undefined); - expect(mod.parent.children).to.not.include(mod); - - require(FIXTURE_COMPILER_PATH); - - const freshModule = require.cache[FIXTURE_COMPILER_PATH]; - - expect(freshModule.parent).to.eql(module); - expect(freshModule.parent.children).to.include(freshModule); - - purgeModule(FIXTURE_COMPILER_PATH); - - expect(require.cache[FIXTURE_COMPILER_PATH]).to.eql(undefined); - expect(freshModule.parent.children).to.not.include(mod); - }); -}); From 64448c099f1f835134ee29df3277cf6b5282f807 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Thu, 25 Feb 2021 21:43:15 -0500 Subject: [PATCH 07/13] Avoid building the template compiler cache key repeatedly (cherry picked from commit 47041c9003e6e7347822251f11d62014e69b0d81) --- lib/template-compiler-plugin.js | 15 +++++---------- lib/utils.js | 15 +++++++++++++-- node-tests/ast_plugins_test.js | 1 + node-tests/template_compiler_test.js | 1 + 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/lib/template-compiler-plugin.js b/lib/template-compiler-plugin.js index 4beb2d80..0074792e 100644 --- a/lib/template-compiler-plugin.js +++ b/lib/template-compiler-plugin.js @@ -1,6 +1,5 @@ 'use strict'; -const fs = require('fs'); const path = require('path'); const utils = require('./utils'); const Filter = require('broccoli-persistent-filter'); @@ -105,20 +104,16 @@ class TemplateCompiler extends Filter { return strippedOptions; } - _templateCompilerContents() { - if (this.options.templateCompilerPath) { - return fs.readFileSync(this.options.templateCompilerPath, { encoding: 'utf8' }); - } else { - return ''; - } - } - optionsHash() { if (!this._optionsHash) { + let templateCompilerCacheKey = utils.getTemplateCompilerCacheKey( + this.options.templateCompilerPath + ); + this._optionsHash = crypto .createHash('md5') .update(stringify(this._buildOptionsForHash()), 'utf8') - .update(stringify(this._templateCompilerContents()), 'utf8') + .update(templateCompilerCacheKey, 'utf8') .digest('hex'); } diff --git a/lib/utils.js b/lib/utils.js index b6a54428..6b078146 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -225,10 +225,20 @@ function setup(pluginInfo, options) { return plugin; } -function makeCacheKey(templateCompilerPath, pluginInfo, extra) { +function getTemplateCompilerCacheKey(templateCompilerPath) { let templateCompilerFullPath = require.resolve(templateCompilerPath); - let { templateCompilerCacheKey } = TemplateCompilerCache.get(templateCompilerFullPath); + let cacheData = TemplateCompilerCache.get(templateCompilerFullPath); + + if (cacheData === undefined) { + getTemplateCompiler(templateCompilerFullPath); + cacheData = TemplateCompilerCache.get(templateCompilerFullPath); + } + + return cacheData.templateCompilerCacheKey; +} +function makeCacheKey(templateCompilerPath, pluginInfo, extra) { + let templateCompilerCacheKey = getTemplateCompilerCacheKey(templateCompilerPath); let cacheItems = [templateCompilerCacheKey, extra].concat(pluginInfo.cacheKeys.sort()); // extra may be undefined @@ -306,4 +316,5 @@ module.exports = { isInlinePrecompileBabelPluginRegistered, buildParalleizedBabelPlugin, getTemplateCompiler, + getTemplateCompilerCacheKey, }; diff --git a/node-tests/ast_plugins_test.js b/node-tests/ast_plugins_test.js index 658b41c7..60705958 100644 --- a/node-tests/ast_plugins_test.js +++ b/node-tests/ast_plugins_test.js @@ -26,6 +26,7 @@ describe('AST plugins', function() { htmlbarsOptions = { isHTMLBars: true, templateCompiler: templateCompiler, + templateCompilerPath: require.resolve('ember-source/dist/ember-template-compiler.js'), }; }) ); diff --git a/node-tests/template_compiler_test.js b/node-tests/template_compiler_test.js index d320d3de..9dce7c1e 100644 --- a/node-tests/template_compiler_test.js +++ b/node-tests/template_compiler_test.js @@ -38,6 +38,7 @@ describe('TemplateCompiler', function() { htmlbarsOptions = { isHTMLBars: true, templateCompiler: require('ember-source/dist/ember-template-compiler.js'), + templateCompilerPath: require.resolve('ember-source/dist/ember-template-compiler.js'), }; htmlbarsPrecompile = htmlbarsOptions.templateCompiler.precompile; From 41afd221a0365abe5a07f995d7d2274701e3b019 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Thu, 25 Feb 2021 19:59:09 -0500 Subject: [PATCH 08/13] 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 1c813dc86a24d5c5828e02f5a49d4f727116e476) --- lib/template-compiler-plugin.js | 26 +++--- lib/utils.js | 37 +++------ node-tests/ast_plugins_test.js | 116 +++++++++++++-------------- node-tests/template_compiler_test.js | 16 +--- 4 files changed, 81 insertions(+), 114 deletions(-) diff --git a/lib/template-compiler-plugin.js b/lib/template-compiler-plugin.js index 0074792e..bca91191 100644 --- a/lib/template-compiler-plugin.js +++ b/lib/template-compiler-plugin.js @@ -39,9 +39,8 @@ 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); } @@ -49,19 +48,6 @@ class TemplateCompiler extends Filter { 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); diff --git a/lib/utils.js b/lib/utils.js index 6b078146..83a050d3 100644 --- a/lib/utils.js +++ b/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, diff --git a/node-tests/ast_plugins_test.js b/node-tests/ast_plugins_test.js index 60705958..5edc0ddc 100644 --- a/node-tests/ast_plugins_test.js +++ b/node-tests/ast_plugins_test.js @@ -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; @@ -18,6 +17,12 @@ describe('AST plugins', function() { 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,9 +179,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], @@ -195,55 +186,60 @@ describe('AST plugins', function() { 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); } }) ); diff --git a/node-tests/template_compiler_test.js b/node-tests/template_compiler_test.js index 9dce7c1e..9336ab4d 100644 --- a/node-tests/template_compiler_test.js +++ b/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); }); From 336d4d8f6a4d574e02f66c2529020f8692660cc3 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 26 Feb 2021 09:02:56 -0500 Subject: [PATCH 09/13] Ensure Ember 3.27+ can determine global for template compilation. Node 12+ has access to `globalThis` (including within a VM context), but older versions do not. Due to the detection done in https://git.io/Jtb7s, when we can't find `globalThis` (and don't define `global` global) evaluating `ember-template-compiler.js` throws an error "unable to locate global object". This ensures that either `globalThis` or `global` are defined. (cherry picked from commit 957dbc67ea8d1681389441d3441c229555d2f70b) --- lib/utils.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 83a050d3..f312f3a0 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -130,11 +130,21 @@ function getTemplateCompiler(templateCompilerPath, EmberENV = {}) { // the shared global config let clonedEmberENV = JSON.parse(JSON.stringify(EmberENV)); - let context = vm.createContext({ + let sandbox = { EmberENV: clonedEmberENV, module: { require, exports: {} }, require, - }); + }; + + // if we are running on a Node version _without_ a globalThis + // we must provide a `global` + // + // this is due to https://git.io/Jtb7s (Ember 3.27+) + if (typeof globalThis === 'undefined') { + sandbox.global = sandbox; + } + + let context = vm.createContext(sandbox); script.runInContext(context); From c1f98b113fd4242c7d33aab51e2a9d6d59d70fd8 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 26 Feb 2021 23:02:46 -0500 Subject: [PATCH 10/13] Ensure AST plugins have the same ordering as < ember-cli-htmlbars@5.5.0. We have to reverse these for reasons that are a bit bonkers. The initial version of this system used `registeredPlugin` from `ember-template-compiler.js` to set up these plugins (because Ember ~ 1.13 only had `registerPlugin`, and there was no way to pass plugins directly to the call to `compile`/`precompile`). Calling `registerPlugin` unfortunately **inverted** the order of plugins (it essentially did `PLUGINS = [plugin, ...PLUGINS]`). Sooooooo...... we are forced to maintain that **absolutely bonkers** ordering. (cherry picked from commit d8e5ddaa6bde22f6c34b66b6ebbed6bb38611cfc) --- lib/template-compiler-plugin.js | 13 ++++++++++++- lib/utils.js | 7 ++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/template-compiler-plugin.js b/lib/template-compiler-plugin.js index bca91191..3195b768 100644 --- a/lib/template-compiler-plugin.js +++ b/lib/template-compiler-plugin.js @@ -52,6 +52,17 @@ class TemplateCompiler extends Filter { let srcDir = this.inputPaths[0]; let srcName = path.join(srcDir, relativePath); try { + // we have to reverse these for reasons that are a bit bonkers. the initial + // version of this system used `registeredPlugin` from + // `ember-template-compiler.js` to set up these plugins (because Ember ~ 1.13 + // only had `registerPlugin`, and there was no way to pass plugins directly + // to the call to `compile`/`precompile`). calling `registerPlugin` + // unfortunately **inverted** the order of plugins (it essentially did + // `PLUGINS = [plugin, ...PLUGINS]`). + // + // sooooooo...... we are forced to maintain that **absolutely bonkers** ordering + let astPlugins = this.options.plugins ? [].concat(this.options.plugins.ast).reverse() : []; + let result = 'export default ' + utils.template(this.options.templateCompiler, stripBom(string), { @@ -67,7 +78,7 @@ class TemplateCompiler extends Filter { // all of the built in AST transforms into plugins.ast, which breaks // persistent caching) plugins: { - ast: this.options.plugins ? this.options.plugins.ast : [], + ast: astPlugins, }, }) + ';'; diff --git a/lib/utils.js b/lib/utils.js index f312f3a0..47dce06e 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -195,9 +195,14 @@ function setup(pluginInfo, options) { let templatePrecompile = templateCompiler.precompile; let precompile = (template, options) => { + let plugins = pluginInfo.plugins || []; + // concat so we ensure we don't mutate the original plugins + // reverse to ensure that original AST plugin ordering is preserved + let astPlugins = [].concat(plugins).reverse(); + options = options || {}; options.plugins = { - ast: pluginInfo.plugins, + ast: astPlugins, }; return templatePrecompile(template, options); From b97ad948a9dbbb9e2dd45581d0eea3fe7e5476bb Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Wed, 3 Mar 2021 16:57:37 -0500 Subject: [PATCH 11/13] Make `setTimeout`/`clearTimeout` available to the template compiler sandbox --- lib/utils.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/utils.js b/lib/utils.js index 47dce06e..9aec3b15 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -132,6 +132,13 @@ function getTemplateCompiler(templateCompilerPath, EmberENV = {}) { let sandbox = { EmberENV: clonedEmberENV, + + // Older versions of ember-template-compiler (up until ember-source@3.1.0) + // eagerly access `setTimeout` without checking via `typeof` first + setTimeout, + clearTimeout, + + // fake the module into thinking we are running inside a Node context module: { require, exports: {} }, require, }; From b781424b114d2a25ec62ed63a71ec970592fdc9a Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Wed, 3 Mar 2021 17:09:01 -0500 Subject: [PATCH 12/13] Add 4.5.0 to CHANGELOG.md. --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f3f362e..0c377ebf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +## v4.5.0 (2021-03-03) + +#### :rocket: Enhancement +* [#673](https://github.com/ember-cli/ember-cli-htmlbars/pull/673) Backport template compiler improvements from 5.x ([@rwjblue](https://github.com/rwjblue)) +* [#661](https://github.com/ember-cli/ember-cli-htmlbars/pull/661) Remove usage of registerPlugin / unregisterPlugin ([@rwjblue](https://github.com/rwjblue)) +* [#660](https://github.com/ember-cli/ember-cli-htmlbars/pull/660) Replace `purgeModule` cache busting with `vm` based sandboxing ([@rwjblue](https://github.com/rwjblue)) + +#### Committers: 1 +- Robert Jackson ([@rwjblue](https://github.com/rwjblue)) + ## v4.4.1 (2021-02-05) #### :rocket: Enhancement From 212cc389cb81c76f14b8e8e4420607332c7f3c8d Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Wed, 3 Mar 2021 17:09:19 -0500 Subject: [PATCH 13/13] 4.5.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 05312cf5..b8a3784a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ember-cli-htmlbars", - "version": "4.4.1", + "version": "4.5.0", "description": "A library for adding htmlbars to ember CLI", "keywords": [ "ember-addon",