amzn / style-dictionary

A build system for creating cross-platform styles.
https://styledictionary.com
Apache License 2.0
3.87k stars 543 forks source link

Redesign logging #1039

Closed jorenbroekema closed 6 months ago

jorenbroekema commented 11 months ago

@tahul

Needs a bit of discussion, I aim to redesign this a bit and ensure that we have better comprehensive logging, ensure we don't bloat the console too much (e.g. collision and reference errors can get quite huge, see if we can be more concise).

I think the logging can also be improved to show better where the reference errors are coming from, e.g. which file.

Logs

We should determine which ones should be logged based on --verbose, below is a suggestion:

--silent just means no logs at all?

Reference errors

I'd also like to improve the verbose side of reference errors so it's a bit easier to trace reference chains and where exactly in which file the ref error is occurring, so make it easier to debug these and fix them for users with many token files.

lukasoppermann commented 6 months ago

This is very important.

I'd also love if there would be an option like --verbose or something.

Ideally there would be options like:

jorenbroekema commented 6 months ago

I've updated the original comment to reflect a bit better my plans, let me know what you think!

lukasoppermann commented 6 months ago

Hey, I have one thing I'd like to differ:

Token Collision warnings -> only a concise message by default, --verbose shows every collision, allow throwing error instead of console.warn in error-mode

We have a ton of token collisions, but they don't matter. This happens because of overrides or other things. I'd like to be able to mute them but still get reference errors.

So, I think either they should be mutable OR when using --silent Token Reference errors should still be raised. I don't know of any scenario were a ref error would not be an error.

jorenbroekema commented 6 months ago

Hm true, silencing reference errors sounds like a bad idea, as they're fatal.

Token collisions are warnings by default, rather than errors, unless using verbose I'd say the warning should only be quite small:

While building /css/vars.css, token collisions were found; output may be unexpected.

And when using --silent it shouldn't be logged at all?

What do you think about the success logs, should they not happen when --silent?

lukasoppermann commented 6 months ago

I think --silent should only show ref errors (are there other actual errors)?

Then without --verbose you would have a very concise log with success and short collsions were found messages.

And only with --verbose would you get a long log.

I am using logging a lot in dev and I know some others do to. This is very hard with the current amount of noise from SD. So for me the goal should be to make it very quiet, so that my own logs mostly what I see.

jorenbroekema commented 6 months ago

Yea there's quite a few other errors like unparseable JSON files, transformGroups with unrecognized transforms, etc. etc., those should be logged always I guess, they're basically always (fatal) user errors.

Totally agree about the noise, in the configurator I have a snackbar component that forwards any errors logs to the UI (designers dont tend to check console) and this just doesn't look nice when there's a lot of them 😆 image

jorenbroekema commented 6 months ago

Released in prerelease.19, see changelog https://github.com/amzn/style-dictionary/blob/v4/CHANGELOG.md