Merge f88f04e into c9a4abe

adobe · Aug 11, 2021 · c51e165 · c51e165
2 parents c9a4abe + f88f04e
commit c51e165
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 95 deletions.
diff --git a/src/__tests__/createSettingsFileTransformer.test.js b/src/__tests__/createSettingsFileTransformer.test.js
@@ -54,7 +54,7 @@ describe('isDynamicEnforced=false', function () {
     });
 
     it('handles null', function () {
-      expect(settingsFileTransformer(null, null, null)).toBe(null);
+      expect(settingsFileTransformer(null, null, null)).toBe(undefined);
     });
 
     it('reflects back out a settings object untouched', function () {
@@ -67,14 +67,22 @@ describe('isDynamicEnforced=false', function () {
         ],
         someUrl: '/some/relativeUrl'
       };
+      var oldReference = oldSettings;
 
-      var newSettings = settingsFileTransformer(oldSettings, [
-        'someUrl',
-        'key2[].someUrl'
-      ]);
+      var expectedSettings = {
+        key1: 'foo',
+        key2: [
+          {
+            someUrl: '/some/relative/url'
+          }
+        ],
+        someUrl: '/some/relativeUrl'
+      };
+
+      settingsFileTransformer(oldSettings, ['someUrl', 'key2[].someUrl']);
 
-      expect(oldSettings).toEqual(newSettings);
-      expect(oldSettings === newSettings).toBeTrue();
+      expect(oldSettings).toEqual(expectedSettings);
+      expect(oldReference === oldSettings).toBeTrue();
     });
   });
 });
@@ -96,7 +104,7 @@ describe('isDynamicEnforced=true', function () {
     });
 
     it('handles null', function () {
-      expect(settingsFileTransformer(null, null, null)).toBe(null);
+      expect(settingsFileTransformer(null, null, null)).toBe(undefined);
     });
   });
 
@@ -105,17 +113,13 @@ describe('isDynamicEnforced=true', function () {
       source: '/some/relative/url'
     };
 
-    var newSettings = settingsFileTransformer(
-      oldSettings,
-      ['source'],
-      moduleReferencePath
-    );
+    settingsFileTransformer(oldSettings, ['source'], moduleReferencePath);
 
     var expectedNewSettings = {
       source: 'https://assets.adobedtm.com/some/relative/url'
     };
 
-    expect(newSettings).toEqual(expectedNewSettings);
+    expect(oldSettings).toEqual(expectedNewSettings);
   });
 
   it('can update a top level setting (array)', function () {
@@ -135,7 +139,7 @@ describe('isDynamicEnforced=true', function () {
       ]
     };
 
-    var newSettings = settingsFileTransformer(
+    settingsFileTransformer(
       oldSettings,
       ['sources[].someUrl'],
       moduleReferencePath
@@ -157,7 +161,7 @@ describe('isDynamicEnforced=true', function () {
       ]
     };
 
-    expect(newSettings).toEqual(expectedNewSettings);
+    expect(oldSettings).toEqual(expectedNewSettings);
   });
 
   it('can update a top level setting (object)', function () {
@@ -173,11 +177,7 @@ describe('isDynamicEnforced=true', function () {
       topValue: '/some/relative/url'
     };
 
-    var newSettings = settingsFileTransformer(
-      oldSettings,
-      ['topValue'],
-      moduleReferencePath
-    );
+    settingsFileTransformer(oldSettings, ['topValue'], moduleReferencePath);
 
     var expectedNewSettings = {
       a: {
@@ -191,7 +191,7 @@ describe('isDynamicEnforced=true', function () {
       topValue: 'https://assets.adobedtm.com/some/relative/url'
     };
 
-    expect(newSettings).toEqual(expectedNewSettings);
+    expect(oldSettings).toEqual(expectedNewSettings);
   });
 
   it('can update a nested object', function () {
@@ -211,11 +211,7 @@ describe('isDynamicEnforced=true', function () {
       ]
     };
 
-    var newSettings = settingsFileTransformer(
-      oldSettings,
-      ['a.b.someUrl'],
-      moduleReferencePath
-    );
+    settingsFileTransformer(oldSettings, ['a.b.someUrl'], moduleReferencePath);
 
     var expectedNewSettings = {
       a: {
@@ -233,7 +229,7 @@ describe('isDynamicEnforced=true', function () {
       ]
     };
 
-    expect(newSettings).toEqual(expectedNewSettings);
+    expect(oldSettings).toEqual(expectedNewSettings);
   });
 
   it('can update a nested object', function () {
@@ -253,11 +249,7 @@ describe('isDynamicEnforced=true', function () {
       ]
     };
 
-    var newSettings = settingsFileTransformer(
-      oldSettings,
-      ['a.b.someUrl'],
-      moduleReferencePath
-    );
+    settingsFileTransformer(oldSettings, ['a.b.someUrl'], moduleReferencePath);
 
     var expectedNewSettings = {
       a: {
@@ -275,7 +267,7 @@ describe('isDynamicEnforced=true', function () {
       ]
     };
 
-    expect(newSettings).toEqual(expectedNewSettings);
+    expect(oldSettings).toEqual(expectedNewSettings);
   });
 
   it('does not unexpectedly transform a setting too early', function () {
@@ -301,7 +293,7 @@ describe('isDynamicEnforced=true', function () {
       }
     };
 
-    var newSettings = settingsFileTransformer(
+    settingsFileTransformer(
       oldSettings,
       ['a.someUrl.nestedList[].someUrl'],
       moduleReferencePath
@@ -329,7 +321,7 @@ describe('isDynamicEnforced=true', function () {
       }
     };
 
-    expect(newSettings).toEqual(expectedNewSettings);
+    expect(oldSettings).toEqual(expectedNewSettings);
   });
 
   it('transforms many file paths', function () {
@@ -374,7 +366,7 @@ describe('isDynamicEnforced=true', function () {
       ]
     };
 
-    var newSettings = settingsFileTransformer(
+    settingsFileTransformer(
       oldSettings,
       [
         'a.someUrl.nestedList[].someUrl',
@@ -427,7 +419,7 @@ describe('isDynamicEnforced=true', function () {
       ]
     };
 
-    expect(newSettings).toEqual(expectedNewSettings);
+    expect(oldSettings).toEqual(expectedNewSettings);
   });
 
   describe('leaves values alone that are not strings', function () {
@@ -436,13 +428,9 @@ describe('isDynamicEnforced=true', function () {
         someKey: 5
       };
 
-      var newSettings = settingsFileTransformer(
-        oldSettings,
-        ['someKey'],
-        moduleReferencePath
-      );
+      settingsFileTransformer(oldSettings, ['someKey'], moduleReferencePath);
 
-      expect(oldSettings).toEqual(newSettings);
+      expect(oldSettings).toEqual({ someKey: 5 });
     });
 
     it('boolean check', function () {
@@ -452,7 +440,7 @@ describe('isDynamicEnforced=true', function () {
 
       var newSettings = settingsFileTransformer(oldSettings, ['someKey']);
 
-      expect(oldSettings).toEqual(newSettings);
+      expect(oldSettings).toEqual({ someKey: true });
     });
 
     it('object check', function () {
@@ -466,21 +454,17 @@ describe('isDynamicEnforced=true', function () {
         moduleReferencePath
       );
 
-      expect(oldSettings).toEqual(newSettings);
+      expect(oldSettings).toEqual({ someKey: { isAnObject: true } });
     });
 
     it('array check', function () {
       var oldSettings = {
         someKey: ['is', 'a', 'list']
       };
 
-      var newSettings = settingsFileTransformer(
-        oldSettings,
-        ['someKey'],
-        moduleReferencePath
-      );
+      settingsFileTransformer(oldSettings, ['someKey'], moduleReferencePath);
 
-      expect(oldSettings).toEqual(newSettings);
+      expect(oldSettings).toEqual({ someKey: ['is', 'a', 'list'] });
     });
   });
 
@@ -490,33 +474,36 @@ describe('isDynamicEnforced=true', function () {
       someOtherKey: '/some/relative/url'
     };
 
-    var newSettings = settingsFileTransformer(
+    settingsFileTransformer(
       oldSettings,
       ['this.key[].doesNotExist'],
       moduleReferencePath
     );
 
-    expect(oldSettings).toEqual(newSettings);
+    expect(oldSettings).toEqual({
+      someKey: { isAnObject: true },
+      someOtherKey: '/some/relative/url'
+    });
   });
 
   describe('handles the Adobe Custom Code action correctly', function () {
     beforeEach(function () {
       moduleReferencePath = 'core/src/lib/actions/customCode.js';
     });
 
-    it('does not transform when isExternal is not present', function () {
+    // @TODO: make sure this makes sense
+    it('transforms when isExternal is not present', function () {
       var oldSettings = {
         source: '/some/relative/url',
         isExternal: undefined
       };
 
-      var newSettings = settingsFileTransformer(
-        oldSettings,
-        ['source'],
-        moduleReferencePath
-      );
+      settingsFileTransformer(oldSettings, ['source'], moduleReferencePath);
 
-      expect(oldSettings).toEqual(newSettings);
+      expect(oldSettings).toEqual({
+        source: 'https://assets.adobedtm.com/some/relative/url',
+        isExternal: undefined
+      });
     });
 
     it('does not transform when isExternal=false', function () {
@@ -525,13 +512,12 @@ describe('isDynamicEnforced=true', function () {
         isExternal: false
       };
 
-      var newSettings = settingsFileTransformer(
-        oldSettings,
-        ['source'],
-        moduleReferencePath
-      );
+      settingsFileTransformer(oldSettings, ['source'], moduleReferencePath);
 
-      expect(oldSettings).toEqual(newSettings);
+      expect(oldSettings).toEqual({
+        source: '/some/relative/url',
+        isExternal: false
+      });
     });
 
     it('transforms when isExternal=true', function () {
@@ -540,18 +526,14 @@ describe('isDynamicEnforced=true', function () {
         isExternal: true
       };
 
-      var newSettings = settingsFileTransformer(
-        oldSettings,
-        ['source'],
-        moduleReferencePath
-      );
+      settingsFileTransformer(oldSettings, ['source'], moduleReferencePath);
 
       var expectedNewSettings = {
         source: 'https://assets.adobedtm.com/some/relative/url',
         isExternal: true
       };
 
-      expect(newSettings).toEqual(expectedNewSettings);
+      expect(oldSettings).toEqual(expectedNewSettings);
     });
   });
 });
diff --git a/src/createSettingsFileTransformer.js b/src/createSettingsFileTransformer.js
@@ -97,26 +97,14 @@ module.exports = function (isDynamicEnforced, decorateWithDynamicHost) {
       !Array.isArray(filePaths) ||
       !filePaths.length
     ) {
-      return settings;
+      return;
     }
 
     // pull out the file paths by the module's reference path and loop over each urlPath
     filePaths.forEach(function (filePathString) {
-      // The custom code action provides the ability to have the source code in the 'source'
-      // variable or to have an external file. Therefore, this module has 2 behaviors.
-      // It also does not provide a value of false for isExternal just as all other extensions
-      // that use fileTransform do not provide an isExternal variable check. Therefore, we need
-      // to treat Adobe's custom code action special, and don't augment the 'source' variable
-      // if isExternal is not also present.
-      var isAdobeCustomCodeAction = Boolean(
-        moduleReferencePath != null &&
-          /^core\/.*actions.*\/customCode\.js$/.test(moduleReferencePath)
-      );
-      if (
-        isAdobeCustomCodeAction &&
-        filePathString === 'source' &&
-        !settings.isExternal
-      ) {
+      // isExternal is not emitted for extensions that use the fileTransform, however CustomCode
+      // emits an explicit true or false.
+      if (settings.isExternal === false) {
         return;
       }
 
@@ -128,6 +116,6 @@ module.exports = function (isDynamicEnforced, decorateWithDynamicHost) {
       );
     });
 
-    return settings;
+    return;
   };
 };
diff --git a/src/hydrateModuleProvider.js b/src/hydrateModuleProvider.js
@@ -39,12 +39,9 @@ module.exports = function (
 
     Object.keys(extensions).forEach(function (extensionName) {
       var extension = extensions[extensionName];
-      var extensionSettings = extension.settings;
+      var extensionSettings = extension.settings || {};
       if (Array.isArray(extension.filePaths)) {
-        extensionSettings = settingsFileTransformer(
-          extensionSettings,
-          extension.filePaths
-        );
+        settingsFileTransformer(extensionSettings, extension.filePaths);
       }
       var getExtensionSettings = createGetExtensionSettings(
         replaceTokens,