Ensure app macro configs take precedence over addon configs

embroider-build · Jul 5, 2021 · 2f82a76 · 2f82a76
1 parent 81ca52f
commit 2f82a76
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 9 deletions.
diff --git a/packages/macros/src/ember-addon-main.ts b/packages/macros/src/ember-addon-main.ts
@@ -14,13 +14,24 @@ export = {
     // if parent is an addon it has root. If it's an app it has project.root.
     let source = parent.root || parent.project.root;
 
+    // In order to allow to overwrite config from consumed addons, we keep a priority
+    // This is stripped before being returned via getConfig/getOwnConfig
+    // Own config has lowest priority - e.g. default values for an addon
+    // Addons can overwrite other addon's config (priority 1)
+    // The host app has highest priority (priority 2)
     if (ownOptions.setOwnConfig) {
-      MacrosConfig.for(appInstance).setOwnConfig(source, ownOptions.setOwnConfig);
+      MacrosConfig.for(appInstance).setOwnConfig(source, Object.assign({ __priority: 0 }, ownOptions.setOwnConfig));
     }
 
     if (ownOptions.setConfig) {
+      let parentIsHostApp = parent === appInstance;
+
       for (let [packageName, config] of Object.entries(ownOptions.setConfig)) {
-        MacrosConfig.for(appInstance).setConfig(source, packageName, config);
+        MacrosConfig.for(appInstance).setConfig(
+          source,
+          packageName,
+          Object.assign({ __priority: parentIsHostApp ? 2 : 1 }, config)
+        );
       }
     }
 

diff --git a/packages/macros/src/macros-config.ts b/packages/macros/src/macros-config.ts
@@ -210,7 +210,9 @@ export default class MacrosConfig {
         if (configs.length > 1) {
           combined = this.mergerFor(pkgRoot)(configs);
         } else {
-          combined = configs[0];
+          let config = configs[0] as unknown & { __priority?: number };
+          delete config.__priority;
+          combined = config;
         }
         userConfigs[pkgRoot] = combined;
       }
@@ -339,5 +341,19 @@ export default class MacrosConfig {
 }
 
 function defaultMerger(configs: unknown[]): unknown {
-  return Object.assign({}, ...configs);
+  configs.sort((a, b) => {
+    let priorityA = (a as unknown & { __priority?: number }).__priority || 0;
+    let priorityB = (b as unknown & { __priority?: number }).__priority || 0;
+
+    if (priorityA === priorityB) {
+      return 0;
+    }
+
+    return priorityA > priorityB ? 1 : -1;
+  });
+
+  let mergedConfig = Object.assign({}, ...configs);
+  delete mergedConfig.__priority;
+
+  return mergedConfig;
 }
diff --git a/tests/fixtures/macro-sample-addon/index.js b/tests/fixtures/macro-sample-addon/index.js
@@ -5,7 +5,8 @@ module.exports = {
   options: {
     '@embroider/macros': {
       setOwnConfig: {
-        hello: 'world',
+        shouldBeOverwritten: 'not overwritten',
+        configFromAddonItself: 'this is the addon',
       },
     },
   },

diff --git a/tests/fixtures/macro-test/ember-cli-build.js b/tests/fixtures/macro-test/ember-cli-build.js
@@ -19,6 +19,7 @@ module.exports = function (defaults) {
         },
         'macro-sample-addon': {
           configFromMacrosTests: 'exists',
+          shouldBeOverwritten: 'overwritten',
         },
       },
     },

diff --git a/tests/fixtures/macro-test/tests/integration/components/cross-package-config-test.js b/tests/fixtures/macro-test/tests/integration/components/cross-package-config-test.js
@@ -13,22 +13,34 @@ module('Integration | cross-package-config', function (hooks) {
     this.owner.register(
       'helper:my-assertion',
       helper(function ([value]) {
-        assert.deepEqual(value, { hello: 'world', configFromMacrosTests: 'exists' });
+        assert.deepEqual(value, {
+          shouldBeOverwritten: 'overwritten',
+          configFromAddonItself: 'this is the addon',
+          configFromMacrosTests: 'exists',
+        });
       })
     );
     await render(hbs`{{my-assertion (reflect-config)}}`);
   });
 
   test(`app's JS can see addon's merged config`, async function (assert) {
-    assert.deepEqual(reflectAddonConfig(), { hello: 'world', configFromMacrosTests: 'exists' });
+    assert.deepEqual(reflectAddonConfig(), {
+      shouldBeOverwritten: 'overwritten',
+      configFromAddonItself: 'this is the addon',
+      configFromMacrosTests: 'exists',
+    });
   });
 
   test(`addon's HBS can see addon's merged config`, async function (assert) {
     assert.expect(1);
     this.owner.register(
       'helper:my-assertion',
       helper(function ([value]) {
-        assert.deepEqual(value, { hello: 'world', configFromMacrosTests: 'exists' });
+        assert.deepEqual(value, {
+          shouldBeOverwritten: 'overwritten',
+          configFromAddonItself: 'this is the addon',
+          configFromMacrosTests: 'exists',
+        });
       })
     );
     await render(hbs`{{#reflect-hbs-config as |config|}} {{my-assertion config}} {{/reflect-hbs-config}}`);
@@ -39,7 +51,11 @@ module('Integration | cross-package-config', function (hooks) {
     this.owner.register(
       'helper:my-assertion',
       helper(function ([value]) {
-        assert.deepEqual(value, { hello: 'world', configFromMacrosTests: 'exists' });
+        assert.deepEqual(value, {
+          shouldBeOverwritten: 'overwritten',
+          configFromAddonItself: 'this is the addon',
+          configFromMacrosTests: 'exists',
+        });
       })
     );
     await render(hbs`{{my-assertion (macroGetConfig "macro-sample-addon" )}}`);