From ac4109646876d377ed5bcb7a786ce7292d6a0b2e Mon Sep 17 00:00:00 2001 From: Travis Hoover Date: Thu, 29 Jul 2021 15:06:35 -0700 Subject: [PATCH 01/10] Rerun macros babel plugins if dependencies change dependencySatisfies can become invalid if babel caches the macros plugin + the dependency that is being checked for changes its package version. This adds a cache busting plugin that tracks a hash of the contents of the apps lock file so any changes to dependencies will bust the cache. Fixes: #906 --- .../macros/src/babel-plugin-cache-busting.ts | 3 +++ packages/macros/src/ember-addon-main.ts | 26 +++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 packages/macros/src/babel-plugin-cache-busting.ts diff --git a/packages/macros/src/babel-plugin-cache-busting.ts b/packages/macros/src/babel-plugin-cache-busting.ts new file mode 100644 index 000000000..ec7a51b05 --- /dev/null +++ b/packages/macros/src/babel-plugin-cache-busting.ts @@ -0,0 +1,3 @@ +export default function makePlugin(): any { + return {}; +} diff --git a/packages/macros/src/ember-addon-main.ts b/packages/macros/src/ember-addon-main.ts index bdd019146..735424646 100644 --- a/packages/macros/src/ember-addon-main.ts +++ b/packages/macros/src/ember-addon-main.ts @@ -1,4 +1,6 @@ +import fs from 'fs'; import { join } from 'path'; +import crypto from 'crypto'; import { MacrosConfig, isEmbroiderMacrosPlugin } from './node'; export = { @@ -70,6 +72,30 @@ export = { let appInstance = this._findHost(); let source = appOrAddonInstance.root || appOrAddonInstance.project.root; babelPlugins.unshift(MacrosConfig.for(appInstance).babelPluginConfig(source)); + + let yarnLockPath = join(appInstance.project.root, 'yarn.lock'); + let npmLockPath = join(appInstance.project.root, 'package-lock.json'); + let packagePath = join(appInstance.project.root, 'package.json'); + let lockFileBuffer; + + if (fs.existsSync(yarnLockPath)) { + lockFileBuffer = fs.readFileSync(yarnLockPath); + } else if (fs.existsSync(npmLockPath)) { + lockFileBuffer = fs.readFileSync(npmLockPath); + } else { + // no lock file found, using package.json as a fall back + lockFileBuffer = fs.readFileSync(packagePath); + } + + // @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'); + babelPlugins.push([require.resolve('./babel-plugin-cache-busting'), { version: cacheKey }]); } }, From 0ea82c1eccb4bc6e2e216c631644f811d3920aba Mon Sep 17 00:00:00 2001 From: Travis Hoover Date: Wed, 4 Aug 2021 16:51:16 -0700 Subject: [PATCH 02/10] adding test case --- packages/core/src/app.ts | 2 +- .../macros/src/babel-plugin-cache-busting.ts | 3 -- packages/macros/src/ember-addon-main.ts | 9 +++- packages/shared-internals/package.json | 3 +- .../src/babel-plugin-cache-busting.ts | 0 .../macro-test/app/controllers/application.js | 8 ++- .../macro-test/app/templates/application.hbs | 3 +- .../fixtures/macro-test/config/environment.js | 52 +++++++++++++++++++ .../macro-test/tests/acceptance/basic-test.js | 21 +++++--- tests/scenarios/macro-test.ts | 28 +++++++++- tests/scenarios/package.json | 7 +-- yarn.lock | 38 +++++++++----- 12 files changed, 144 insertions(+), 30 deletions(-) delete mode 100644 packages/macros/src/babel-plugin-cache-busting.ts rename packages/{core => shared-internals}/src/babel-plugin-cache-busting.ts (100%) create mode 100644 tests/fixtures/macro-test/config/environment.js diff --git a/packages/core/src/app.ts b/packages/core/src/app.ts index de4f75779..747b0f2e0 100644 --- a/packages/core/src/app.ts +++ b/packages/core/src/app.ts @@ -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, }; diff --git a/packages/macros/src/babel-plugin-cache-busting.ts b/packages/macros/src/babel-plugin-cache-busting.ts deleted file mode 100644 index ec7a51b05..000000000 --- a/packages/macros/src/babel-plugin-cache-busting.ts +++ /dev/null @@ -1,3 +0,0 @@ -export default function makePlugin(): any { - return {}; -} diff --git a/packages/macros/src/ember-addon-main.ts b/packages/macros/src/ember-addon-main.ts index 735424646..45ccfcb31 100644 --- a/packages/macros/src/ember-addon-main.ts +++ b/packages/macros/src/ember-addon-main.ts @@ -75,6 +75,7 @@ export = { let yarnLockPath = join(appInstance.project.root, 'yarn.lock'); let npmLockPath = join(appInstance.project.root, 'package-lock.json'); + let pnpmLockPath = join(appInstance.project.root, 'pnpm-lock.yaml'); let packagePath = join(appInstance.project.root, 'package.json'); let lockFileBuffer; @@ -82,6 +83,8 @@ export = { lockFileBuffer = fs.readFileSync(yarnLockPath); } else if (fs.existsSync(npmLockPath)) { lockFileBuffer = fs.readFileSync(npmLockPath); + } else if (fs.existsSync(pnpmLockPath)) { + lockFileBuffer = fs.readFileSync(pnpmLockPath); } else { // no lock file found, using package.json as a fall back lockFileBuffer = fs.readFileSync(packagePath); @@ -95,7 +98,11 @@ export = { // 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'); - babelPlugins.push([require.resolve('./babel-plugin-cache-busting'), { version: cacheKey }]); + babelPlugins.push([ + require.resolve('@embroider/shared-internals/src/babel-plugin-cache-busting.js'), + { version: cacheKey }, + '@embroider/macros cache buster', + ]); } }, diff --git a/packages/shared-internals/package.json b/packages/shared-internals/package.json index 259211fea..d06b06880 100644 --- a/packages/shared-internals/package.json +++ b/packages/shared-internals/package.json @@ -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", diff --git a/packages/core/src/babel-plugin-cache-busting.ts b/packages/shared-internals/src/babel-plugin-cache-busting.ts similarity index 100% rename from packages/core/src/babel-plugin-cache-busting.ts rename to packages/shared-internals/src/babel-plugin-cache-busting.ts diff --git a/tests/fixtures/macro-test/app/controllers/application.js b/tests/fixtures/macro-test/app/controllers/application.js index e75f85988..6c85764c3 100644 --- a/tests/fixtures/macro-test/app/controllers/application.js +++ b/tests/fixtures/macro-test/app/controllers/application.js @@ -1,5 +1,5 @@ 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() { @@ -7,5 +7,11 @@ export default class Application extends Controller { this.mode = getOwnConfig()['mode']; this.isTesting = isTesting(); this.isDeveloping = isDevelopingApp(); + + if (macroCondition(dependencySatisfies('lodash', '^4'))) { + this.lodashVersion = 'four'; + } else { + this.lodashVersion = 'three'; + } } } diff --git a/tests/fixtures/macro-test/app/templates/application.hbs b/tests/fixtures/macro-test/app/templates/application.hbs index 1d0d1451e..dad21ec84 100644 --- a/tests/fixtures/macro-test/app/templates/application.hbs +++ b/tests/fixtures/macro-test/app/templates/application.hbs @@ -1,4 +1,5 @@
{{this.mode}}
{{macroGetOwnConfig "count"}}
isDeveloping: {{this.isDeveloping}}
-
isTesting: {{this.isTesting}}
\ No newline at end of file +
isTesting: {{this.isTesting}}
+
{{this.lodashVersion}}
diff --git a/tests/fixtures/macro-test/config/environment.js b/tests/fixtures/macro-test/config/environment.js new file mode 100644 index 000000000..43bb7ed81 --- /dev/null +++ b/tests/fixtures/macro-test/config/environment.js @@ -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; +}; diff --git a/tests/fixtures/macro-test/tests/acceptance/basic-test.js b/tests/fixtures/macro-test/tests/acceptance/basic-test.js index f3144f51b..d5141189e 100644 --- a/tests/fixtures/macro-test/tests/acceptance/basic-test.js +++ b/tests/fixtures/macro-test/tests/acceptance/basic-test.js @@ -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 @@ -49,4 +50,12 @@ module('Acceptance | smoke tests', function(hooks) { 'ONE', ]); }); + + test('foobar', async function (assert) { + await visit('/'); + assert.equal(currentURL(), '/'); + + let expectedVersion = ENV.LODASH_VERSION; + assert.equal(this.element.querySelector('[data-test-version]').textContent.trim(), expectedVersion); + }); }); diff --git a/tests/scenarios/macro-test.ts b/tests/scenarios/macro-test.ts index 7ddcfe3c4..4f866e041 100644 --- a/tests/scenarios/macro-test.ts +++ b/tests/scenarios/macro-test.ts @@ -2,8 +2,9 @@ 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; appScenarios @@ -27,6 +28,7 @@ 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); @@ -53,5 +55,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(`multiple dependency check`, async function (assert) { + let pkgJson = fs.readJsonSync(join(app.dir, 'package.json')); + let pkgJsonLodash = fs.readJsonSync(join(app.dir, 'node_modules', 'lodash', 'package.json')); + + pkgJson.devDependencies.lodash = '4.0.0'; + pkgJsonLodash.version = '4.0.0'; + + fs.writeJsonSync(join(app.dir, 'package.json'), pkgJson); + fs.writeJsonSync(join(app.dir, 'node_modules', 'lodash', 'package.json'), pkgJsonLodash); + + let lodashFourRun = await app.execute(`yarn test`); + assert.equal(lodashFourRun.exitCode, 0, lodashFourRun.output); + + // downgrade lodash to 3.0.0 + pkgJson.devDependencies.lodash = '3.0.0'; + pkgJsonLodash.version = '3.0.0'; + + fs.writeJsonSync(join(app.dir, 'package.json'), pkgJson); + fs.writeJsonSync(join(app.dir, 'node_modules', 'lodash', 'package.json'), pkgJsonLodash); + + let lodashThreeRun = await app.execute(`cross-env LODASH_VERSION=three yarn test`); + assert.equal(lodashThreeRun.exitCode, 0, lodashThreeRun.output); + }); }); }); diff --git a/tests/scenarios/package.json b/tests/scenarios/package.json index 8d863dc5a..34ef8ffac 100644 --- a/tests/scenarios/package.json +++ b/tests/scenarios/package.json @@ -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", @@ -29,8 +30,8 @@ "ember-cli-3.20": "npm:ember-cli@~3.20.0", "ember-cli-3.24": "npm:ember-cli@~3.24.0", "ember-cli-beta": "npm:ember-cli@beta", - "ember-cli-latest": "npm:ember-cli@latest", "ember-cli-fastboot": "^2.2.3", + "ember-cli-latest": "npm:ember-cli@latest", "ember-composable-helpers": "^4.4.1", "ember-data-3.16": "npm:ember-data@~3.16.0", "ember-data-3.20": "npm:ember-data@~3.20.0", @@ -38,11 +39,11 @@ "ember-data-latest": "npm:ember-data@latest", "ember-engines": "^0.8.16", "ember-inline-svg": "^0.2.1", - "ember-source-latest": "npm:ember-source@latest", - "ember-source-beta": "npm:ember-source@beta", "ember-source-3.16": "npm:ember-source@~3.16.0", "ember-source-3.20": "npm:ember-source@~3.20.0", "ember-source-3.24": "npm:ember-source@~3.24.0", + "ember-source-beta": "npm:ember-source@beta", + "ember-source-latest": "npm:ember-source@latest", "ember-truth-helpers": "^3.0.0", "fastboot-addon": "link:../../test-packages/fastboot-addon" }, diff --git a/yarn.lock b/yarn.lock index 827180565..3fbecf00b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5673,15 +5673,15 @@ browserify-zlib@^0.2.0: pako "~1.0.5" browserslist@^3.2.6, browserslist@^4.0.0, browserslist@^4.14.0, browserslist@^4.14.5, browserslist@^4.16.6: - version "4.16.6" - resolved "https://registry.yarnpkg.com/browserslist/-/browserslist-4.16.6.tgz#d7901277a5a88e554ed305b183ec9b0c08f66fa2" - integrity sha512-Wspk/PqO+4W9qp5iUTJsa1B/QrYn1keNCcEP5OvP7WBwT4KaDly0uONYmC6Xa3Z5IqnUgS0KcgLYu1l74x0ZXQ== + version "4.16.7" + resolved "https://registry.yarnpkg.com/browserslist/-/browserslist-4.16.7.tgz#108b0d1ef33c4af1b587c54f390e7041178e4335" + integrity sha512-7I4qVwqZltJ7j37wObBe3SoTz+nS8APaNcrBOlgoirb6/HbEU2XxW/LpUDTCngM6iauwFqmRTuOMfyKnFGY5JA== dependencies: - caniuse-lite "^1.0.30001219" + caniuse-lite "^1.0.30001248" colorette "^1.2.2" - electron-to-chromium "^1.3.723" + electron-to-chromium "^1.3.793" escalade "^3.1.1" - node-releases "^1.1.71" + node-releases "^1.1.73" bser@2.1.1: version "2.1.1" @@ -5917,11 +5917,16 @@ caniuse-api@^3.0.0: lodash.memoize "^4.1.2" lodash.uniq "^4.5.0" -caniuse-lite@^1.0.0, caniuse-lite@^1.0.30001219: +caniuse-lite@^1.0.0: version "1.0.30001236" resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001236.tgz#0a80de4cdf62e1770bb46a30d884fc8d633e3958" integrity sha512-o0PRQSrSCGJKCPZcgMzl5fUaj5xHe8qA2m4QRvnyY4e1lITqoNkr7q/Oh1NcpGSy0Th97UZ35yoKcINPoq7YOQ== +caniuse-lite@^1.0.30001248: + version "1.0.30001248" + resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001248.tgz#26ab45e340f155ea5da2920dadb76a533cb8ebce" + integrity sha512-NwlQbJkxUFJ8nMErnGtT0QTM2TJ33xgz4KXJSMIrjXIbDVdaYueGyjOrLKRtJC+rTiWfi6j5cnZN1NBiSBJGNw== + capture-exit@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/capture-exit/-/capture-exit-2.0.0.tgz#fb953bfaebeb781f62898239dabb426d08a509a4" @@ -7181,10 +7186,10 @@ ee-first@1.1.1: resolved "https://registry.yarnpkg.com/ee-first/-/ee-first-1.1.1.tgz#590c61156b0ae2f4f0255732a158b266bc56b21d" integrity sha1-WQxhFWsK4vTwJVcyoViyZrxWsh0= -electron-to-chromium@^1.3.723: - version "1.3.752" - resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.752.tgz#0728587f1b9b970ec9ffad932496429aef750d09" - integrity sha512-2Tg+7jSl3oPxgsBsWKh5H83QazTkmWG/cnNwJplmyZc7KcN61+I10oUgaXSVk/NwfvN3BdkKDR4FYuRBQQ2v0A== +electron-to-chromium@^1.3.793: + version "1.3.796" + resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.796.tgz#bd74a4367902c9d432d129f265bf4542cddd9f54" + integrity sha512-agwJFgM0FUC1UPPbQ4aII3HamaaJ09fqWGAWYHmzxDWqdmTleCHyyA0kt3fJlTd5M440IaeuBfzXzXzCotnZcQ== elliptic@^6.5.3: version "6.5.4" @@ -10982,6 +10987,15 @@ fs-extra@^0.30.0: path-is-absolute "^1.0.0" rimraf "^2.2.8" +fs-extra@^10.0.0: + version "10.0.0" + resolved "https://registry.yarnpkg.com/fs-extra/-/fs-extra-10.0.0.tgz#9ff61b655dde53fb34a82df84bb214ce802e17c1" + integrity sha512-C5owb14u9eJwizKGdchcDUQeFtlSHHthBk8pbX9Vc1PFZrLombudjDnNns88aYslCyF6IY5SUw3Roz6xShcEIQ== + dependencies: + graceful-fs "^4.2.0" + jsonfile "^6.0.1" + universalify "^2.0.0" + fs-extra@^3.0.1: version "3.0.1" resolved "https://registry.yarnpkg.com/fs-extra/-/fs-extra-3.0.1.tgz#3794f378c58b342ea7dbbb23095109c4b3b62291" @@ -14563,7 +14577,7 @@ node-notifier@^9.0.1: uuid "^8.3.0" which "^2.0.2" -node-releases@^1.1.71: +node-releases@^1.1.73: version "1.1.73" resolved "https://registry.yarnpkg.com/node-releases/-/node-releases-1.1.73.tgz#dd4e81ddd5277ff846b80b52bb40c49edf7a7b20" integrity sha512-uW7fodD6pyW2FZNZnp/Z3hvWKeEW1Y8R1+1CnErE8cXFXzl5blBOoVB41CvMer6P6Q0S5FXDwcHgFd1Wj0U9zg== From 7cd3589eab5c11430363d74316083727e2e6e87a Mon Sep 17 00:00:00 2001 From: Travis Hoover Date: Wed, 4 Aug 2021 16:57:23 -0700 Subject: [PATCH 03/10] cleaning up --- tests/fixtures/macro-test/tests/acceptance/basic-test.js | 2 +- tests/scenarios/package.json | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/fixtures/macro-test/tests/acceptance/basic-test.js b/tests/fixtures/macro-test/tests/acceptance/basic-test.js index d5141189e..6d866ddf4 100644 --- a/tests/fixtures/macro-test/tests/acceptance/basic-test.js +++ b/tests/fixtures/macro-test/tests/acceptance/basic-test.js @@ -51,7 +51,7 @@ module('Acceptance | smoke tests', function (hooks) { ]); }); - test('foobar', async function (assert) { + test('dependency satisfies works correctly', async function (assert) { await visit('/'); assert.equal(currentURL(), '/'); diff --git a/tests/scenarios/package.json b/tests/scenarios/package.json index 34ef8ffac..c325e4c86 100644 --- a/tests/scenarios/package.json +++ b/tests/scenarios/package.json @@ -30,8 +30,8 @@ "ember-cli-3.20": "npm:ember-cli@~3.20.0", "ember-cli-3.24": "npm:ember-cli@~3.24.0", "ember-cli-beta": "npm:ember-cli@beta", - "ember-cli-fastboot": "^2.2.3", "ember-cli-latest": "npm:ember-cli@latest", + "ember-cli-fastboot": "^2.2.3", "ember-composable-helpers": "^4.4.1", "ember-data-3.16": "npm:ember-data@~3.16.0", "ember-data-3.20": "npm:ember-data@~3.20.0", @@ -39,11 +39,11 @@ "ember-data-latest": "npm:ember-data@latest", "ember-engines": "^0.8.16", "ember-inline-svg": "^0.2.1", + "ember-source-beta": "npm:ember-source@beta", + "ember-source-latest": "npm:ember-source@latest", "ember-source-3.16": "npm:ember-source@~3.16.0", "ember-source-3.20": "npm:ember-source@~3.20.0", "ember-source-3.24": "npm:ember-source@~3.24.0", - "ember-source-beta": "npm:ember-source@beta", - "ember-source-latest": "npm:ember-source@latest", "ember-truth-helpers": "^3.0.0", "fastboot-addon": "link:../../test-packages/fastboot-addon" }, From 351f07e82642c6974623fab8de2b340f55441b5f Mon Sep 17 00:00:00 2001 From: Travis Hoover Date: Wed, 4 Aug 2021 16:58:45 -0700 Subject: [PATCH 04/10] more cleanup --- tests/scenarios/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scenarios/package.json b/tests/scenarios/package.json index c325e4c86..02e231bf7 100644 --- a/tests/scenarios/package.json +++ b/tests/scenarios/package.json @@ -39,8 +39,8 @@ "ember-data-latest": "npm:ember-data@latest", "ember-engines": "^0.8.16", "ember-inline-svg": "^0.2.1", - "ember-source-beta": "npm:ember-source@beta", "ember-source-latest": "npm:ember-source@latest", + "ember-source-beta": "npm:ember-source@beta", "ember-source-3.16": "npm:ember-source@~3.16.0", "ember-source-3.20": "npm:ember-source@~3.20.0", "ember-source-3.24": "npm:ember-source@~3.24.0", From f830fe4f79b5ee2e160ce2144a5f8e85bd9e4b6c Mon Sep 17 00:00:00 2001 From: Travis Hoover Date: Fri, 6 Aug 2021 12:51:24 -0700 Subject: [PATCH 05/10] addressing comments --- packages/core/src/app.ts | 2 +- packages/macros/package.json | 1 + packages/macros/src/ember-addon-main.ts | 35 +-------------------- packages/macros/src/macros-config.ts | 31 ++++++++++++++++-- packages/macros/tests/babel/helpers.ts | 4 +-- tests/scenarios/macro-test.ts | 42 ++++++++++++++++++------- 6 files changed, 64 insertions(+), 51 deletions(-) diff --git a/packages/core/src/app.ts b/packages/core/src/app.ts index 747b0f2e0..acbc572a4 100644 --- a/packages/core/src/app.ts +++ b/packages/core/src/app.ts @@ -393,7 +393,7 @@ export class AppBuilder { ]); // 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')]); diff --git a/packages/macros/package.json b/packages/macros/package.json index e34881645..9493775b6 100644 --- a/packages/macros/package.json +++ b/packages/macros/package.json @@ -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" diff --git a/packages/macros/src/ember-addon-main.ts b/packages/macros/src/ember-addon-main.ts index 45ccfcb31..cfa908521 100644 --- a/packages/macros/src/ember-addon-main.ts +++ b/packages/macros/src/ember-addon-main.ts @@ -1,6 +1,4 @@ -import fs from 'fs'; import { join } from 'path'; -import crypto from 'crypto'; import { MacrosConfig, isEmbroiderMacrosPlugin } from './node'; export = { @@ -71,38 +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)); - - let yarnLockPath = join(appInstance.project.root, 'yarn.lock'); - let npmLockPath = join(appInstance.project.root, 'package-lock.json'); - let pnpmLockPath = join(appInstance.project.root, 'pnpm-lock.yaml'); - let packagePath = join(appInstance.project.root, 'package.json'); - let lockFileBuffer; - - if (fs.existsSync(yarnLockPath)) { - lockFileBuffer = fs.readFileSync(yarnLockPath); - } else if (fs.existsSync(npmLockPath)) { - lockFileBuffer = fs.readFileSync(npmLockPath); - } else if (fs.existsSync(pnpmLockPath)) { - lockFileBuffer = fs.readFileSync(pnpmLockPath); - } else { - // no lock file found, using package.json as a fall back - lockFileBuffer = fs.readFileSync(packagePath); - } - - // @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'); - babelPlugins.push([ - require.resolve('@embroider/shared-internals/src/babel-plugin-cache-busting.js'), - { version: cacheKey }, - '@embroider/macros cache buster', - ]); + babelOptions.plugins = babelPlugins.concat(MacrosConfig.for(appInstance).babelPluginConfig(source)); } }, diff --git a/packages/macros/src/macros-config.ts b/packages/macros/src/macros-config.ts index b121390d2..41fe8a310 100644 --- a/packages/macros/src/macros-config.ts +++ b/packages/macros/src/macros-config.ts @@ -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'; @@ -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 @@ -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']); + + if (!lockFilePath) { + lockFilePath = findUp.sync('package.json'); + } + + 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): { diff --git a/packages/macros/tests/babel/helpers.ts b/packages/macros/tests/babel/helpers.ts index ddd04fbf9..4f6bde184 100644 --- a/packages/macros/tests/babel/helpers.ts +++ b/packages/macros/tests/babel/helpers.ts @@ -54,7 +54,7 @@ export function makeBabelConfig(_babelVersion: number, macroConfig: MacrosConfig return { filename: join(__dirname, 'sample.js'), presets: [], - plugins: [macroConfig.babelPluginConfig()], + plugins: macroConfig.babelPluginConfig(), }; } @@ -100,7 +100,7 @@ export function allBabelVersions(createTests: CreateTests | CreateTestsWithConfi return { filename: join(__dirname, 'sample.js'), presets: [], - plugins: [config.babelPluginConfig()], + plugins: config.babelPluginConfig(), }; }, diff --git a/tests/scenarios/macro-test.ts b/tests/scenarios/macro-test.ts index 4f866e041..58b4a059b 100644 --- a/tests/scenarios/macro-test.ts +++ b/tests/scenarios/macro-test.ts @@ -7,6 +7,17 @@ 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')), { @@ -36,8 +47,12 @@ appScenarios .forEachScenario(scenario => { Qmodule(scenario.name, function (hooks) { let app: PreparedApp; + let pkgJson: any; + let pkgJsonLodash: any; + hooks.before(async () => { app = await scenario.prepare(); + updateLodashVersion(app, '4.0.0'); }); test(`yarn test`, async function (assert) { @@ -56,27 +71,30 @@ appScenarios assert.equal(result.exitCode, 0, result.output); }); - test(`multiple dependency check`, async function (assert) { - let pkgJson = fs.readJsonSync(join(app.dir, 'package.json')); - let pkgJsonLodash = fs.readJsonSync(join(app.dir, 'node_modules', 'lodash', 'package.json')); + test(`@embroider/macros babel caching plugin works`, async function (assert) { + let lodashFourRun = await app.execute(`yarn test`); + assert.equal(lodashFourRun.exitCode, 0, lodashFourRun.output); - pkgJson.devDependencies.lodash = '4.0.0'; - pkgJsonLodash.version = '4.0.0'; + // simulate a different version being installed + updateLodashVersion(app, '3.0.0'); - fs.writeJsonSync(join(app.dir, 'package.json'), pkgJson); - fs.writeJsonSync(join(app.dir, 'node_modules', 'lodash', 'package.json'), pkgJsonLodash); + let lodashThreeRun = await app.execute(`cross-env LODASH_VERSION=three yarn test`); + assert.equal(lodashThreeRun.exitCode, 0, lodashThreeRun.output); + }); - let lodashFourRun = await app.execute(`yarn test`); + 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); - // downgrade lodash to 3.0.0 - pkgJson.devDependencies.lodash = '3.0.0'; - pkgJsonLodash.version = '3.0.0'; + // simulate a different version being installed + updateLodashVersion(app, '3.0.0'); fs.writeJsonSync(join(app.dir, 'package.json'), pkgJson); fs.writeJsonSync(join(app.dir, 'node_modules', 'lodash', 'package.json'), pkgJsonLodash); - let lodashThreeRun = await app.execute(`cross-env LODASH_VERSION=three yarn test`); + let lodashThreeRun = await app.execute(`cross-env LODASH_VERSION=three CLASSIC=true yarn test`); assert.equal(lodashThreeRun.exitCode, 0, lodashThreeRun.output); }); }); From 751f13df7545530adc3b0ddadd534ad6274378c6 Mon Sep 17 00:00:00 2001 From: Travis Hoover Date: Fri, 6 Aug 2021 13:33:16 -0700 Subject: [PATCH 06/10] fix test --- tests/scenarios/macro-test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/scenarios/macro-test.ts b/tests/scenarios/macro-test.ts index 58b4a059b..5bc192dd6 100644 --- a/tests/scenarios/macro-test.ts +++ b/tests/scenarios/macro-test.ts @@ -91,9 +91,6 @@ appScenarios // simulate a different version being installed updateLodashVersion(app, '3.0.0'); - fs.writeJsonSync(join(app.dir, 'package.json'), pkgJson); - fs.writeJsonSync(join(app.dir, 'node_modules', 'lodash', 'package.json'), pkgJsonLodash); - let lodashThreeRun = await app.execute(`cross-env LODASH_VERSION=three CLASSIC=true yarn test`); assert.equal(lodashThreeRun.exitCode, 0, lodashThreeRun.output); }); From edef9bcdac0d78861f1d9adddd1edf25a49f17c2 Mon Sep 17 00:00:00 2001 From: Travis Hoover Date: Fri, 6 Aug 2021 13:39:16 -0700 Subject: [PATCH 07/10] fixin linting --- tests/scenarios/macro-test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/scenarios/macro-test.ts b/tests/scenarios/macro-test.ts index 5bc192dd6..bfd398fe5 100644 --- a/tests/scenarios/macro-test.ts +++ b/tests/scenarios/macro-test.ts @@ -47,8 +47,6 @@ appScenarios .forEachScenario(scenario => { Qmodule(scenario.name, function (hooks) { let app: PreparedApp; - let pkgJson: any; - let pkgJsonLodash: any; hooks.before(async () => { app = await scenario.prepare(); From 51464fa60bc6e8294ee1c1c7a5b4333a4de0e07b Mon Sep 17 00:00:00 2001 From: Travis Hoover Date: Fri, 6 Aug 2021 14:30:57 -0700 Subject: [PATCH 08/10] adding cwd location --- packages/macros/src/macros-config.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/macros/src/macros-config.ts b/packages/macros/src/macros-config.ts index 41fe8a310..d1ccd77f5 100644 --- a/packages/macros/src/macros-config.ts +++ b/packages/macros/src/macros-config.ts @@ -285,10 +285,10 @@ export default class MacrosConfig { importSyncImplementation: this.importSyncImplementation, }; - let lockFilePath = findUp.sync(['yarn.lock', 'package-lock.json', 'pnpm-lock.yaml']); + let lockFilePath = findUp.sync(['yarn.lock', 'package-lock.json', 'pnpm-lock.yaml'], { cwd: opts.appPackageRoot }); if (!lockFilePath) { - lockFilePath = findUp.sync('package.json'); + lockFilePath = findUp.sync('package.json', { cwd: opts.appPackageRoot }); } let lockFileBuffer = lockFilePath ? fs.readFileSync(lockFilePath) : 'no-cache-key'; From 895cb002f54427dcc3432b010793db4ca8b39fda Mon Sep 17 00:00:00 2001 From: Travis Hoover Date: Mon, 9 Aug 2021 10:46:19 -0700 Subject: [PATCH 09/10] use splat unshift --- packages/macros/src/ember-addon-main.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/macros/src/ember-addon-main.ts b/packages/macros/src/ember-addon-main.ts index cfa908521..d5e1b2a0f 100644 --- a/packages/macros/src/ember-addon-main.ts +++ b/packages/macros/src/ember-addon-main.ts @@ -69,7 +69,7 @@ export = { if (!babelPlugins.some(isEmbroiderMacrosPlugin)) { let appInstance = this._findHost(); let source = appOrAddonInstance.root || appOrAddonInstance.project.root; - babelOptions.plugins = babelPlugins.concat(MacrosConfig.for(appInstance).babelPluginConfig(source)); + babelPlugins.unshift(...MacrosConfig.for(appInstance).babelPluginConfig(source)); } }, From 1de3ec9b93b553b662eb53b44d593f257fbd4011 Mon Sep 17 00:00:00 2001 From: Travis Hoover Date: Mon, 9 Aug 2021 11:05:31 -0700 Subject: [PATCH 10/10] moving back to splat push --- packages/core/src/app.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/app.ts b/packages/core/src/app.ts index acbc572a4..ff5a0330c 100644 --- a/packages/core/src/app.ts +++ b/packages/core/src/app.ts @@ -393,7 +393,7 @@ export class AppBuilder { ]); // this is @embroider/macros configured for full stage3 resolution - babel.plugins = babel.plugins.concat(this.macrosConfig.babelPluginConfig()); + babel.plugins.push(...this.macrosConfig.babelPluginConfig()); babel.plugins.push([require.resolve('./template-colocation-plugin')]);