From 61fb0b430b3634246caaaac04a6164b968e698aa Mon Sep 17 00:00:00 2001 From: Charles Dorner Date: Sat, 20 Apr 2019 17:23:10 -0700 Subject: [PATCH 1/2] better error messaging when the configuration includes an unknown transform or transformGroup --- __tests__/__output/json-nested.json | 12 ++++++ __tests__/buildPlatform.test.js | 10 ++++- __tests__/transform/config.test.js | 62 +++++++++++++++++++++++++++++ lib/buildPlatform.js | 4 +- lib/cleanPlatform.js | 2 +- lib/exportPlatform.js | 2 +- lib/transform/config.js | 37 ++++++++++++++++- lib/utils/groupMessages.js | 1 + 8 files changed, 122 insertions(+), 8 deletions(-) create mode 100644 __tests__/__output/json-nested.json create mode 100644 __tests__/transform/config.test.js diff --git a/__tests__/__output/json-nested.json b/__tests__/__output/json-nested.json new file mode 100644 index 000000000..943677682 --- /dev/null +++ b/__tests__/__output/json-nested.json @@ -0,0 +1,12 @@ +{ + "color": { + "base": { + "red": { + "primary": "#611D1C", + "secondary": { + "inverse": "#000000" + } + } + } + } +} \ No newline at end of file diff --git a/__tests__/buildPlatform.test.js b/__tests__/buildPlatform.test.js index 1f9d2a472..fc90b70c0 100644 --- a/__tests__/buildPlatform.test.js +++ b/__tests__/buildPlatform.test.js @@ -25,7 +25,7 @@ describe('buildPlatform', () => { it('should throw if passed a platform that doesn\'t exist', () => { expect( StyleDictionaryExtended.buildPlatform.bind(test, 'foobar'), - ).toThrow('Platform foobar doesn\'t exist'); + ).toThrow('Platform "foobar" does not exist'); expect( function() { @@ -164,9 +164,15 @@ describe('buildPlatform', () => { } } }); + + let err = ` +Unknown transformGroup "bar" found in platform "foo": +"bar" does not match the name of a registered transformGroup. +`; + expect( StyleDictionaryExtended.buildPlatform.bind(StyleDictionaryExtended, 'foo'), - ).toThrow('transformGroup bar doesn\'t exist'); + ).toThrow(err); }); }); diff --git a/__tests__/transform/config.test.js b/__tests__/transform/config.test.js new file mode 100644 index 000000000..b3845d56d --- /dev/null +++ b/__tests__/transform/config.test.js @@ -0,0 +1,62 @@ +/* + * Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with + * the License. A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR + * CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions + * and limitations under the License. + */ + +var transformConfig = require('../../lib/transform/config'); + +const dictionary = { + transformGroup: { + fooTransformGroup: ['barTransform'] + }, + transform: { + fooTransform: { + type: 'attribute', + transformer: function() { + return {bar: 'foo'} + } + } + } +}; + +describe('transform', () => { + describe('config', () => { + it('Emits error when called with a transformGroup that does not exist in the dictionary', () => { + const noTransformGroupCfg = { + transformGroup: 'barTransformGroup' + }; + + let err = ` +Unknown transformGroup "barTransformGroup" found in platform "test": +"barTransformGroup" does not match the name of a registered transformGroup. +`; + + expect( + transformConfig.bind(null, noTransformGroupCfg, dictionary, 'test') + ).toThrow(err); + }); + + it('Emits errors when called with a transform that does not exist', () => { + const noTransformCfg = { + transforms: ['fooTransform', 'barTransform', 'bazTransform'] + }; + + let err = ` +Unknown transforms "barTransform", "bazTransform" found in platform "test": +None of "barTransform", "bazTransform" match the name of a registered transform. +`; + + expect( + transformConfig.bind(null, noTransformCfg, dictionary, 'test') + ).toThrow(err); + }); + }); +}); diff --git a/lib/buildPlatform.js b/lib/buildPlatform.js index 739afbfde..375ad4f12 100644 --- a/lib/buildPlatform.js +++ b/lib/buildPlatform.js @@ -43,12 +43,12 @@ function buildPlatform(platform) { console.log('\n' + platform); if (!this.options || !_.has(this.options.platforms, platform)) { - throw new Error('Platform ' + platform + ' doesn\'t exist'); + throw new Error(`Platform "${platform}" does not exist`); } var properties; // We don't want to mutate the original object - var platformConfig = transformConfig(this.options.platforms[platform], this); + var platformConfig = transformConfig(this.options.platforms[platform], this, platform); // We need to transform the object before we resolve the // variable names because if a value contains concatenated diff --git a/lib/cleanPlatform.js b/lib/cleanPlatform.js index fbaddae9b..87f506dd6 100644 --- a/lib/cleanPlatform.js +++ b/lib/cleanPlatform.js @@ -38,7 +38,7 @@ function cleanPlatform(platform) { var properties; // We don't want to mutate the original object - var platformConfig = transformConfig(this.options.platforms[platform], this); + var platformConfig = transformConfig(this.options.platforms[platform], this, platform); // We need to transform the object before we resolve the // variable names because if a value contains concatenated diff --git a/lib/exportPlatform.js b/lib/exportPlatform.js index f2d347de7..61f79522b 100644 --- a/lib/exportPlatform.js +++ b/lib/exportPlatform.js @@ -37,7 +37,7 @@ function exportPlatform(platform) { } // We don't want to mutate the original object - var platformConfig = transformConfig(this.options.platforms[platform], this); + var platformConfig = transformConfig(this.options.platforms[platform], this, platform); // We need to transform the object before we resolve the // variable names because if a value contains concatenated diff --git a/lib/transform/config.js b/lib/transform/config.js index e55e589cd..9cd89d252 100644 --- a/lib/transform/config.js +++ b/lib/transform/config.js @@ -15,6 +15,7 @@ var _ = require('lodash'), GroupMessages = require('../utils/groupMessages'); var TEMPLATE_DEPRECATION_WARNINGS = GroupMessages.GROUP.TemplateDeprecationWarnings; +var MISSING_TRANSFORM_ERRORS = GroupMessages.GROUP.MissingRegisterTransformErrors; /** * Takes a platform config object and returns a new one @@ -23,9 +24,10 @@ var TEMPLATE_DEPRECATION_WARNINGS = GroupMessages.GROUP.TemplateDeprecationWarni * @private * @param {Object} config * @param {Object} dictionary + * @param {Object} platformName (only used for error messaging) * @returns {Object} */ -function transformConfig(config, dictionary) { +function transformConfig(config, dictionary, platformName) { var to_ret = _.clone(config); // The platform can define either a transformGroup or an array @@ -40,7 +42,11 @@ function transformConfig(config, dictionary) { if (dictionary.transformGroup[to_ret.transformGroup]) { transforms = dictionary.transformGroup[to_ret.transformGroup]; } else { - throw new Error('transformGroup ' + to_ret.transformGroup + ' doesn\'t exist'); + let err = ` +Unknown transformGroup "${to_ret.transformGroup}" found in platform "${platformName}": +"${to_ret.transformGroup}" does not match the name of a registered transformGroup. +`; + throw new Error(err); } } @@ -48,9 +54,36 @@ function transformConfig(config, dictionary) { // the StyleDictionary module. We need to map the strings to // the actual functions. to_ret.transforms = _.map(transforms, function(name) { + if(!dictionary.transform[name]) { + GroupMessages.add( + MISSING_TRANSFORM_ERRORS, + `"${name}"` + ); + } return dictionary.transform[name]; }); + let missingTransformCount = GroupMessages.count(MISSING_TRANSFORM_ERRORS); + if(missingTransformCount > 0) { + var transform_warnings = GroupMessages.flush(MISSING_TRANSFORM_ERRORS).join(', '); + let err; + + if(missingTransformCount==1) { + err = ` +Unknown transform ${transform_warnings} found in platform "${platformName}": +${transform_warnings} does not match the name of a registered transform. +`; + } + else { + err = ` +Unknown transforms ${transform_warnings} found in platform "${platformName}": +None of ${transform_warnings} match the name of a registered transform. +`; + } + + throw new Error(err); + } + to_ret.files = _.map(config.files, function(file) { const ext = {}; if (file.filter) { diff --git a/lib/utils/groupMessages.js b/lib/utils/groupMessages.js index 221a89df0..4c51ebb44 100644 --- a/lib/utils/groupMessages.js +++ b/lib/utils/groupMessages.js @@ -19,6 +19,7 @@ var GroupMessages = { TemplateDeprecationWarnings: 'Template Deprecation Warnings', RegisterTemplateDeprecationWarnings: 'Register Template Deprecation Warnings', SassMapFormatDeprecationWarnings: 'Sass Map Format Deprecation Warnings', + MissingRegisterTransformErrors: 'Missing Register Transform Errors', }, flush: function (messageGroup) { From 8c535f88754955eafa94628f6714753bdc3fdafb Mon Sep 17 00:00:00 2001 From: Charles Dorner Date: Tue, 23 Apr 2019 12:39:22 -0700 Subject: [PATCH 2/2] removing test output file --- __tests__/__output/json-nested.json | 12 ------------ 1 file changed, 12 deletions(-) delete mode 100644 __tests__/__output/json-nested.json diff --git a/__tests__/__output/json-nested.json b/__tests__/__output/json-nested.json deleted file mode 100644 index 943677682..000000000 --- a/__tests__/__output/json-nested.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "color": { - "base": { - "red": { - "primary": "#611D1C", - "secondary": { - "inverse": "#000000" - } - } - } - } -} \ No newline at end of file