Skip to content

Commit

Permalink
Fix Babel-Loader Caching for ember-template-compiler
Browse files Browse the repository at this point in the history
Today, when switching template-compiler versions, we can end up with global cache pollution. Although this can manifest many ways, it most commonly manifests when running ember-try across versions of ember with different template compilation semantics when also using inline templates.

To address this:

We now include the checksum of the template compiler in the inline-template-compiler babel configuration options.
This busts the cache, as any functioning caching babel subsystem (such as babel-loader) must consider option changes as cache invalidating scenarios. Today, it is in-fact the case that babel-loader operates as we expect, and with this change, caches are correctly invalidated.

We co-located the checksum with the template-compiler options, rather then a single global cache key to both simplify debugging and not prevent some more granular caching strategy from functioning optimally.

Finally this commit uses the existing caching infrastructure to load / generate cacheKey’s for the template-compiler. It changes that infrastructure slightly only to ensure a vm.script is create when absolutely needed.

This does not explicitly include integration tests due to my believe that this is already covered sufficiently:

1) today’s tests already include this (running yarn test currently fails without this fix)
2) type checking ensure we now always pass the key in
3) this is ultimately a configuration change of a babel-loader feature (which itself should test this)
  • Loading branch information
stefanpenner committed Jun 9, 2021
1 parent 64e59a8 commit cb8cca4
Show file tree
Hide file tree
Showing 10 changed files with 54 additions and 25 deletions.
7 changes: 6 additions & 1 deletion packages/compat/src/compat-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import bind from 'bind-decorator';
import { pathExistsSync } from 'fs-extra';
import { tmpdir } from 'os';
import { Options as AdjustImportsOptions } from '@embroider/core/src/babel-plugin-adjust-imports';
import { getEmberExports } from '@embroider/core/src/load-ember-template-compiler';
import semver from 'semver';

interface TreeNames {
Expand Down Expand Up @@ -401,11 +402,15 @@ class CompatAppAdapter implements AppAdapter<TreeNames> {
adjustImportsOptions: this.makeAdjustImportOptions(false),
});

const compilerPath = resolveSync(this.templateCompilerPath(), { basedir: this.root });
const { cacheKey: compilerChecksum } = getEmberExports(compilerPath);
// It's ok that this isn't a fully configured template compiler. We're only
// using it to parse component snippets out of rules.
resolver.astTransformer(
new NodeTemplateCompiler({
compilerPath: resolveSync(this.templateCompilerPath(), { basedir: this.root }),
compilerPath,
compilerChecksum,

EmberENV: {},
plugins: {},
})
Expand Down
4 changes: 4 additions & 0 deletions packages/compat/src/v1-addon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { isEmberAutoImportDynamic, isCompactReexports, isColocationPlugin } from
import { ResolvedDep } from '@embroider/core/src/resolver';
import TemplateCompilerBroccoliPlugin from './template-compiler-broccoli-plugin';
import { fromPairs } from 'lodash';
import { getEmberExports } from '@embroider/core/src/load-ember-template-compiler';

const stockTreeNames = Object.freeze([
'addon',
Expand Down Expand Up @@ -132,8 +133,11 @@ export default class V1Addon {
// our macros don't run here in stage1
options.plugins.ast = options.plugins.ast.filter((p: any) => !isEmbroiderMacrosPlugin(p));
if (options.plugins.ast.length > 0) {
const { cacheKey: compilerChecksum } = getEmberExports(options.templateCompilerPath);

return new NodeTemplateCompiler({
compilerPath: options.templateCompilerPath,
compilerChecksum,
EmberENV: {},
plugins: options.plugins,
resolver: this.templateResolver(),
Expand Down
1 change: 1 addition & 0 deletions packages/compat/tests/audit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ describe('audit', function () {
let templateCompiler = templateCompilerModule(
{
compilerPath: emberTemplateCompilerPath(),
compilerChecksum: `mock-compiler-checksum${Math.random()}`,
EmberENV: {},
plugins: { ast: [] },
resolver: new CompatResolver({
Expand Down
3 changes: 2 additions & 1 deletion packages/compat/tests/resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import Resolver from '../src/resolver';
import { PackageRules } from '../src';

const compilerPath = emberTemplateCompilerPath();
const compilerChecksum = `mock-compiler-checksum${Math.random()}`;

describe('compat-resolver', function () {
let appDir: string;
Expand Down Expand Up @@ -45,7 +46,7 @@ describe('compat-resolver', function () {
otherOptions.adjustImportsImports
),
});
let compiler = new NodeTemplateCompiler({ compilerPath, resolver, EmberENV, plugins });
let compiler = new NodeTemplateCompiler({ compilerPath, compilerChecksum, resolver, EmberENV, plugins });
return function (relativePath: string, contents: string) {
let moduleName = givenFile(relativePath);
let { dependencies } = compiler.precompile(moduleName, contents);
Expand Down
7 changes: 6 additions & 1 deletion packages/core/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import cloneDeep from 'lodash/cloneDeep';
import type { Params as InlineBabelParams } from './babel-plugin-inline-hbs-node';
import { PortableHint } from './portable';
import escapeRegExp from 'escape-string-regexp';
import { getEmberExports } from './load-ember-template-compiler';

export type EmberENV = unknown;

Expand Down Expand Up @@ -898,9 +899,13 @@ export class AppBuilder<TreeNames> {
plugins.ast.push(macroPlugin);
}

const compilerPath = resolve.sync(this.adapter.templateCompilerPath(), { basedir: this.root });
const compilerChecksum = getEmberExports(compilerPath).cacheKey;

return {
plugins,
compilerPath: resolve.sync(this.adapter.templateCompilerPath(), { basedir: this.root }),
compilerPath,
compilerChecksum,
resolver: this.adapter.templateResolver(),
EmberENV: config,
};
Expand Down
47 changes: 26 additions & 21 deletions packages/core/src/load-ember-template-compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,34 +34,39 @@ export function getEmberExports(templateCompilerPath: string): EmbersExports {
let stat = statSync(templateCompilerPath);

let source = patch(readFileSync(templateCompilerPath, 'utf8'), templateCompilerPath);

// matches (essentially) what ember-cli-htmlbars does in https://git.io/Jtbpj
let sandbox = {
module: { require, exports: {} },
require,
};
if (typeof globalThis === 'undefined') {
// for Node 10 usage with Ember 3.27+ we have to define the `global` global
// in order for ember-template-compiler.js to evaluate properly
// due to this code https://git.io/Jtb7
(sandbox as any).global = sandbox;
}

// using vm.createContext / vm.Script to ensure we evaluate in a fresh sandbox context
// so that any global mutation done within ember-template-compiler.js does not leak out
let context = createContext(sandbox);
let script = new Script(source, { filename: templateCompilerPath });

script.runInContext(context);
let theExports: any = context.module.exports;
let theExports: any = undefined;

// cacheKey, theExports
let cacheKey = createHash('md5').update(source).digest('hex');

entry = Object.freeze({
value: {
cacheKey,
theExports,
get theExports() {
if (theExports) {
return theExports;
}

// matches (essentially) what ember-cli-htmlbars does in https://git.io/Jtbpj
let sandbox = {
module: { require, exports: {} },
require,
};

if (typeof globalThis === 'undefined') {
// for Node 10 usage with Ember 3.27+ we have to define the `global` global
// in order for ember-template-compiler.js to evaluate properly
// due to this code https://git.io/Jtb7
(sandbox as any).global = sandbox;
}
// using vm.createContext / vm.Script to ensure we evaluate in a fresh sandbox context
// so that any global mutation done within ember-template-compiler.js does not leak out
let context = createContext(sandbox);
let script = new Script(source, { filename: templateCompilerPath });

script.runInContext(context);
return (theExports = context.module.exports);
},
},
stat, // This is stored, so we can reload the templateCompiler if it changes mid-build.
});
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/template-compiler-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import adjustImportsPlugin from './babel-plugin-adjust-imports';

export interface NodeTemplateCompilerParams {
compilerPath: string;
compilerChecksum: string;
resolver?: Resolver;
EmberENV: unknown;
plugins: Plugins;
Expand Down
3 changes: 3 additions & 0 deletions packages/core/tests/inline-hbs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ describe('inline-hbs', () => {
babelConfig() {
let templateCompiler: NodeTemplateCompilerParams = {
compilerPath: emberTemplateCompilerPath(),
compilerChecksum: `mock-compiler-checksum${Math.random()}`,
EmberENV: {},
plugins: {
ast: [sampleTransform],
Expand All @@ -105,6 +106,7 @@ describe('inline-hbs', () => {
babelConfig() {
let templateCompiler: NodeTemplateCompilerParams = {
compilerPath: emberTemplateCompilerPath(),
compilerChecksum: `mock-compiler-checksum${Math.random()}`,
EmberENV: {},
plugins: {
ast: [],
Expand All @@ -128,6 +130,7 @@ describe('inline-hbs', () => {
babelConfig(major: number) {
let templateCompiler: NodeTemplateCompilerParams = {
compilerPath: emberTemplateCompilerPath(),
compilerChecksum: `mock-compiler-checksum${Math.random()}`,
EmberENV: {},
plugins: {
ast: [],
Expand Down
4 changes: 4 additions & 0 deletions packages/macros/tests/glimmer/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { NodeTemplateCompiler } from '@embroider/core';
import { getEmberExports } from '@embroider/core/src/load-ember-template-compiler';
import { emberTemplateCompilerPath, Project } from '@embroider/test-support';
import { MacrosConfig } from '../../src/node';
import { join } from 'path';

const compilerPath = emberTemplateCompilerPath();
const { cacheKey: compilerChecksum } = getEmberExports(compilerPath);

export { Project };

Expand All @@ -19,6 +22,7 @@ export function templateTests(createTests: CreateTestsWithConfig | CreateTests)
setConfig(config);
let compiler = new NodeTemplateCompiler({
compilerPath,
compilerChecksum,
EmberENV: {},
plugins: {
ast: plugins,
Expand Down
2 changes: 1 addition & 1 deletion packages/webpack/src/ember-webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ const Webpack: PackagerConstructor<Options> = class Webpack implements Packager
this.consoleWrite(stats.toString());
}
resolve(stats.toJson());
} catch(e) {
} catch (e) {
reject(e);
}
});
Expand Down

0 comments on commit cb8cca4

Please sign in to comment.