amzn / style-dictionary

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

Disable alias resolving #1021

Open markusnissl opened 1 year ago

markusnissl commented 1 year ago

Hey,

Context: we are following a custom approach to generate SCSS and CSS tokens taking multiple brands and themes into account and also look towards optimising bundle size and performance for our component library. In order to achieve this, we generate per component an own css file that references a semantic variable which name is shared between different themes, yet each theme on the semantic level has its own theme file.

Problem: On the component level, we do not need any alias resolving for this use case and we do not want to load any theme data. In case a variable cannot be resolved, the program terminates with an error.

Workaround: We load the tokens of one of the themes so that the alias tokens can be resolved and then filter out all tokens that do not belong to the component layer. However, this triggers following warning:

While building ***.css, filtered out token references were found; output may be unexpected. Here are the references that are used but not defined in the file

which makes the workaround inconvenient.

Anticipated solution: Similar to outputReferences: true in the formatting part, we would like to turn off the alias resolving for the complete configuration in case we do not required the value at any point of the pipeline.

I have searched for this feature and for any open issue that covers this topic, but have not found any solution regarding this. Any fix or input to resolve this warning or the workaround would be appreciated.

Thanks

jorenbroekema commented 1 year ago

I am planning on rethinking how outputReferences option works in general in style-dictionary, and I think making it a global option in the SD config makes sense, where it will just leave references alone. Then, in formats, they can deal with values that contain references accordingly, by searching the referenced token and for example putting a CSS variable in the dictionary reference's place.

Correct me if I misunderstand, but I think for your case it's more about configuring style-dictionary in such a way that it does not resolve references and also does not complain if a reference cannot be resolved.

So perhaps there should be two options:

Some examples to clarify

Assuming the following tokenset:

{
  "foo": {
    "value": "12px"
  },
  "ref": {
    "value": "{foo}"
  },
  "brokenRef": {
    "value": "{bar}"
  }
}

1. Normal situation (defaults)

{
  "resolveReferences": true,
  "warnOnBrokenReferences": false
}

Will result in an error, due to the broken reference to "bar" which is a token that does not exist in the dictionary. Basically, just because we don't resolve the reference and replace the reference with the referenced value, does not mean we don't check if the reference is broken or not. We still check if the reference is valid, even though we leave it in place. I think this is valuable as a default behavior so that broken references that are not intended by the user, are easily spotted.

2. Do not resolve references

{
  "resolveReferences": false,
  "warnOnBrokenReferences": false
}

Will result in an error, due to the broken reference to "bar" which is a token that does not exist in the dictionary.

3. Resolve references, but only warn on broken references

{
  "resolveReferences": true,
  "warnOnBrokenReferences": true
}

Will result in:

:root {
  --foo: 12px;
  --ref: 12px;
  --broken-ref: var(--bar);
}

Assuming that the CSS format is reference-aware and resolves it to CSS variables.

4. Do not resolve references, and only warn on broken references

{
  "resolveReferences": false,
  "warnOnBrokenReferences": true
}

Will result in:

:root {
  --foo: 12px;
  --ref: var(--foo);
  --broken-ref: var(--bar);
}

Assuming that the CSS format is reference-aware and resolves it to CSS variables.

I'm just thinking out loud here, I saw this issue and I was planning on brainstorming a bit about this topic anyways for v4, so here it is :)

jorenbroekema commented 1 year ago

Another idea I had is that resolveReferences (and maybe even warnOnBrokenReferences) should accept a Function which is ran for each token and returns a boolean, to selectively resolve or not resolve references depending on certain token properties, e.g. which file they originate from.

We should also keep in mind this issue https://github.com/amzn/style-dictionary/issues/882 to ensure that it takes into account filters.

lukasoppermann commented 7 months ago

@jorenbroekema

Another idea I had is that resolveReferences (and maybe even warnOnBrokenReferences) should accept a Function which is ran for each token and returns a boolean, to selectively resolve or not resolve references depending on certain token properties, e.g. which file they originate from.

We should also keep in mind this issue https://github.com/amzn/style-dictionary/issues/882 to ensure that it takes into account filters.

We have a related issue: https://github.com/amzn/style-dictionary/issues/882#issuecomment-2031767030

Is this change still planned or already implemented?

is there a way of hotfixing it in V3 using a custom format or something?

jorenbroekema commented 7 months ago

Is this change still planned or already implemented?

Not implemented, unlikely to make v4 since it's gonna be quite a refactor of reference resolution in SD. https://github.com/amzn/style-dictionary/blob/97f26a8f61f10531f6e75f8a063b409482e3bb00/references-transforms.md this document contains some of my thoughts on the matter, but I doubt I will have the time to really dive into this for v4 release.

is there a way of hotfixing it in V3 using a custom format or something?

Probably, you'd need to implement your own format with a custom outputReferences implementation.

I would be open however to consider if we can add a small patch that allows outputReferences to be either a boolean or a function, if it's a function, the token is passed and based on its attributes you could decide whether or not to output refs. That should be quite an easy change I think. Should be easy enough to add to v3 as well. Maybe we should create a different issue for that though to track it, since that's selectively disabling the outputting of refs, and is a bit off-topic considering this issue is more about not resolving references whatsoever to begin with and keeping all refs in there until the format stage.