Incorrect/unexpected output for $dimension token types when using DTCG format spec

[adamstankiewicz]

According to the DTCG format spec, the $dimension token type (source) should have a $value that:

MUST be an object containing a numeric value (integer or floating-point) and unit of measurement ("px" or "rem").

From the DTCG spec example:

{
  "spacing-stack-0": {
    "$value": {
      "value": 0,
      "unit": "px"
    },
    "$type": "dimension"
  },
  "spacing-stack-1": {
    "$value": {
      "value": 0.5,
      "unit": "rem"
    },
    "$type": "dimension"
  }
}

When trying this syntax for $dimension tokens in style-dictionary v4, we're seeing the following output:

Token definition:

{
  "size": {
    "$type": "dimension",
    "border": {
      "width": {
        "$value": {
          "value": 1,
          "unit": "px"
        },
        "$description": "Default border width."
      }
    }
  }
}

Token output in CSS variables

(Includes a pgn prefix).

--pgn-size-border-width-value: 1; /* Default border width. */
--pgn-size-border-width-unit: 0px; /* Default border width. */

I believe the expected CSS variables output should be:

--pgn-size-border-width: 1px; /* Default border width. */

Our current workaround is to define the token with a $value that combines the value/unit together, e.g.:

{
  "size": {
    "$type": "dimension",
    "border": {
      "width": {
        "$value": "1px"
        "$description": "Default border width."
      }
    }
  }
}

It seems like style-dictionary's support for composite tokens (per the DTCG format spec) is causing $dimension tokens to output multiple variables (i.e., splitting the value from the unit) vs. outputting a single CSS variable combining the configured value/unit.

Is this by design or is there a potential bug when trying to adopt the DTCG spec for $dimension tokens? Thanks!

[jorenbroekema]

I think the dimension type token was updated quite recently in the DTCG spec, we haven't updated Style Dictionary accordingly yet. We need to find a way to do this in a non-breaking manner so that the old method is also still valid for some time at least until the next major version of SD.

• Support new syntax
• Support old syntax
• Update the convertToDTCG utility to correctly convert a set with the old syntax for dimension, to the new, so that they are forward compatible with Style-Dictionary v5

In v5, we will likely stop supporting outdated versions of the DTCG spec (color type will get an update in the spec soon as well).
Additionally in v5, potentially stop supporting the old SD JSON format altogether, but imo that also depends a bit on how stable and complete the DTCG spec is at that point..

[jorenbroekema]

This probably could be picked up by outside contributors as well just FYI, it shouldn't be too difficult from my estimation, so I added good first issue label

[brian-smith-tcril]

@jorenbroekema for clarification, when you say "find a way to do this in a non-breaking manner" do you mean both

{
  "size": {
    "$type": "dimension",
    "border": {
      "width": {
        "$value": "1px"
        "$description": "Default border width."
      }
    }
  }
}

and

{
  "size": {
    "$type": "dimension",
    "border": {
      "width": {
        "$value": {
          "value": 1,
          "unit": "px"
        },
        "$description": "Default border width."
      }
    }
  }
}

from the examples should produce

--pgn-size-border-width: 1px; /* Default border width. */

or do you mean that there should also be a way to produce

--pgn-size-border-width-value: 1; /* Default border width. */
--pgn-size-border-width-unit: 0px; /* Default border width. */

from

{
  "size": {
    "$type": "dimension",
    "border": {
      "width": {
        "$value": {
          "value": 1,
          "unit": "px"
        },
        "$description": "Default border width."
      }
    }
  }
}

[jorenbroekema]

The former:

--pgn-size-border-width: 1px; /* Default border width. */

The latter can be produced as well already, if you use expand and expand the dimension token with the appropriate typesMap: https://styledictionary.com/reference/config/#expand but this is optional of course

[brian-smith-tcril]

I started looking into this a bit today and hit a bit of a snag. My plan was to:

• Add tests for both the value as string and value as object use cases
• Update the code to support both use cases and make the tests pass

but it wasn't clear to me where to add a test that verifies generated css. I found https://github.com/amzn/style-dictionary/blob/main/__integration__/css.test.js and saw

style-dictionary/__integration__/css.test.js

Line 26 in 737a02b

source: [`__integration__/tokens/**/[!_]*.json?(c)`],

but adding a new json file to https://github.com/amzn/style-dictionary/tree/main/__integration__/tokens/size seems like it touches more tests than I was hoping to.

Any advice that could point me in the right direction (docs I'm missing/PRs doing adding similar tests to reference) would be greatly appreciated!

[jorenbroekema]

I imagine we would handle this in a built-in transform hook, that transforms the object type to the string type, and apply that to all of the transformGroups as a way of ensuring automatic compatibility for all platforms. In my opinion, it should be added at the end of the array, because the object-format is easier to work with for other dimension transforms. We may need to update some of the dimension transforms accordingly, to also be able to handle object-type dimension tokens, e.g. the transforms that add units to unitless tokens, or those that do conversions such as px to rem or vice versa.

We'll probably need to touch quite a few test files:

• the specific ones for the built-in transforms (the ones that get changed) found here: https://github.com/amzn/style-dictionary/blob/main/__tests__/common/transforms.test.js should be able to handle object dimension types
• integration tests https://github.com/amzn/style-dictionary/tree/main/__integration__/tokens/size adding some dimension tokens here with object values makes sense. A lot of the integration tests will fail subsequently, but if you've established that the snapshot mismatches are "expected" (coz you added tokens to it), you can hit npm run test:update-snapshots locally which will update the snapshots to the new state, and the tests will then pass from then on.