feat(warning): catch property name collisions during file output (#273)

amzn · May 28, 2019 · 9a40407 · 9a40407
1 parent 6837b47
commit 9a40407
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 6 deletions.
diff --git a/__tests__/buildFile.test.js b/__tests__/buildFile.test.js
@@ -13,6 +13,8 @@
 
 var buildFile = require('../lib/buildFile');
 var helpers   = require('./__helpers');
+var GroupMessages = require('../lib/utils/groupMessages');
+var chalk = require('chalk');
 
 function format() {
   return "hi";
@@ -52,6 +54,37 @@ describe('buildFile', () => {
     ).toThrow('Please enter a valid destination');
   });
 
+  let dest = 'test.collisions';
+  var PROPERTY_NAME_COLLISION_WARNINGS = GroupMessages.GROUP.PropertyNameCollisionWarnings + ":" + dest;
+  it('should generate warning messages for output name collisions', () => {
+    let name = 'someName';
+    let properties = {
+      allProperties: [{
+        name: name,
+        path: ['some', 'name', 'path1'],
+        value: 'value1'
+      }, {
+        name: name,
+        path: ['some', 'name', 'path2'],
+        value: 'value2'
+      }]
+    };
+
+    GroupMessages.clear(PROPERTY_NAME_COLLISION_WARNINGS);
+    buildFile(dest, format, {}, properties);
+
+    let collisions = properties.allProperties.map((properties) => {
+      let propertyPathText = chalk.keyword('orangered')(properties.path.join('.'));
+      let valueText = chalk.keyword('darkorange')(properties.value);
+      return propertyPathText + '   ' + valueText;
+    }).join('\n        ');
+    let output = `Output name ${chalk.keyword('orangered').bold(name)} was generated by:\n        ${collisions}`;
+    let expectJSON = JSON.stringify([output]);
+
+    expect(GroupMessages.count(PROPERTY_NAME_COLLISION_WARNINGS)).toBe(1);
+    expect(JSON.stringify(GroupMessages.fetchMessages(PROPERTY_NAME_COLLISION_WARNINGS))).toBe(expectJSON);
+  });
+
   it('should write to a file properly', () => {
     buildFile('test.txt', format, {buildPath: '__tests__/__output/'}, {});
     expect(helpers.fileExists('./__tests__/__output/test.txt')).toBeTruthy();

diff --git a/lib/buildFile.js b/lib/buildFile.js
@@ -14,7 +14,8 @@
 var path = require('path'),
     fs   = require('fs-extra'),
     chalk = require('chalk'),
-    filterProperties = require('./filterProperties');
+    filterProperties = require('./filterProperties'),
+    GroupMessages = require('./utils/groupMessages');
 
 /**
  * Takes the style property object and a format and returns a
@@ -33,17 +34,62 @@ function buildFile(destination, format, platform, dictionary, filter) {
   if (!destination || typeof destination !== 'string')
     throw new Error('Please enter a valid destination');
 
-  // if there is a build path, prepend the destination with it
+  var fullDestination = destination;
+
+  // if there is a build path, prepend the full destination with it
   if (platform.buildPath) {
-    destination = platform.buildPath + destination;
+    fullDestination = platform.buildPath + fullDestination;
   }
 
-  var dirname = path.dirname(destination);
+  var dirname = path.dirname(fullDestination);
   if (!fs.existsSync(dirname))
     fs.mkdirsSync(dirname);
 
-  fs.writeFileSync(destination, format(filterProperties(dictionary, filter), platform));
-  console.log(chalk.bold.green('✔︎') + ' ' + destination);
+  var filteredProperties = filterProperties(dictionary, filter);
+
+  // Check for property name Collisions
+  var nameCollisionObj = {};
+  filteredProperties.allProperties && filteredProperties.allProperties.forEach((propertyData) => {
+    let propertyName = propertyData.name;
+    if(!nameCollisionObj[propertyName]) {
+      nameCollisionObj[propertyName] = [];
+    }
+    nameCollisionObj[propertyName].push(propertyData);
+  });
+
+  var PROPERTY_NAME_COLLISION_WARNINGS = GroupMessages.GROUP.PropertyNameCollisionWarnings + ":" + destination;
+  GroupMessages.clear(PROPERTY_NAME_COLLISION_WARNINGS);
+  Object.keys(nameCollisionObj).forEach((propertyName) => {
+    if(nameCollisionObj[propertyName].length > 1) {
+      let collisions = nameCollisionObj[propertyName].map((properties) => {
+        let propertyPathText = chalk.keyword('orangered')(properties.path.join('.'));
+        let valueText = chalk.keyword('darkorange')(properties.value);
+        return propertyPathText + '   ' + valueText;
+      }).join('\n        ');
+      GroupMessages.add(
+        PROPERTY_NAME_COLLISION_WARNINGS,
+        `Output name ${chalk.keyword('orangered').bold(propertyName)} was generated by:\n        ${collisions}`
+      );
+    }
+  });
+
+  let propertyNamesCollisionCount = GroupMessages.count(PROPERTY_NAME_COLLISION_WARNINGS);
+
+  fs.writeFileSync(fullDestination, format(filteredProperties, platform));
+  console.log((propertyNamesCollisionCount>0 ? '⚠️ ' : chalk.bold.green('✔︎ ')) + ' ' + fullDestination);
+
+  if(propertyNamesCollisionCount > 0) {
+    let propertyNamesCollisionWarnings = GroupMessages.fetchMessages(PROPERTY_NAME_COLLISION_WARNINGS).join('\n    ');
+    let title = `While building ${chalk.keyword('orangered').bold(destination)}, token collisions were found; output may be unexpected.`;
+    let help = chalk.keyword('orange')([
+      'This many-to-one issue is usually caused by some combination of:',
+          '* conflicting or similar paths/names in property definitions',
+          '* platform transforms/transformGroups affecting names, especially when removing specificity',
+          '* overly inclusive file filters',
+    ].join('\n    '));
+    let warn = `${title}\n    ${propertyNamesCollisionWarnings}\n${help}`;
+    console.log(chalk.keyword('darkorange').bold(warn));
+  }
 }
 
 

diff --git a/lib/utils/groupMessages.js b/lib/utils/groupMessages.js
@@ -20,6 +20,7 @@ var GroupMessages = {
     RegisterTemplateDeprecationWarnings: 'Register Template Deprecation Warnings',
     SassMapFormatDeprecationWarnings: 'Sass Map Format Deprecation Warnings',
     MissingRegisterTransformErrors: 'Missing Register Transform Errors',
+    PropertyNameCollisionWarnings: 'Property Name Collision Warnings',
   },
 
   flush: function (messageGroup) {