Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure dependencySatisfies invalidates when installed packages change #913

Merged
merged 10 commits into from
Aug 9, 2021
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/core/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export function excludeDotFiles(files: string[]) {
}

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

Expand Down Expand Up @@ -393,7 +393,7 @@ export class AppBuilder<TreeNames> {
]);

// this is @embroider/macros configured for full stage3 resolution
babel.plugins.push(this.macrosConfig.babelPluginConfig());
babel.plugins = babel.plugins.concat(this.macrosConfig.babelPluginConfig());

babel.plugins.push([require.resolve('./template-colocation-plugin')]);

Expand Down
1 change: 1 addition & 0 deletions packages/macros/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"@embroider/shared-internals": "0.43.4",
"assert-never": "^1.2.1",
"ember-cli-babel": "^7.26.6",
"find-up": "^5.0.0",
"lodash": "^4.17.21",
"resolve": "^1.20.0",
"semver": "^7.3.2"
Expand Down
2 changes: 1 addition & 1 deletion packages/macros/src/ember-addon-main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export = {
if (!babelPlugins.some(isEmbroiderMacrosPlugin)) {
let appInstance = this._findHost();
let source = appOrAddonInstance.root || appOrAddonInstance.project.root;
babelPlugins.unshift(MacrosConfig.for(appInstance).babelPluginConfig(source));
babelOptions.plugins = babelPlugins.concat(MacrosConfig.for(appInstance).babelPluginConfig(source));
thoov marked this conversation as resolved.
Show resolved Hide resolved
}
},

Expand Down
31 changes: 29 additions & 2 deletions packages/macros/src/macros-config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import fs from 'fs';
import { join } from 'path';
import crypto from 'crypto';
import findUp from 'find-up';
import type { PluginItem } from '@babel/core';
import { PackageCache, getOrCreate } from '@embroider/shared-internals';
import { makeFirstTransform, makeSecondTransform } from './glimmer/ast-transform';
Expand Down Expand Up @@ -255,7 +258,7 @@ export default class MacrosConfig {
// normal node_modules resolution can find their dependencies. In other words,
// owningPackageRoot is needed when you use this inside classic ember-cli, and
// it's not appropriate inside embroider.
babelPluginConfig(owningPackageRoot?: string): PluginItem {
babelPluginConfig(owningPackageRoot?: string): PluginItem[] {
let self = this;
let opts: State['opts'] = {
// this is deliberately lazy because we want to allow everyone to finish
Expand All @@ -281,7 +284,31 @@ export default class MacrosConfig {

importSyncImplementation: this.importSyncImplementation,
};
return [join(__dirname, 'babel', 'macros-babel-plugin.js'), opts];

let lockFilePath = findUp.sync(['yarn.lock', 'package-lock.json', 'pnpm-lock.yaml'], { cwd: opts.appPackageRoot });

if (!lockFilePath) {
lockFilePath = findUp.sync('package.json', { cwd: opts.appPackageRoot });
}

let lockFileBuffer = lockFilePath ? fs.readFileSync(lockFilePath) : 'no-cache-key';

// @embroider/macros provides a macro called dependencySatisfies which checks if a given
// package name satisfies a given semver version range. Due to the way babel caches this can
// cause a problem where the macro plugin does not run (because it has been cached) but the version
// of the dependency being checked for changes (due to installing a different version). This will lead to
// the old evaluated state being used which might be invalid. This cache busting plugin keeps track of a
// hash representing the lock file of the app and if it ever changes forces babel to rerun its plugins.
// more information in issue #906
let cacheKey = crypto.createHash('sha256').update(lockFileBuffer).digest('hex');
return [
[join(__dirname, 'babel', 'macros-babel-plugin.js'), opts],
[
require.resolve('@embroider/shared-internals/src/babel-plugin-cache-busting.js'),
{ version: cacheKey },
`@embroider/macros cache buster: ${owningPackageRoot}`,
],
];
}

static astPlugins(owningPackageRoot?: string): {
Expand Down
4 changes: 2 additions & 2 deletions packages/macros/tests/babel/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export function makeBabelConfig(_babelVersion: number, macroConfig: MacrosConfig
return {
filename: join(__dirname, 'sample.js'),
presets: [],
plugins: [macroConfig.babelPluginConfig()],
plugins: macroConfig.babelPluginConfig(),
};
}

Expand Down Expand Up @@ -100,7 +100,7 @@ export function allBabelVersions(createTests: CreateTests | CreateTestsWithConfi
return {
filename: join(__dirname, 'sample.js'),
presets: [],
plugins: [config.babelPluginConfig()],
plugins: config.babelPluginConfig(),
};
},

Expand Down
3 changes: 2 additions & 1 deletion packages/shared-internals/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
".": {
"browser": "./src/browser-index.js",
"default": "./src/index.js"
}
},
"./src/babel-plugin-cache-busting.js": "./src/babel-plugin-cache-busting.js"
},
"license": "MIT",
"author": "Edward Faulkner",
Expand Down
8 changes: 7 additions & 1 deletion tests/fixtures/macro-test/app/controllers/application.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
import Controller from '@ember/controller';
import { getOwnConfig, isTesting, isDevelopingApp } from '@embroider/macros';
import { getOwnConfig, isTesting, isDevelopingApp, macroCondition, dependencySatisfies } from '@embroider/macros';

export default class Application extends Controller {
constructor() {
super(...arguments);
this.mode = getOwnConfig()['mode'];
this.isTesting = isTesting();
this.isDeveloping = isDevelopingApp();

if (macroCondition(dependencySatisfies('lodash', '^4'))) {
this.lodashVersion = 'four';
} else {
this.lodashVersion = 'three';
}
}
}
3 changes: 2 additions & 1 deletion tests/fixtures/macro-test/app/templates/application.hbs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<div data-test-mode>{{this.mode}}</div>
<div data-test-count>{{macroGetOwnConfig "count"}}</div>
<div>isDeveloping: <span data-test-developing>{{this.isDeveloping}}</span></div>
<div>isTesting: <span data-test-testing>{{this.isTesting}}</span></div>
<div>isTesting: <span data-test-testing>{{this.isTesting}}</span></div>
<div data-test-version>{{this.lodashVersion}}</div>
52 changes: 52 additions & 0 deletions tests/fixtures/macro-test/config/environment.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
'use strict';

module.exports = function (environment) {
let ENV = {
modulePrefix: 'app-template',
environment,
rootURL: '/',
locationType: 'auto',
EmberENV: {
FEATURES: {
// Here you can enable experimental features on an ember canary build
// e.g. EMBER_NATIVE_DECORATOR_SUPPORT: true
},
EXTEND_PROTOTYPES: {
// Prevent Ember Data from overriding Date.parse.
Date: false,
},
},

APP: {
// Here you can pass flags/options to your application instance
// when it is created
},
LODASH_VERSION: process.env.LODASH_VERSION || 'four',
};

if (environment === 'development') {
// ENV.APP.LOG_RESOLVER = true;
// ENV.APP.LOG_ACTIVE_GENERATION = true;
// ENV.APP.LOG_TRANSITIONS = true;
// ENV.APP.LOG_TRANSITIONS_INTERNAL = true;
// ENV.APP.LOG_VIEW_LOOKUPS = true;
}

if (environment === 'test') {
// Testem prefers this...
ENV.locationType = 'none';

// keep test console output quieter
ENV.APP.LOG_ACTIVE_GENERATION = false;
ENV.APP.LOG_VIEW_LOOKUPS = false;

ENV.APP.rootElement = '#ember-testing';
ENV.APP.autoboot = false;
}

if (environment === 'production') {
// here you can enable a production-specific feature
}

return ENV;
};
21 changes: 15 additions & 6 deletions tests/fixtures/macro-test/tests/acceptance/basic-test.js
Original file line number Diff line number Diff line change
@@ -1,36 +1,37 @@
import { module, test } from 'qunit';
import { visit, currentURL } from '@ember/test-helpers';
import { setupApplicationTest } from 'ember-qunit';
import ENV from 'app-template/config/environment';

module('Acceptance | smoke tests', function(hooks) {
module('Acceptance | smoke tests', function (hooks) {
setupApplicationTest(hooks);

test('ensure all scripts in index.html 200', async function(assert) {
test('ensure all scripts in index.html 200', async function (assert) {
for (let { src } of document.scripts) {
let { status } = await fetch(src);
assert.equal(status, 200, `expected: '${src}' to be accessible`);
}
});

test('JS getOwnConfig worked', async function(assert) {
test('JS getOwnConfig worked', async function (assert) {
await visit('/');
assert.equal(currentURL(), '/');
assert.equal(this.element.querySelector('[data-test-mode]').textContent.trim(), 'amazing');
});

test('HBS getOwnConfig worked', async function(assert) {
test('HBS getOwnConfig worked', async function (assert) {
await visit('/');
assert.equal(currentURL(), '/');
assert.equal(this.element.querySelector('[data-test-count]').textContent.trim(), '42');
});

test('JS isTesting worked', async function(assert) {
test('JS isTesting worked', async function (assert) {
await visit('/');
assert.equal(currentURL(), '/');
assert.equal(this.element.querySelector('[data-test-testing]').textContent.trim(), 'true');
});

test('/ordered.js is ordered correctly', function(assert) {
test('/ordered.js is ordered correctly', function (assert) {
assert.deepEqual(self.ORDER, [
// these come via app.import(name, { prepend: true });
// which ultimately end up in vendor.js
Expand All @@ -49,4 +50,12 @@ module('Acceptance | smoke tests', function(hooks) {
'ONE',
]);
});

test('dependency satisfies works correctly', async function (assert) {
await visit('/');
assert.equal(currentURL(), '/');

let expectedVersion = ENV.LODASH_VERSION;
assert.equal(this.element.querySelector('[data-test-version]').textContent.trim(), expectedVersion);
});
});
41 changes: 40 additions & 1 deletion tests/scenarios/macro-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,22 @@ import { appScenarios } from './scenarios';
import { PreparedApp, Project } from 'scenario-tester';
import QUnit from 'qunit';
import merge from 'lodash/merge';
import { dirname } from 'path';
import { dirname, join } from 'path';
import { loadFromFixtureData } from './helpers';
import fs from 'fs-extra';
const { module: Qmodule, test } = QUnit;

function updateLodashVersion(app: PreparedApp, version: string) {
let pkgJson = fs.readJsonSync(join(app.dir, 'package.json'));
let pkgJsonLodash = fs.readJsonSync(join(app.dir, 'node_modules', 'lodash', 'package.json'));

pkgJson.devDependencies.lodash = version;
pkgJsonLodash.version = version;

fs.writeJsonSync(join(app.dir, 'package.json'), pkgJson);
fs.writeJsonSync(join(app.dir, 'node_modules', 'lodash', 'package.json'), pkgJsonLodash);
}

appScenarios
.map('macro-tests', project => {
let macroSampleAddon = Project.fromDir(dirname(require.resolve('../addon-template/package.json')), {
Expand All @@ -27,15 +39,18 @@ appScenarios
funkySampleAddon.linkDependency('@embroider/macros', { baseDir: __dirname });
macroSampleAddon.linkDependency('@embroider/macros', { baseDir: __dirname });
project.linkDevDependency('@embroider/macros', { baseDir: __dirname });
project.linkDevDependency('lodash', { baseDir: __dirname });

project.addDevDependency(macroSampleAddon);
project.addDevDependency(funkySampleAddon);
})
.forEachScenario(scenario => {
Qmodule(scenario.name, function (hooks) {
let app: PreparedApp;

hooks.before(async () => {
app = await scenario.prepare();
updateLodashVersion(app, '4.0.0');
});

test(`yarn test`, async function (assert) {
Expand All @@ -53,5 +68,29 @@ appScenarios
let result = await app.execute(`cross-env THROW_UNLESS_PARALLELIZABLE=1 CLASSIC=true yarn test`);
assert.equal(result.exitCode, 0, result.output);
});

test(`@embroider/macros babel caching plugin works`, async function (assert) {
let lodashFourRun = await app.execute(`yarn test`);
assert.equal(lodashFourRun.exitCode, 0, lodashFourRun.output);

// simulate a different version being installed
updateLodashVersion(app, '3.0.0');

let lodashThreeRun = await app.execute(`cross-env LODASH_VERSION=three yarn test`);
assert.equal(lodashThreeRun.exitCode, 0, lodashThreeRun.output);
});

test(`CLASSIC=true @embroider/macros babel caching plugin works`, async function (assert) {
updateLodashVersion(app, '4.0.1');

let lodashFourRun = await app.execute(`cross-env CLASSIC=true yarn test`);
assert.equal(lodashFourRun.exitCode, 0, lodashFourRun.output);

// simulate a different version being installed
updateLodashVersion(app, '3.0.0');

let lodashThreeRun = await app.execute(`cross-env LODASH_VERSION=three CLASSIC=true yarn test`);
assert.equal(lodashThreeRun.exitCode, 0, lodashThreeRun.output);
});
});
});
1 change: 1 addition & 0 deletions tests/scenarios/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"dependencies": {
"@types/qunit": "^2.11.1",
"fastboot": "^3.1.0",
"fs-extra": "^10.0.0",
"globby": "^11.0.3",
"jsdom": "^16.2.2",
"lodash": "^4.17.20",
Expand Down
Loading