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 https://github.com/amzn/style-dictionary/blob/737a02b638b9450eb4f989fe9718357f5cba4239/integration/css.test.js#L26

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.

[aharvard]

Just wanted to express my interest in using this when ready. Thanks for driving this work to keep us in sync with the evolving spec!

[AlexisCharp]

Hey @brian-smith-tcril, Are you still working on this? Otherwise, I can try to take a look into it :)

[brian-smith-tcril]

@AlexisCharp I haven't had a chance to dig back into this, so if you'd like to take a look that would be wonderful! Thank you!

[scottfoxus]

I wanted to check in to see if there is active effort on this issue, because I need to figure out a fix rather quickly. This could be me putting effort into a fix here or writing a shim on our side if a patch for 4.x.x is imminent.

I also addressed this issue with DTCG because the spec changed rather quietly from the time we prototyped switching to DTCG format to integration testing. We mistakenly developed on the latest version even though we prototyped on the older revision., and we're now realizing many token types won't build.

This is our miss, however, the documentation for Amazon Style Dictionary still states "first class support" for DTCG and links to the schema documentation that no longer works. Unfortunately, there's no good way for the doc to describe a DTCG support level, because DTCG doesn't specifically version the schema. This is why I created an issue with DTCG to start versioning their schema and provide a way to link to documentation at that revision level.

If this is going to be a long-running issue, the documentation should at least be modified to call out the breaking change.

[jorenbroekema]

Yeah fully agree with everything you've said @scottfoxus , I've also communicated with the other DTCG members that they should specifically involve me when the spec changes so I'm at least aware that I have to update Style Dictionary or that I can block something if it's not feasible from a token parsing perspective. We've also internally at Tokens Studio discussed publishing a TS interface & JSON schema for DTCG format module, and semver versioning that, so that we can be transparent about which versions of SD are compatible with which version ranges of the DTCG format.

I believe @AlexisCharp wanted to work on this, perhaps they have an update? I'll probably pick some of these up in the next weeks if not, because I agree this is kinda urgent given that we promote to have first class DTCG support. Not saying it's negligent or misleading that we're not fully compatible atm, I think that's definitely due to the somewhat "silent", unversioned DTCG updates, but I do still want to be true to that claim ;)

[AlexisCharp]

Hey @scottfoxus @jorenbroekema, I wanted to help and had even started some small things but I lack time atm and can't continue. Feel free to proceed ;)

[scottfoxus]

We've also internally at Tokens Studio discussed publishing a TS interface & JSON schema for DTCG format module, and semver versioning that, so that we can be transparent about which versions of SD are compatible with which version ranges of the DTCG format.

I worked on one internally at my company and I see that someone else has also done some TypeScript work around this. We're all redoing the same work, so it would definitely be nice to see types that are formally published with DTCG changes and potentially available in npm. This has been suggested in one of the issues on the DTCG side, but I do not see any changes getting adopted as a result.

If I get time to work on this bug myself, I will update here.

[artursopelnik]

Here you could find a workaround for dimension:

https://github.com/amzn/style-dictionary/issues/848#issuecomment-2855874849