amzn / style-dictionary

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

outputReferencesTransformed resolves opacity variable in RGBA #1201

Open okadurin opened 3 months ago

okadurin commented 3 months ago

In v4 the value as follows

--overlay: rgba(var(--color-600), var(--overlay-value))

is transformed to something like

--overlay: rgba(51, 51, 51, 0.7)

But on v3 it was transformed to

--overlay: rgba(51, 51, 51, var(--overlay-value));

Note the opacity variable is resolved on v4 and it is preserved on v3

Here is the code of my setup: token:

{
  "bg": {
    "overlay": {
      "value": "rgba({gray.color-600}, {opacity.overlay-value})",
      "type": "color"
    }
  },
  "gray": {
    "color-600": {
      "value": "#333333",
      "type": "color"
    }
  },  
  "opacity": {
    "overlay-value": {
      "value": "70%",
      "type": "opacity"
    }  
  }
}

And the config:

import {
registerTransforms as registerTokenStudioTransformsAndParsers,
transforms as tokenStudioTransforms,
} from '@tokens-studio/sd-transforms';
import StyleDictionary from 'style-dictionary';
import { outputReferencesTransformed } from 'style-dictionary/utils';

registerTokenStudioTransformsAndParsers(StyleDictionary);

const config = {
  include: [`tokens/alias.json`],
  platforms: {
    css: {
      transforms: [...tokenStudioTransforms, 'ts/color/css/hexrgba'],
      buildPath: `build/`,
      files: [{
        destination: `variables.css`,
        format: `css/variables`,
        options: {
          outputReferences: outputReferencesTransformed
        }
      }]
    }
  }
};

Could you suggest how to make configure v4 so it stops resolving opacity variables in an rgba function? That is instead of --overlay: rgba(51, 51, 51, 0.7) I would like v4 to generate --overlay: rgba(51, 51, 51, var(--overlay-value)); as it was on v3.

jorenbroekema commented 3 months ago

Yes I can explain why it happens and unfortunately no, this cannot be fixed/worked around.

Let's take a look at the token value: rgba({gray.color-600}, {opacity.overlay-value}) When you resolve this, it becomes: rgba(#333333, 70%) which is invalid CSS

It is then transformed by 'ts/color/css/hexrgba' transform, which is a transitive transform otherwise it wouldn't be able to transform values that contain references, to become: rgba(51, 51, 51, 70%) which is now valid CSS

The token value has been transitively transformed, and fundamentally speaking a token cannot be a reference to another token in the output, while also being a transformed version of itself simultaneously, it's "either, or".

So what the outputReferencesTransformed utility does, is determine if a transformation has happened to the value. If this is the case, it is unsafe to output references, because you won't be able to tell if the work of a transform will be undone by doing so. In your example is may be the case that the alpha (overlay-value) wasn't transformed, only the rgb was, but 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, if a transformation to the value happened we have to assume it's unsafe to do it for all references inside the token.

Style Dictionary v3 was eagerly outputting references, and while it didn't cause a problem in your specific scenario, it was very prone to nasty and hard to explain bugs.

Related issues: https://github.com/amzn/style-dictionary/issues/1124 https://github.com/amzn/style-dictionary/issues/974 https://github.com/tokens-studio/sd-transforms/issues/13 https://github.com/tokens-studio/sd-transforms/issues/121

I would have liked to work on this topic more for v4 but I was running out of the time and the release is already huge, I'm hoping to revisit this topic in more detail for a future v5 but there is no timeline for that yet, definitely not for another 6-12 months I'd say.

jorenbroekema commented 3 months ago

I think it makes sense to create an issue:

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.