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

outputReferencesTransformed to check per ref rather than per token #1203

Closed
jorenbroekema opened this issue May 15, 2024 · 1 comment
Closed

Comments

@jorenbroekema
Copy link
Collaborator

jorenbroekema commented May 15, 2024

Style Dictionary isn't smart enough to know exactly which references within the token value are safe to output as a reference, and which ones aren't

It's pretty late right now and I don't have my early morning brainpower but while writing this it made me go "but can we make it smart enough to determine this per reference in a token, rather than on the token as a whole", and I think that is worth exploring for sure, this may be a non-breaking enhancement to the v4 later down the line.

Originally posted by @jorenbroekema in #1201 (comment)

See original comment for more context on this, but the TL;DR is, imagine:
rgba({gray.color-600}, {opacity.overlay-value})
resolves to:
rgba(#333333, 70%)
which is transitively transformed to
rgba(51, 51, 51, 70%)
which is then outputted as
--overlay: rgba(51, 51, 51, 0.7) instead of --overlay: rgba(51, 51, 51, var(--overlay-alpha))
because the outputReferencesTransformed sees that the token value was transformed and it's unsafe to output the ref.

However, instead of checking whether the token value as a whole has been transformed, maybe it could check whether the parts of the token value that were references were transformed, and determine per reference individually whether it's safe to output a ref or not.

@jorenbroekema
Copy link
Collaborator Author

I am throwing in the towel for this feature enhancement because during implementation I found out this was actually near impossible to get working reliably.

Let me demonstrate why with an example:

{
  base: {
    value: 'rgb(0,0,0)',
    type: 'color',
  },
  alpha: {
    value: 0.12,
    type: 'number',
  },
  referred: {
    value: 'rgba({base},{alpha})',
    type: 'color',
  },
}

rgba({base},{alpha}) resolves to rgba(rgb(0,0,0),0.12) which we will imagine to be transitively transformed to:
rgba(0,0,0,0.12).

We could in theory say that the alpha part of this value is still intact and untransformed, while the base has been transformed to remove the wrapper rgb().

This means that in theory the following output would be desirable: rgba(0,0,0,var(--alpha)).

However, we can quite easily run into issues where when we look at rgba(0,0,0,0.12) and rgba({base},{alpha}), we can't immediately tell which part belongs to base and which part belongs to alpha, after all, what separates these references is a single , character, and in the final resolved value without refs there are 3 commas, so we can't really tell for sure that alpha has been untransformed and the 0,0,0 part belongs to base, because that would require us to know exactly how the transformations work.

We cannot make Style Dictionary smart enough inside formats/output refs logic to know about how transforms have been applied exactly, this kind of context is something that can even trip up a smart human, let alone determining this programmatically.

Therefore, this will not be worked on unless some kind of new insight is found on how we would handle ambiguous cases like this. Otherwise we're opening ourselves up to bugs like these yet again: #974

The safe option is to just not output any refs if we see that the value has been transitively transformed and we cannot reliably put the refs back without causing incorrect output due to undoing the transforms work.

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

No branches or pull requests

1 participant