Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable alias resolving #1021

Open
markusnissl opened this issue Oct 6, 2023 · 4 comments
Open

Disable alias resolving #1021

markusnissl opened this issue Oct 6, 2023 · 4 comments

Comments

@markusnissl
Copy link

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

@markusnissl markusnissl changed the title Disable value resolving Disable alias resolving Oct 6, 2023
@jorenbroekema
Copy link
Collaborator

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:

  • resolveReferences (true by default). When set to false, leave reference values as is in tokens. Put the burden of replacing them with the proper values (e.g. CSS variable) on the formats, but supply and document helpers to make it easy to identify reference values and resolving them.
  • warnOnBrokenReferences (false by default). When set to true, instead of throwing an error when not being able to resolve a reference, it will only output a warning instead. The value will be kept as is (with the reference in it), even if resolveReferences is true.

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
Copy link
Collaborator

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 #882 to ensure that it takes into account filters.

@lukasoppermann
Copy link
Contributor

@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 #882 to ensure that it takes into account filters.

We have a related issue: #882 (comment)

Is this change still planned or already implemented?

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

@jorenbroekema
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants