Skip to content

Commit

Permalink
Fix babel plugin cache eviction
Browse files Browse the repository at this point in the history
Today, babel’s caches (via babel-loader today) expires caches only if
the options passed to it change, unfortunately if you upgrade a plugin
without changing the configuration options and the file contents remains
the same, the cache will not be evicted. This can lead to spurious
caching problems if the associated plugins behavior has changed.

To address this defect we now include (when available) the
package.json#version of the babel plugins used in our own no-op babel
plugin’s configuration. This new no-op plugin is then inserted at the
end of the configuration in question.
  • Loading branch information
stefanpenner committed Jun 10, 2021
1 parent 51cdcfa commit 6dddcda
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 12 deletions.
38 changes: 37 additions & 1 deletion packages/core/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ 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';
import resolvePackagePath from 'resolve-package-path';

export type EmberENV = unknown;

Expand Down Expand Up @@ -144,6 +145,39 @@ export function excludeDotFiles(files: string[]) {
return files.filter(file => !file.startsWith('.') && !file.includes('/.'));
}

export const CACHE_BUSTING_PLUGIN = {
path: require.resolve('./babel-plugin-cache-busting'),
version: readJSONSync(`${__dirname}/../package.json`).version,
};

export function addCachablePlugin(babelConfig: TransformOptions) {
if (Array.isArray(babelConfig.plugins) && babelConfig.plugins.length > 0) {
const plugins = Object.create(null);
plugins[CACHE_BUSTING_PLUGIN.path] = CACHE_BUSTING_PLUGIN.version;

for (const plugin of babelConfig.plugins) {
let absolutePathToPlugin: string;
if (Array.isArray(plugin) && typeof plugin[0] === 'string') {
absolutePathToPlugin = plugin[0] as string;
} else if (typeof plugin === 'string') {
absolutePathToPlugin = plugin;
} else {
throw new Error(`[Embroider] a babel plugin without an absolute path was from: ${plugin}`);
}

const pkgPath = resolvePackagePath.findUpPackagePath(absolutePathToPlugin);
plugins[absolutePathToPlugin] = pkgPath ? readJSONSync(pkgPath).version : undefined;
}

babelConfig.plugins.push([
CACHE_BUSTING_PLUGIN.path,
{
plugins,
},
]);
}
}

class ParsedEmberAsset {
kind: 'parsed-ember' = 'parsed-ember';
relativePath: string;
Expand Down Expand Up @@ -383,7 +417,9 @@ export class AppBuilder<TreeNames> {
{ absoluteRuntime: __dirname, useESModules: true, regenerator: false },
]);

return makePortable(babel, { basedir: this.root }, this.portableHints);
const portable = makePortable(babel, { basedir: this.root }, this.portableHints);
addCachablePlugin(portable.config);
return portable;
}

private adjustImportsPlugin(engines: Engine[]): PluginItem {
Expand Down
11 changes: 11 additions & 0 deletions packages/core/src/babel-plugin-cache-busting.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export default function makePlugin(): any {
// Dear future @rwjblue,
//
// This plugin exists as a sentinel plugin which has no behavior, but
// provides a position in the babel configuration to include cache busting
// meta-data about other plugins. Specifically their versions.
//
// Yours sincerely,
// Contributor
return {};
}
6 changes: 2 additions & 4 deletions packages/core/src/portable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import mapValues from 'lodash/mapValues';
import assertNever from 'assert-never';
import { Memoize } from 'typescript-memoize';
import resolvePackagePath from 'resolve-package-path';
import { readJSONSync } from 'fs-extra';

export const protocol = '__embroider_portable_values__';
const { globalValues, nonce } = setupGlobals();
Expand All @@ -25,10 +24,9 @@ export function maybeNodeModuleVersion(path: string) {
const packagePath = findUpPackagePath(path);

if (packagePath === null) {
// no package was found
return undefined; // should this bust the cache or ... ?
throw new Error(`Could not find package.json for '${path}'`);
} else {
return readJSONSync(packagePath).version;
return require(packagePath).version; // eslint-disable-line @typescript-eslint/no-require-imports
}
}

Expand Down
47 changes: 46 additions & 1 deletion packages/core/tests/app.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { excludeDotFiles } from '../src/app';
import { excludeDotFiles, addCachablePlugin, CACHE_BUSTING_PLUGIN } from '../src/app';

describe('dot files can be excluded', () => {
test('excludeDotFiles works', () => {
Expand All @@ -10,3 +10,48 @@ describe('dot files can be excluded', () => {
expect(excludeDotFiles(['foo/bar/baz/.foo.js'])).toEqual([]);
});
});

describe('cacheable-plugin', function () {
test('noop', function () {
const input = {};
addCachablePlugin(input);
expect(input).toEqual({});
});

test('no plugins', function () {
const input = { plugins: [] };
addCachablePlugin(input);
expect(input).toEqual({ plugins: [] });
});

test('some plugins', function () {
const input = {
plugins: ['/nope/not/found', __dirname, [__dirname, []], [`${__dirname}/../`, []], __dirname, [__dirname, []]],
};

addCachablePlugin(input);

expect(input).toEqual({
plugins: [
'/nope/not/found',
__dirname,
[__dirname, []],
[`${__dirname}/../`, []],
__dirname,
[__dirname, []],

[
CACHE_BUSTING_PLUGIN.path,
{
plugins: {
[CACHE_BUSTING_PLUGIN.path]: CACHE_BUSTING_PLUGIN.version,
'/nope/not/found': undefined,
[__dirname]: CACHE_BUSTING_PLUGIN.version,
[`${__dirname}/../`]: CACHE_BUSTING_PLUGIN.version,
},
},
],
],
});
});
});
9 changes: 5 additions & 4 deletions packages/core/tests/portable.test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import { maybeNodeModuleVersion } from '../src/portable';
import { readJSONSync } from 'fs-extra';

const EMBROIDER_CORE_VERSION = readJSONSync('../../package.json').version;
const EMBROIDER_CORE_VERSION = readJSONSync(`${__dirname}/../package.json`).version;

describe('maybeNodeModuleVersion', () => {
test('it', () => {
expect(maybeNodeModuleVersion('/dev/null')).toEqual(undefined);
expect(maybeNodeModuleVersion('/does/not/exist')).toEqual(undefined);
expect(() => maybeNodeModuleVersion('/dev/null')).toThrow(/Could not find package.json for '\/dev\/null'/);
expect(() => maybeNodeModuleVersion('/does/not/exist')).toThrow(
/Could not find package.json for '\/does\/not\/exist'/
);
expect(maybeNodeModuleVersion(__dirname)).toEqual(EMBROIDER_CORE_VERSION);
expect(maybeNodeModuleVersion(__filename)).toEqual(EMBROIDER_CORE_VERSION);
});
});

5 changes: 3 additions & 2 deletions test-packages/macro-sample-addon/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"@embroider/test-support": "0.36.0",
"@embroider/webpack": "0.41.0",
"broccoli-asset-rev": "^3.0.0",
"ember-cli-dependency-checker": "^3.1.0",
"ember-cli": "~3.26.1",
"ember-cli-eslint": "^5.1.0",
"ember-cli-inject-live-reload": "^1.8.2",
Expand All @@ -48,7 +49,7 @@
"ember-maybe-import-regenerator": "^0.1.6",
"ember-qunit": "^4.4.1",
"ember-resolver": "^5.0.1",
"ember-source": "~3.16.0",
"ember-source": "~3.10.0",
"ember-source-channel-url": "^1.1.0",
"ember-try": "^1.0.0",
"ember-welcome-page": "^4.0.0",
Expand All @@ -66,4 +67,4 @@
"volta": {
"extends": "../../package.json"
}
}
}

0 comments on commit 6dddcda

Please sign in to comment.