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

Suppress filtered out reference warnings at the `getReferences()` level? #1287

Open chris-dura opened 2 months ago

chris-dura commented 2 months ago

Relates to https://github.com/amzn/style-dictionary/issues/687

The logging configuration and mechanisms are much more robust in v4, but, I think I've encountered a non-trivial situation where it would be nice to be able to configure the logging at a more granular level, perhaps in a File config, or even at the getReferences() utility.

From what I can tell, the warnings are issued when a call to getReferences() is made, not necessarily when outputReferences: true.

I have a few custom formats I've created where I'm not actually trying to "output references", but I do want to use getReferences() utility so I can actually get more token data and operate on it.

For example, I have a trivial markdown formatter that outputs a token reference table as documentation. I use the getReferences() utility to help in generating a trivial "reference list" for each component. (This is probably the first step towards maybe generating a Graph at some point...)

Anyway, Markdown is a static format, and there's really no such thing as a "broken reference", so in general, I don't really care if the references are filtered out, because I'm just trying to display them in an informational table, and I'm not actually "outputting references", because I'm just using the resolved value in the Value column. But, my console log gets "polluted" with a bunch of warnings because the my-size, and my-spacing tokens have been filtered out (because I don't care to display them in the table, I just want the tokens at the "end of the chain" to be output).

| Name                | Value              | Reference Tree        |
|---------------------|--------------------|-----------------------|
| inline-spacing-md   | 4px                | ↓ 4px                 |
|                     |                    | ↓ my-size             |
|                     |                    | ↓ my-spacing          |
|                     |                    | • inline-spacing-md   |

I could create an entirely new StyleDictionary() object, with logging turned off, just for using this custom markdown format, but that seems heavy-handed? Being able to configure the logging on a per-File output would be better...

However, that being said, if I did want to change to outputReferences: true, so that the reference was in the Value column, I think it would in fact want to get those "filtered out" reference warnings... but only for the getReferences() call I used to generate the value, NOT the getReferences() call I use to generate the tree in the same file.

| Name                | Value              | Reference Tree        |
| ------------------- | ------------------ | --------------------- |
| inline-spacing-md   | {my-spacing}       | ↓ 4px                 |
|                     |                    | ↓ my-size             |
|                     |                    | ↓ my-spacing          |
|                     |                    | • inline-spacing-md   |
// my-md-format.js

allTokens.map((token) => {
  // Get the value
  let refs = getReferences(token.original.value, tokens, { unfilteredTokens }); // <-- Throw warnings for debugging!
  ...
  let colValue = ref;

  // Generate reference tree
  let refs = getReferences(token.original.value, tokens, { unfilteredTokens }); // <-- DO NOT throw warnings, we just need the reference data!
  ...
  let referenceTree = refs.map((ref) => true);

});
jorenbroekema commented 2 months ago
let refs = getReferences(token.original.value, tokens, { unfilteredTokens, warnImmediately: false })

This option is mostly meant for when getReferences is used within the format lifecycle and you want the warnings to be collected and thrown later on once we've checked all tokens, but I guess in your case it might be useful to dodge the warnings as well.

Otherwise you can always refer to this: https://styledictionary.com/reference/logging/ and set log.warnings to 'disabled' to silence all console warnings.

chris-dura commented 2 months ago

but I guess in your case it might be useful to dodge the warnings as well.

Yeah, but it doesn't really dodge them... it just puts the unwanted warnings at the end of the console, instead of the middle.

set log.warnings to 'disabled' to silence all console warnings.

Yeah, but I only want to silence a particular usage... I do want the warnings for other usages (in the same output target).

... that being said, I suppose it is a very-specific, uncommon use case 🤷🏻 and sounds like I'm stuck with creating a specific "platform" for this output target and disable the logging. I'll miss out on potentially useful warnings for that one target, though.


This might be pedantic... but, I do think it's a little odd that calling getReferences() utility causes a warning... because I'm just getting the references, not outputting them. I'd think that the warning would instead be associated with outputReferences: true setting, instead of assuming that every time a user calls getReferences(), they intend to "output" them.

jorenbroekema commented 2 months ago

I think the reason of the oddness is that usually you are either calling getReferences:

Concluding, I agree that there should be a way to use getReferences, even within the format lifecycle, and not warn about filtered out refs when the intention isn't to output refs.

I think in general, the logging capabilities of SD could be further improved by allowing you to specify the warn / verbosity options per log type rather than only for all logs.

chris-dura commented 2 months ago

standalone, not within the format lifecycle, in which case the warnImmediately set to false will mean you won't see the warnings at all

Hmmm... I didn't think about this... I could generate an inheritance tree object outside the format lifecycle, like as a pre-build step, I suppose, and just pull it in within the format lifecycle. A little smelly, but not horrible. 👍🏻

allowing you to specify the warn / verbosity options per log type rather than only for all logs.

Yeah, that's where I was headed too... somewhat like I configure my eslint... "error" for this rule, "warn" for this rule, "ignore" this other one.