Fix errors and improve error messaging #158

chazzmoney · 2018-10-05T20:25:09Z

Description of changes:

fixed a bug with deepExtend
fixed circular reference checking and error reporting
fixed the resolveObject bug with multiple references (issue resolveObject - circular definition error if more than 2 references in a value? #118, via @didoo)
added a test for resolveObject - circular definition error if more than 2 references in a value? #118
added some additional circular reference checking tests
tried to create a standard method to deal with error handling, specifically when you need to allow a section to generate multiple errors and then report them all at once. This is challenging with our well defined module system because the errors need to be generated deep and then thrown shallow. I'm using a key based system to capture groups of errors together.

This will be fun to merge with the jest branch... 😬

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…d testing suite to reflect new test.

chazzmoney · 2018-10-09T18:56:56Z

@didoo @davixyz @dbanksdesign Would love feedback on this, especially the mechanism I'm using to try to catch groups of errors at once.

didoo · 2018-10-09T22:39:58Z

lib/extend.js

-        } else if (options.log === 'error') {
-          throw new Error(str);
-        }
+    var ERROR_TYPE = 'Property Value Collisions';


maybe declare the ERROR_TYPE on top as in lib/exportPlatform.js?

Maybe instead of these ERROR_TYPE strings we could actually subclass Error to have actually different Error types.
https://gist.github.com/justmoon/15511f92e5216fa2624b

Doing what Danny suggested for subclassing.

Also, I'm leaving the declaration in mid-code here because it doesn't apply to the whole file - just the calls immediately below it. Instead, I'm moving the declaration of the error group in exportPlatform to be only next to the areas it is used. It is actually the fact that it is defined the same in exportPlatform and in resolveObject that it works as expected. Let me know if you think that I should make this more clear somehow / pass this information in?

didoo · 2018-10-09T22:46:10Z

lib/extend.js

+    });
+
+    if(handleErrors.count(ERROR_TYPE)) {
+      handleErrors.flush(ERROR_TYPE, options.log === 'error' ? chalk.bold.red : chalk.bold.orange);


no strong opinions here, just an idea: do you think it would be more readable as:

if(handleErrors.count(ERROR_TYPE)) { if (options.log === 'error') { handleErrors.flush(ERROR_TYPE, chalk.bold.red); throw new Error('Collisions detected'); } else { handleErrors.flush(ERROR_TYPE, chalk.bold.orange); } }

I like @didoo's comment, and maybe we should put this code in the handleErrors class itself so we don't have to copy and paste that code every time.

Also we could add the count of errors in throw new Error('Collisions detected')

I was trying to decide what to do about this actually. This is the ONLY place we use options.log.

I thought maybe we should delete it. I doubt anyone is using it as I don't think it is documented anywhere. We do have tests for it, which is why I left it in.

I'm also happy to go the route of options.log = error causing this interaction if thats what we deem as what should happen across the board. We might consider renaming the option as it makes me think of "log_level" which doesn't cause the execution to stop but only changes the level of output.

I think this is important to get settled before we call this work complete. Please let me know what you guys think.

lib/utils/deepExtend.js

lib/utils/handleErrors.js

lib/utils/resolveObject.js

didoo · 2018-10-09T23:16:47Z

test/utils/resolveObject.js

-    resolveObject = require('../../lib/utils/resolveObject');
-
+    resolveObject = require('../../lib/utils/resolveObject'),
+    handleErrors  = require('../../lib/utils/handleErrors');


regarding these tests, how do we do to "convert" them in the Jest branch?

I'm not sure... I agree that will definitely be a problem. Let me know if you have suggestions...

Otherwise, during that merge I assume there will be conflicts to resolve. I'll have to add the corresponding jest tests at that time?

bin/style-dictionary

didoo · 2018-10-09T23:27:42Z

lib/utils/handleErrors.js

+    }
+  },
+
+  add: function (arg1, arg2) {


also, I just noticed that it's always called with both the arguments, so this means that you always have a thread, so you don't need to have this double handling of errors and unsavedErrors (but maybe I am missing something)

didoo · 2018-10-09T23:34:39Z

@chazzmoney I've reviewed the PR code, and added some comments. Overall I understand the intent, and I'm ok with it, but at times I was struggling to follow the code of handleErrors.js. It's quite complex; probably I would try to see if there is a way to simplify it even more.

dbanksdesign

I briefly went over the changes and it is a bit hard to understand what is going on. I left some comments though. Have we also thought about adding custom errors by extending the Error class? Some links for that:
https://gist.github.com/justmoon/15511f92e5216fa2624b
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error
https://rclayton.silvrback.com/custom-errors-in-node-js

dbanksdesign · 2018-10-10T21:30:17Z

lib/exportPlatform.js

    transformObject(this.properties, platformConfig)
  );
+
+  if(handleErrors.count(ERROR_TYPE)) {


Do we need to check for the type of error? If there are other errors wouldn't we want to show them anyways?

The intent of this code is to provide a grouping mechanism for errors. If there are other errors, they will be thrown immediately in the code executed much more deeply. If we make it out to this point, it suggests that we didn't have other errors so we should take our collected errors and display them all at once and then throw.

I could see how this looks like how we should handle errors in general. If thats the case I should rework ALL other times we spit out errors. Probably needs to be resolved in relationship to the options.log = error conversation as well.

dbanksdesign · 2018-10-10T21:31:10Z

lib/extend.js

-        } else if (options.log === 'error') {
-          throw new Error(str);
-        }
+    var ERROR_TYPE = 'Property Value Collisions';


Maybe instead of these ERROR_TYPE strings we could actually subclass Error to have actually different Error types.
https://gist.github.com/justmoon/15511f92e5216fa2624b

dbanksdesign · 2018-10-10T21:32:13Z

lib/extend.js

+    });
+
+    if(handleErrors.count(ERROR_TYPE)) {
+      handleErrors.flush(ERROR_TYPE, options.log === 'error' ? chalk.bold.red : chalk.bold.orange);


I like @didoo's comment, and maybe we should put this code in the handleErrors class itself so we don't have to copy and paste that code every time.

dbanksdesign · 2018-10-10T21:33:03Z

lib/extend.js

+    });
+
+    if(handleErrors.count(ERROR_TYPE)) {
+      handleErrors.flush(ERROR_TYPE, options.log === 'error' ? chalk.bold.red : chalk.bold.orange);


Also we could add the count of errors in throw new Error('Collisions detected')

lib/utils/handleErrors.js

lib/utils/resolveObject.js

test/utils/resolveObject.js

chazzmoney · 2018-10-11T21:57:53Z

Thanks @didoo and @dbanksdesign! Will take a look at all your feedback and create another cut at this overall.

chazzmoney · 2018-10-16T00:00:21Z

@didoo @dbanksdesign Let me know what you think of this revision. There are some notes I put in to conversations that are now marked as outdated, but would love feedback on.

dbanksdesign

Some comments/questions

dbanksdesign · 2018-10-18T06:21:51Z

bin/style-dictionary

+    ErrorHandler.flush();
+  }
+  catch (e) {
+    ErrorHandler.flush();


Can all this stuff and the below in this file be removed? Should the style dictionary node module already flush out all the errors? If not it probably should so using the node module and the CLI should behave the same.

lib/exportPlatform.js

lib/extend.js

dbanksdesign · 2018-10-18T06:26:29Z

lib/utils/errorHandler.js

+var chalk = require('chalk');
+
+var errors = [];
+var unsavedErrors = {};


I still don't quite get the difference between errors and unsavedErrors

From the grouping POV, unsavedErrors are still being grouped and errors have already been grouped.

There is a bit more to it than that, e.g. handling errors that don't need grouping but you want to wait to output. But its just a generalization of the above case where group size is 1.

lib/utils/errorHandler.js

lib/utils/resolveObject.js

dbanksdesign · 2018-10-18T06:31:19Z

lib/utils/resolveObject.js

+          if (foundCircArr.indexOf(reference)!==-1) {
+            // If the current reference is a member of a circular reference, do nothing
+          }
+          else if(stack.indexOf(reference)!==-1) {


It's weird to have an empty body to part of a conditional, this could be rewritten as

if (stack.indexOf(reference) !== -1) { } else if (foundCircArr.indexOf(reference) === -1) { }

Yeah. We may want to do something in the future. I think we had something there before but I removed it. I like having the comment in there to be clear that we are intentionally not doing something there.

Alternatively, I could refactor the logic. Let me know if you think that would be better.

lib/utils/resolveObject.js

chazzmoney · 2018-10-20T00:12:28Z

@didoo Let me know if this is better at all.

…e flush on uncaught exceptions to prevent confusion

chazzmoney · 2018-10-21T06:11:31Z

Ok @didoo and @dbanksdesign I think we are almost there. I spent some time thinking and decided that this isn't an error handler, it is an error grouper.

Based on the idea that grouping is useful only for specific situations, I reworked the naming schemes and usage. It should make more sense now, especially in terms of specificity of use.

Let me know your thoughts; would like to get this merged prior to #152

Thanks!

didoo

@chazzmoney I have added a few comments, and a couple of things that I would change/simplify. let me know if you agree (I know this ticket has gone a long way now, I'm sorry for that)

didoo · 2018-10-21T09:13:38Z

__tests__/__json_files/not_circular.json

+  "prop3" : { "value": "{prop1.value}" },
+  "prop4" : { "value": "{prop3.value}" },
+  "prop12" : { "value": "{prop1.value}, {prop2.value} and some extra stuff" },
+  "prop124" : { "value": "{prop1.value}, {prop2.value} and {prop4.value}" }


Thanks for the examples!

didoo · 2018-10-21T09:20:36Z

__tests__/utils/resolveObject.test.js

+
+      var obj = resolveObject(helpers.fileToJSON(__dirname + '/../__json_files/not_circular.json'));
+      expect(GroupErrors.count(PROPERTY_REFERENCE_ERRORS)).toBe(0);
+      expect(JSON.stringify(obj)).toBe(JSON.stringify({


not about this ticket, but just an occasion to show you one of my perplexities, which is clearly visible here: I am still unsure why the "input" of the tests is stored in a separate file (__json_files/..) while the "output" is in the test itself. I think there must be a better way :) (but let's discuss in a separate future ticket)

didoo · 2018-10-21T09:26:33Z

__tests__/utils/resolveObject.test.js

+
+      resolveObject(helpers.fileToJSON(__dirname + '/../__json_files/multiple_reference_errors.json'));
+      expect(GroupErrors.count(PROPERTY_REFERENCE_ERRORS)).toBe(3);
+      expect(JSON.stringify(GroupErrors.fetchMessages(PROPERTY_REFERENCE_ERRORS))).toBe(JSON.stringify([


do you think there should be a similar test for "multiple circular references" errors too? no idea, just asking.

didoo · 2018-10-21T09:50:53Z

lib/utils/resolveObject.js

+            // Add our found circular reference to the end of the cycle and output to error
+            circStack.push(reference);
+            var err = new CustomError(
+              'Circular definition cycle:  ' + circStack.join(', '),


maybe use a template literal syntax here, as you did above?

didoo · 2018-10-21T09:51:04Z

lib/utils/resolveObject.js

@@ -142,16 +161,14 @@ function selfRef(string, obj) {
  // If the reference doesn't change then it means
  // we didn't find it in the object
  if (ref === obj) {
-    throw new Error('Reference doesn\'t exist: ' + context + ' tries to reference ' + string + ', which is not defined');
+    var err = new CustomError(
+      'Reference doesn\'t exist: ' + context + ' tries to reference ' + string + ', which is not defined',


maybe use a template literal syntax here, as you did above?

didoo · 2018-10-21T10:03:29Z

lib/utils/groupErrors.js

+  },
+
+  flush: function (errorGroup, chalkType) {
+    if(errorGroup) {


I don't understand why we need this test. In the code, I see no case in which it is called without the errorGroup argument. Also, it would also be strange to see a declaration like this:

GroupErrors.flush(,chalk.bold.red);

(or am I missing something?)

also, see the comment above on how to possibly refactor this function

didoo · 2018-10-21T10:10:30Z

lib/utils/groupErrors.js

+  },
+
+  clear: function (errorGroup) {
+    if (errorGroup) {


maybe here is better:

if (errorGroup && groupedErrors[errorGroup]) { delete groupedErrors[errorGroup]; }

(you used the proposed syntax above, so it's also more consistent)

didoo · 2018-10-21T10:13:15Z

lib/utils/groupErrors.js

+    return groupedErrors[errorGroup] ? groupedErrors[errorGroup].length : 0;
+  },
+
+  fetchMessages: function (errorGroup) {


this function is used only in the tests. is there a way to declare it only in the context of the tests, and not here?

didoo · 2018-10-21T10:40:31Z

lib/utils/groupErrors.js

+
+var groupedErrors = {};
+
+var getFinalGroupErrors = function(errorGroup) {


this function is used only once. and is doing two things in one: is "getting the list of all the errors as a 'kind of' array" and "preparing the formatting of the output messages" (see when you add a double newline at the end). in the same way, consoleOutputErrArr is similar. I think it would be better to move these two pieces of code where they are actually used. I understand it may increase the complexity there, but is not that moving the code in another place under a function solves the problem :)
I have tried to refactor the code, removing the extra functions and simplifying the logic. see how it looks in this way:
https://gist.github.com/didoo/2b681bee9e5f1a6a4327d6278e35fa09
(if you want to use it, test it anyway, I've just refactored inside a text editor, not actually tested, ok?)

Yeah, before the refactor they were used in multiple places. I thought about redoing as a single func but thought that there were two reasons to keep it. Clarity of use and that if people wanted the other functionality back it would be nice. It seems like you and Danny both are telling me to stop engineering ahead and just focus on immediate task. I'll remove the functions.

Yeah, before the refactor they were used in multiple places. I thought about redoing as a single function but thought that there were two reasons to keep it. Clarity of use and that if people wanted the other functionality back it would be nice. It seems like you and Danny both are telling me to stop engineering ahead and just focus on immediate task. I'll remove the functions.

chazzmoney · 2018-10-22T06:52:46Z

@didoo Let me know what you think of latest changes. I need to make a couple more minor updates, but I think you'll like it better.

chazzmoney

herk, why does github think I'm reviewing myself?

chazzmoney · 2018-10-22T06:02:16Z

lib/utils/groupErrors.js

+
+var groupedErrors = {};
+
+var getFinalGroupErrors = function(errorGroup) {


Yeah, before the refactor they were used in multiple places. I thought about redoing as a single function but thought that there were two reasons to keep it. Clarity of use and that if people wanted the other functionality back it would be nice. It seems like you and Danny both are telling me to stop engineering ahead and just focus on immediate task. I'll remove the functions.

chazzmoney · 2018-10-22T07:24:20Z

My other laptop is having major problems with github. Will have an updated version for you in ~11 hours @didoo

chazzmoney · 2018-10-22T07:24:35Z

My other laptop is having major problems with github. Will have an updated version for you in ~11 hours @didoo

… fixed issues with simplified GroupMessages

chazzmoney · 2018-10-22T21:39:01Z

@didoo Sorry for the delay, lots of isssues with github. (looks like travis-ci isn't running yet either).

This is the final version of the updates that I had mentioned.

didoo · 2018-10-22T22:01:39Z

@chazzmoney I think is much better and clearer now, no? to me looks good :)

dbanksdesign

Just one more comment

dbanksdesign · 2018-10-23T15:42:48Z

lib/utils/resolveObject.js

-          catch (e) {
-            errors.push(e);
-          }
+          obj[key] = compile_value( obj[key], [current_context.join('.')] );


Either we are not testing something properly or the code isn't working as expected. If you change this line back and run the tests, only one test fails and it only because the message string is different:

utils › resolveObject › should gracefully handle circular references expect(received).toBe(expected) // Object.is equality Expected: "[\"Circular definition cycle: a.b.c, j, a.b.c\"]" Received: "[\"Circular definition cycle: j, a.b.c, j\"]"

* improved error messages for registerTemplate * updated error message * Introduce option to control the generation of the "Do not edit" header (#132) * stage #1 - formats.js * stage #2 - templates * reset changes to template + simplified changes to formats (now the “options” object is assigned to the “file” element) * fixed wrong parameter passed to fileHeader function * updated documentation * updates after PR comments * removing the confusing static-style-guide stuff (#157) * Fixes #72 * handle no command and invalid commands with friendly console output (#156) * Add json5 support (#165) * Removing unnecessary backticks (#172) * Merge Jest Branch (#169) * Jest testing (#133) * moved all the existing tests to Jest * finalised Jest tests for “utils” removing assert dependency * finalised Jest tests for “register” removing assert dependency + moved tests under correct folder * finalised Jest tests for “transform” removing assert dependency + moved tests under correct folder + removed extra file * updated path for “service” files/folders * removed output folder * updated the paths to ignore in the Jest config in package.json * finalised Jest tests for “clean” removing assert dependency + other small changes * added “__output” to the list of folders ignored by Jest * some tunings + more tests * more tests cleanup * fixed test for exportPlatform * fixed last tests, and now all tests are green! * Added first snapshot tests! Yay! * added mock for dates to avoid failing snapshots tests * updated tests * first attempt to fix the UTC date problem on CI (reference: boblauer/MockDate#9) * second attempt to fix the UTC date problem on CI * removed the TZ=UTC env environment to test if is really needed * updated all the occurrences of new Date in the templates * restored linting before running the tests suite * code style fix * fixed wrong porting of the test for buildAllPlatforms * test(all): Fix for all tests to match the date and remove of mockdate (#148) inspiration jestjs/jest#2234 * test(javascript/es6): Add test for es6 (#149) * test: add registerTemplate (#147) * add tests for transform object (#151) * add tests for transform object * split up complex test in multiple smaller tests * Jest flatten props (#163) * Adding tests for lib/utils/flattenProperties.js (#146) * Adding tests for lib/utils/flattenProperties.js * update to use lodash sortby function * update to use lodash sortby function * Add babel-jest (#173) * feat(json-nested): Add JSON nested transform (#167) Added JSON nested transform, Added test for it and Documentation update re #139 * Fix errors and improve error messaging (#158) * updated error messaging. Fixes for issues with references. * adding in didoo's test from #118 * cleanup of terminology * fixed resolveObject to correctly replace multiple references. modified testing suite to reflect new test. * updates per comments by didoo and dbanksdesign * case sensitive, oops. * case sensitive, oops. * minor updates based on PR feedback * merging with develop to ensure we stay synched * removing cli error handling and moving to module * removing per dannys comments * making constants for group errors per Dannys comments * switch to error grouping mindset and naming * switch to error grouping mindset and naming * per danny's comment * fix flush to execute across all groups if called with no group; remove flush on uncaught exceptions to prevent confusion * simplify, simplify, simplify * changed out error naming to message mindset, cleaned out console.log, fixed issues with simplified GroupMessages * sepearate circular reference tests into separate expects * avoid using string so we dont get it confused with String * Deprecating templates (#152) * Displaying a warning when using templates in the config or registerTemplate * Moving built-in templates to formats * Porting over a stragler test (#190) * 2.5.0

@didoo

* improved error messages for registerTemplate * updated error message * Introduce option to control the generation of the "Do not edit" header (#132) * stage #1 - formats.js * stage #2 - templates * reset changes to template + simplified changes to formats (now the “options” object is assigned to the “file” element) * fixed wrong parameter passed to fileHeader function * updated documentation * updates after PR comments * removing the confusing static-style-guide stuff (#157) * Fixes #72 * handle no command and invalid commands with friendly console output (#156) * Add json5 support (#165) * Removing unnecessary backticks (#172) * Merge Jest Branch (#169) * Jest testing (#133) * moved all the existing tests to Jest * finalised Jest tests for “utils” removing assert dependency * finalised Jest tests for “register” removing assert dependency + moved tests under correct folder * finalised Jest tests for “transform” removing assert dependency + moved tests under correct folder + removed extra file * updated path for “service” files/folders * removed output folder * updated the paths to ignore in the Jest config in package.json * finalised Jest tests for “clean” removing assert dependency + other small changes * added “__output” to the list of folders ignored by Jest * some tunings + more tests * more tests cleanup * fixed test for exportPlatform * fixed last tests, and now all tests are green! * Added first snapshot tests! Yay! * added mock for dates to avoid failing snapshots tests * updated tests * first attempt to fix the UTC date problem on CI (reference: boblauer/MockDate#9) * second attempt to fix the UTC date problem on CI * removed the TZ=UTC env environment to test if is really needed * updated all the occurrences of new Date in the templates * restored linting before running the tests suite * code style fix * fixed wrong porting of the test for buildAllPlatforms * test(all): Fix for all tests to match the date and remove of mockdate (#148) inspiration jestjs/jest#2234 * test(javascript/es6): Add test for es6 (#149) * test: add registerTemplate (#147) * add tests for transform object (#151) * add tests for transform object * split up complex test in multiple smaller tests * Jest flatten props (#163) * Adding tests for lib/utils/flattenProperties.js (#146) * Adding tests for lib/utils/flattenProperties.js * update to use lodash sortby function * update to use lodash sortby function * Add babel-jest (#173) * feat(json-nested): Add JSON nested transform (#167) Added JSON nested transform, Added test for it and Documentation update re #139 * Fix errors and improve error messaging (#158) * updated error messaging. Fixes for issues with references. * adding in didoo's test from #118 * cleanup of terminology * fixed resolveObject to correctly replace multiple references. modified testing suite to reflect new test. * updates per comments by didoo and dbanksdesign * case sensitive, oops. * case sensitive, oops. * minor updates based on PR feedback * merging with develop to ensure we stay synched * removing cli error handling and moving to module * removing per dannys comments * making constants for group errors per Dannys comments * switch to error grouping mindset and naming * switch to error grouping mindset and naming * per danny's comment * fix flush to execute across all groups if called with no group; remove flush on uncaught exceptions to prevent confusion * simplify, simplify, simplify * changed out error naming to message mindset, cleaned out console.log, fixed issues with simplified GroupMessages * sepearate circular reference tests into separate expects * avoid using string so we dont get it confused with String * Deprecating templates (#152) * Displaying a warning when using templates in the config or registerTemplate * Moving built-in templates to formats * Porting over a stragler test (#190) * 2.5.0 * Added 'json/flat' format (#192) * Fix: #195 (#196) * updating contributing to reflect the package manager and testing suite correctly (#197) * Add Sass maps formats (#193) * added ‘sass/map-flat’ and ‘sass/map-deep’ formats + updated tests * fixed inconsistend newlines in templates for sass maps * improved recursive processJsonNode function * updated snapshots tests * removed unused function * Better examples (#164) * changed folder structure * removed table in Readme of Basic example (not clear and probably also some cells were wrong) * small update for the Basic example to make it more clear how aliases are referenced * renamed the “npm” example to “npm module” * updated “npm” example to use the same config and properties as the “basic” example * removed license (no sense here) and updated package.json * updated the s3 example making it more similar to other examples and adding some more assets to be uploaded and linked/embedded in tokens * updated logo in main Readme in example folder * updated the Readme for the S3 example * tried to re-organise the “react” folder in two separate folders the web app doesn’t compile * removed spaces from “example” sub-folder * renamed “example” folder to “examples” * removed numbers from “examples” sub-folder names * removed space in sub-folder names * added advanced example on how to use a watcher to auto-rebuild see: #171 * small update to Readme for “auto-rebuild-watcher” * added advanced example on how to have a multi-platform multi-brand suite * added advanced example on how to use custom templates * fixed “watch” npm script declaration * moved packages under “devDependencies” for “custom templates” package * added a comment in an example of the lodash templating syntax * remove invisible characters from Readme * added “clean” npm script call where missing in examples package.json * added .gitignore file where was missing in examples folder * updated the config file for the “npm module” example * added a comment to explain better how the “formatter” function works * updated the “init” command to expose only the possible/meaningful options + updated documentation for the “examples” page * added comment about collecting more examples * updated the Readme for the “examples” folder * updated “version.js” script as per Danny suggestion * added advanced example on how to use custom transforms * updated basic example to use “format” instead of “template” to avoid the alert in console * added advanced example about referencing/aliasing * updated example to show reference to an “object-like” value * removed the advanced examples for react and react native * added a “create react app” example (with Sass) * better config for S3 example * simplified the example for “S3” * re-introduced android + ios in S3 example * added a “assets-base64-embed” example * finalised the “assets-base64-embed” example * updated Readme for “npm” example + fixed the “prepublishOnly” script option (previous one was deprecated) * removed the “create-react-app-sass” example (I’ll add it later in a separate ticket) * updated the documentation * New cut at documentation PR using current develop branch (#198) * New cut at documentation PR using current develop branch * Apply @didoo's suggestions from code review Co-Authored-By: chazzmoney <charles@pgina.org> * updates based on didoo's thoughts * Updating the architecture documentation page (#200) * updates per didoo and dbanks * typo * generation differences * minor fixes and updates * making sure sd init command documentation is correct, for now * updates for clarity around properties and references * fixing up another alias piece * Addressing some comments in architecture diagram (#204) * Final touches on build diagram and architecture (#206) * Final touches on build diagram and architecture * Updating build diagram * Updating build diagram * Configuration doc update * fixing snapshot whitespace issues, discovered actual failing test on merge... * Fixing merge conflict issues * v2.6.0 release (#210)

Charles Dorner added 5 commits October 5, 2018 02:02

updated error messaging. Fixes for issues with references.

9ab877d

adding in didoo's test from #118

3aada21

cleanup of terminology

313a23c

merge with most recent changes in develop

a11ea07

fixed resolveObject to correctly replace multiple references. modifie…

a3d17ec

…d testing suite to reflect new test.

didoo reviewed Oct 9, 2018

View reviewed changes

dbanksdesign reviewed Oct 10, 2018

View reviewed changes

chazzmoney mentioned this pull request Oct 12, 2018

Decide whether combineJSON should report the actual file locations of property collisions. #166

Closed

Charles Dorner added 3 commits October 12, 2018 10:25

updates per comments by didoo and dbanksdesign

7f32410

case sensitive, oops.

90345f5

case sensitive, oops.

55308c7

dbanksdesign reviewed Oct 18, 2018

View reviewed changes

Charles Dorner added 6 commits October 19, 2018 15:03

minor updates based on PR feedback

3834001

merging with develop to ensure we stay synched

fa4a221

merging with develop to ensure we stay synched

f0d54c4

removing cli error handling and moving to module

6ae3445

removing per dannys comments

5daa56a

making constants for group errors per Dannys comments

455b32d

Charles Dorner added 4 commits October 20, 2018 22:37

switch to error grouping mindset and naming

dbf042b

switch to error grouping mindset and naming

baf7134

per danny's comment

7eb7383

fix flush to execute across all groups if called with no group; remov…

2147cc6

…e flush on uncaught exceptions to prevent confusion

didoo suggested changes Oct 21, 2018

View reviewed changes

simplify, simplify, simplify

23a63e3

chazzmoney commented Oct 22, 2018

View reviewed changes

changed out error naming to message mindset, cleaned out console.log,…

aac23f5

… fixed issues with simplified GroupMessages

chazzmoney mentioned this pull request Oct 22, 2018

Rewrite to ES6 #153

Closed

chazzmoney mentioned this pull request Oct 23, 2018

Create an example showing how to use a watcher to auto-rebuild #171

Closed

dbanksdesign reviewed Oct 23, 2018

View reviewed changes

Charles Dorner added 2 commits October 23, 2018 16:35

sepearate circular reference tests into separate expects

9cd8a70

avoid using string so we dont get it confused with String

c7f99ca

chazzmoney merged commit f5c0486 into develop Oct 24, 2018

dbanksdesign deleted the error-messaging branch November 13, 2018 22:38

wv91364210zhuozh mentioned this pull request Feb 2, 2021

[Snyk] Upgrade tinycolor2 from 1.4.1 to 1.4.2 wv91364210zhuozh/style-dictionary#1

Open


		var groupedErrors = {};

		var getFinalGroupErrors = function(errorGroup) {

Fix errors and improve error messaging #158

Fix errors and improve error messaging #158

Conversation

chazzmoney commented Oct 5, 2018 • edited Loading

chazzmoney commented Oct 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

didoo commented Oct 9, 2018

dbanksdesign left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chazzmoney commented Oct 11, 2018

chazzmoney commented Oct 16, 2018

dbanksdesign left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chazzmoney commented Oct 20, 2018

chazzmoney commented Oct 21, 2018

didoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chazzmoney commented Oct 22, 2018

chazzmoney left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chazzmoney commented Oct 22, 2018

chazzmoney commented Oct 22, 2018

chazzmoney commented Oct 22, 2018

didoo commented Oct 22, 2018

dbanksdesign left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chazzmoney commented Oct 5, 2018 •

edited

Loading