amzn / style-dictionary

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

outputReference does not handle multiple references properly #886

Open Javernus opened 2 years ago

Javernus commented 2 years ago

I came across a weird behaviour when working with outputReference. The output seems to be bugged. Here is a config to recreate the issue:

{
    "colour": {
      "10": {
        "value": "#000000"
      }
    },
    "opacity": {
      "0": {
        "value": "0"
      }
    },
    "test": {
      "value": "{colour.10.value}, {opacity.0.value}"
    }
}

Here is the output token test for this code for both css and scss exports:

$test: $colour-1$opacity-0, 0;
--test: var(--colour-1var(--opacity-0)), 0;

As is clear, this is incorrect. I would expect this:

$test: $colour-10, $opacity-0;
--test: var(--colour-10), var(--opacity-0);

I tested this on version 3.7.1.

Javernus commented 1 year ago

Has there been any focus on this issue?

ohearonkg commented 1 year ago

I recently encountered a similar issue. As mentioned in the other referenced issue, I believe the problem area is related to https://github.com/amzn/style-dictionary/blob/main/lib/common/formatHelpers/createPropertyFormatter.js#L115

Specifically, when resolving references and updating the content of the value property.

In your example the string would start out as:

#000000 0

My understanding is that Style Dictionary works backwards and resolved each value to it's top level reference. So on the first pass the string would be updated to:

var(--color-10), 0

Then it attempts to call JavaScript's string.replace function on this string, attempting to replace the value 0 with the referenced token opacity-0. This results in the unexpected out:

var(--color-1var(--opacity-0), 0

Where the first instance of the string 0 has been replaced with what Style Dictionary believes is its referenced token name.

ohearonkg commented 1 year ago

@Javernus - For what it's worth, if you're willing to (hopefully temporarily patch the package) it seems that updating the code within the createPropertyFormatter.js file as done within this PR fixed the issue for me :)