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

Reference is not added to transformed border and shadow token #1415

Open
lukasoppermann opened this issue Dec 17, 2024 · 9 comments
Open

Reference is not added to transformed border and shadow token #1415

lukasoppermann opened this issue Dec 17, 2024 · 9 comments

Comments

@lukasoppermann
Copy link
Contributor

lukasoppermann commented Dec 17, 2024

Hey,

I am not sure if I am missing something.

I use this in my settings for the file:

{
  // ...
 outputReferences: (token, platformOptions) =>
            outputReferencesFilter(token, platformOptions) && outputReferencesTransformed(token, platformOptions),
}

My border token looks like this:

  border: {
    default: {
      $value: {
        color: '{borderColor.default}',
        style: 'solid',
        width: '{borderWidth.default}',
      },
      $type: 'border',
    },
  }

The borderColor token is:

  borderColor: {
    default: {
      $value: '{base.color.neutral.6}',
      $type: 'color',
    },
  }

And the borderWidth token is:

  borderWidth: {
    default: {
      $value: '{borderWidth.thin}',
      $type: 'dimension',
    },
    thin: {
      $value: '1px',
      $type: 'dimension',
    },
  }

My transformer borderToCss looks like this:

export const borderToCss: Transform = {
  name: 'border/css',
  type: 'value',
  transitive: true,
  filter: isBorder,
  transform: (token: TransformedToken) => {
    const value = token.$value ?? token.value
    // if value === string it was probably already transformed
    if (typeof value === 'string') {
      return value
    }
    /* width | style | color */
    return `${value.width} ${value.style} ${value.color}`
  },
}

The expect output is:

--border-default: var(--borderWidth-default) solid var(--borderColor-default);

However, the actual output is:

--border-default: 0.0625rem solid #3d444d;

What am I doing wrong?

// Addition

I just noticed that it is working for shadows without a color and for text shorthands. I am beginning to expect it to be related to the color value.

--boxShadow-thick: inset 0 0 0 var(--borderWidth-thick);
  --text-title-shorthand-small: var(--text-title-weight-small) var(--text-title-size-small) / var(--text-title-lineHeight-small) var(--fontStack-sansSerif); 
@lukasoppermann
Copy link
Contributor Author

If I look at this https://github.com/amzn/style-dictionary/blob/main/lib/utils/references/outputReferencesTransformed.js it seems like objects in tokens are not supported?

Is the token that gets passed to outputReferencesTransformed one of the referenced tokens (e.g. borderColor.default), or is it the entire token, e.g. my border.default token?

@jorenbroekema
Copy link
Collaborator

jorenbroekema commented Dec 17, 2024

Yeah unfortunately this is a limitation of outputReferences.

When you are converting from Object to String, you no longer "know" which property of the object is where exactly in the string, so it's always fundamentally unsafe to then try to put back refs.

Let's take an example e.g. shadow where we can see this problem:

{
  offsetX: '{spacing.1}', // this resolves to 1px
  offsetY: '{spacing.11}', // this resolves to 11px
  blur: '21px',
  color: '#000'
}

This would be transformed to 1px 11px 21px #000, let's imagine we want to put back references now so that we can get from:
1px 11px 21px #000 to
var(--spacing-1) var(--spacing-11) 21px #000

You might see the problem here, we would take the original value (the object one), check for references, we will find two references, the first one is {spacing.1} which resolves to 1px, but this has multiple occurances in the final value... how do we know which one belongs to offsetX and which one belongs to offsetY? We didn't track this information when we stringified the object, Style Dictionary is not that smart so this information gets lost when a complex datatype gets transformed to a simpler/flatter one like Object -> string.

So, if we ignore this problem, we would take the resolved final value and replace occurences of 1px with the reference as a CSS var, which would result in:
var(--spacing-1) 1var(--spacing-1) 2var(--spacing-1) #000

Oops... that's not what we wanted...

So long story short, OutputReferencesTransformed prevents this from happening by not allowing you to output references for tokens that were originally objects but were simplified to strings, because it is unsafe unless you know precisely e.g. through the use of an Abstract Syntax Tree (AST) the exact locations where a object property is sitting inside the string value.
Theoretically we could make this library smart enough to do that but I don't think this is worthwhile. It's a lot of complexity for very little value. Imo it is better to only outputReferences for tokens that are simple / dumb references to other tokens (e.g. a component token that is just a reference to a semantic token), not for tokens that undergo complicated transformations.

@jorenbroekema
Copy link
Collaborator

I should note that we could also think outside the box here and allow:

  • transforms to transform values even if they have references
  • do not resolve references for certain tokens at any given time

which would theoretically let you end up with this value:
{spacing-1} {spacing-11} 21px #000

Which you could then format in the CSS formatter to:
var(--spacing-1) var(--spacing-11) 21px #000

So it would be theoretically possible if you could hook into the resolve references and deny certain tokens from resolving altogether, and relying on the format hook to convert the token refs to CSS vars.

The downside of that approach is that transitive transforms and regular transforms would never work on these tokens, since they normally have to wait for references to be resolved in order to do their work. Two use cases are Studio's transitive color modifers and resolve math transforms, but those can also be handled in CSS itself using color-mix() and calc(), but these have their own limitations as well. So even if this is made possible e.g. in v5 of Style Dictionary, you'll still find yourself struggling.

Hence why my personal recommendation is to not use outputReferences for anything other than tokens that are solely references (I call them dumb tokens). https://github.com/tokens-studio/lion-example?tab=readme-ov-file#tokens-structure documents this a bit for my example repo, where I do use outputReferences, but only for my component token layer which are pure references to the semantic layer (which is theme-dependent/dynamic, hence why CSS vars are useful).

@lukasoppermann
Copy link
Contributor Author

lukasoppermann commented Dec 17, 2024

Ahh, okay that does make sense. Is this why it is working if there is only one reference in the output?

The above issue could be solved by simply searching for 1px (including spaces). Probably more realistically adding a regex to also look for a training comma or semicolon.

Another option would be, that a transformer could provide some kind of map like this: {offsetX} {offsetY} {blur} {color} and than it can be resolved later with references.

I understand that this could be an issue, but is there a way that I can implement it for myself?
I could not just create a custom version of the outputReferencesTransformed because this does not output the token, only true or false, right? So where is the actual exchange happening?

Or would I do what you seem to suggest above, that I just replace the resolved value with the reference in my border transformer?

@jorenbroekema
Copy link
Collaborator

jorenbroekema commented Dec 17, 2024

What about:

{
  offsetX: '{spacing.foo}', // this resolves to 1px
  offsetY: '{spacing.bar}', // this also resolves to 1px
  blur: '1px',
  color: '#000'
}

Even if we had a map {offsetX} {offsetY} {blur} {color}, there are still scenarios that we can theory-craft where this system would fail e.g. when there aren't convenient space separators in the stringified value.

It feels like every time I dive deep into this topic and try to come up with solutions, I find more edge cases that aren't covered and it feels like putting bandages on open wounds.

Whereas if you rethink the way you use references through your token taxonomy/structure and selectively output refs only for dumb tokens, all of these issues can easily be prevented.

That said, outputReferences is very flexible now as you can pass in your own function and selectively enable/disable it, but you cannot dictate unfortunately how the refs are outputted, only whether they are or not for a given token.

For you to completely control also the "how" would mean you'd have to create a custom format and put in place your own outputReferences feature. For inspiration, the createPropertyFormatter is the most sophisticated example of outputReferences that's built-in for Style Dictionary, so for example for the css/variables format:
https://github.com/amzn/style-dictionary/blob/main/lib/common/formats.js#L195 which uses
https://github.com/amzn/style-dictionary/blob/main/lib/common/formatHelpers/formattedVariables.js#L59 which is a small wrapper around
https://github.com/amzn/style-dictionary/blob/main/lib/common/formatHelpers/createPropertyFormatter.js#L176-L247 which is where the magic happens

Edit: no, I think if you replace the value with the references in your border transformer, you will still end up with the same problem because in order to put back refs in the createPropertyFormatter helper, it refers to the token's original value which is still the object. What I meant is that you could have another layer of abstraction in your tokens itself:

  • token "border" refers to "{border2}" -> this will output a CSS var safely
  • border2 uses the object notation and refers to {borderwidth} and border {bordercolor}. this will get stringified, so for this token you will not get a CSS var

@lukasoppermann
Copy link
Contributor Author

That said, outputReferences is very flexible now as you can pass in your own function and selectively enable/disable it, but you cannot dictate unfortunately how the refs are outputted, only whether they are or not for a given token.

Does this mean I could try to enable it and see if the output works? Or will this definitely not give me the desired result?

Whereas if you rethink the way you use references through your token taxonomy/structure and selectively output refs only for dumb tokens, all of these issues can easily be prevented.

Maybe I am not getting what you mean, but this sounds like not using tokens to the max because of a technical issue? I want references so that I can adjust them on the fly (in the browser).

I understand that offering this solution as the tool is problematic, because you have to deal with all the edge cases. But as a consumer, I may be able to avoid all the edge cases, so it would be fine.

@jorenbroekema
Copy link
Collaborator

jorenbroekema commented Dec 17, 2024

Does this mean I could try to enable it and see if the output works? Or will this definitely not give me the desired result?

In the particular use case you mentioned, I think it would work, but the odds that you will run into cases where it creates invalid CSS is pretty high, it depends on a few factors but comes down to how many transitive transforms are applied to your tokens that can cause the exact issue I outlined where the find & replace may make mistakes

not using tokens to the max because of a technical issue

It is a fundamental issue: One entity cannot be a reference to another entity and yet simultaneously be a transformed or otherwise processed or computed variant of itself. Once you do the transformation, the reference is lost because you have just diverged from the referenced value.

We have to think outside the box quite a bit in order to work around that problem. For example, if you would output refs first, and then do the transformation from object to string, then you're fine.

So perhaps the lifecycle should be:

1) pre-transforms (value-changing, can be transitive) / resolve refs
2) output refs (if possible) 
3) post-transforms (syntax/formatting only!)
4) format (handles {ref} -> var(--ref) )

That way, if a pre-transform changes the value, outputting refs will still be disallowed due to the fundamental issue, but your border object to string transformation can just be done at the very end, after all, it's just a syntax-only transformation, it doesn't change the inherent value of the token, it just reorders it or stringifies it in your case. Similar to converting from hex to rgb, it doesn't change the value, just the syntax.

It's something I would be open to playing with, but completely rethinking this part of the lifecycle is very costly even for v5 milestone. I just don't know if I'm convinced we really need to do this when it is my opinion that you should never end up in a spot where you should need this.

I want references so that I can adjust them on the fly (in the browser)

Yeah, my point is that you should adjust only 1 layer of your tokens on the fly (e.g. on theme switch), usually your semantics, and have everything else reference those. "everything else" is what you want to output references for, but the semantic layer of tokens that are "smart" (e.g. can contain math, color modifiers, object tokens with properties that contain/are references) doesn't benefit from outputting as CSS variables unless the thing that it references is also dynamic.

In summary, usually people build their tokens in this way:

  • a layer of primitives, core, global tokens, think of spacer scale, color palettes
  • a layer of semantic tokens that refer to the primitives, this layer is often dynamic through theming, meaning that your background accent color primary may be referring to colors.red.500 in one theme, but colors.blue.500 in another, and there can be multiple sublayers here for example, it may be colors.blue.800 in dark mode but colors.blue.300 in light mode.
  • a layer of component or option/semantic tokens that are mostly just pure references to the semantic dynamic layer, meaning that you benefit from outputting references here, so you can keep the CSS exactly the same, and through CSS variable references the rendered values change when the active theme changes.

There's many variations of this but it always tends to come down to a form static -> dynamic -> static and only the final static layer really benefits from using CSS variables, at least that has been my observation looking at a lot of people's tokens over the past year.

@jorenbroekema
Copy link
Collaborator

Btw I'm open to having my view changed on all this but I'd have to see more clearly why outputting refs is so important for these tokens that imo wouldn't really benefit from it

@lukasoppermann
Copy link
Contributor Author

In summary, usually people build their tokens in this way:

  • a layer of primitives, core, global tokens, think of spacer scale, color palettes
  • a layer of semantic tokens that refer to the primitives, this layer is often dynamic through theming, meaning that your background accent color primary may be referring to colors.red.500 in one theme, but colors.blue.500 in another, and there can be multiple sublayers here for example, it may be colors.blue.800 in dark mode but colors.blue.300 in light mode.
  • a layer of component or option/semantic tokens that are mostly just pure references to the semantic dynamic layer, meaning that you benefit from outputting references here, so you can keep the CSS exactly the same, and through CSS variable references the rendered values change when the active theme changes.

There's many variations of this but it always tends to come down to a form static -> dynamic -> static and only the final static layer really benefits from using CSS variables, at least that has been my observation looking at a lot of people's tokens over the past year.

This is exactly how our tokens are built. And the last layer is where I want the references. But it does not work because border is a component token.

Within this last layer there are references. From my experience expecting to only have one token is not realistic for even semi-complex systems.

Consider the following, you have a color: borderColor.danger that may be used by itself, but you also want to offer a border.danger which would need to use the borderColor.danger token and some size tokens. This already breaks the system.

I don't think this is a pretty complex or "edge-casey" idea.

Let's say borderColor.danger is red, but you want to offer a user with color vision deficiency to change this color, as they can't differentiate red from green.

Now you would either have to build a complex logic that replaces all uses of a specific hex value in your css, OR you just set the borderColor.danger token (css variable) to whatever the user wants, since this is referenced everywhere else.

Other use cases include adjusting contrast, choosing custom colors for components, etc.

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
@lukasoppermann @jorenbroekema and others