Regression in transitive transform from version 3.9.0

[pascalduez]

Greetings,

prior to version 3.9.0 applying a transformation on transitive tokens used to work fine, but from version 3.9.0 the transformation is not applied anymore and only the reference output. Maybe introduced in https://github.com/amzn/style-dictionary/pull/1002.

Please find a reproduction here https://github.com/pascalduez/style-dictionary-transform-repro Stripped down obviously.

{
  "length": {
    "base": {
      "type": "dimension",
      "value": "4"
    },
    "1x": {
      "type": "dimension",
      "value": "{length.base}",
      "$extensions": {
        "scale": 1
      }
    },
    "2x": {
      "type": "dimension",
      "value": "{length.base}",
      "$extensions": {
        "scale": 2
      }
    },
    "3x": {
      "type": "dimension",
      "value": "{length.base}",
      "$extensions": {
        "scale": 3
      }
    }
  }
}

 'length': {
    type: 'value',
    transitive: true,
    matcher: token =>
      token.type === 'dimension' && Object.hasOwn(token, '$extensions'),
    transformer: token => {
      return token.value * token.$extensions.scale;
  },
},

style-dictionary < 3.9.0 (expected)

:root {
  --length-base: 4;
  --length-3x: 12;
  --length-2x: 8;
  --length-1x: 4;
}

style-dictionary >= 3.9.0

:root {
  --length-base: 4;
  --length-3x: var(--length-base);
  --length-2x: var(--length-base);
  --length-1x: var(--length-base);
}

[jorenbroekema]

This is actually a bugfix rather than a regression.

Firstly, I believe your expected outcome should be:

:root {
  --length-base: 4;
  --length-3x: 12;
  --length-2x: 8;
  --length-1x: var(--length-base);
}

Let me know if that's correct, I'm assuming by your issue that you're using outputReferences: true

I'll try to explain now why this is not a regression but rather a bugfix, it's a bit complex though...

A design token, fundamentally speaking, cannot be a reference to another token while also being a transitively transformed version of itself in the output, this is a contradiction. For example, your length-3x is a reference to the base length token, but it's a different version of that, so it doesn't actually refer to the base length token anymore when we get to the final output, that reference was only functional during the transformation. The reason < 3.8 works in your scenario is because it cannot find the reference anymore because the value was altered, so it ditches the reference and just puts the resolved value there, and it appears as if it's "working", but in fact, the token in the source is a reference, so when you say "output references", it's contradicting that a bit by not spitting out a reference, while in the token source it was a reference.

Summary: a token reference in the source does not guarantee that this reference remains intact in the output. Sometimes that's ok, but often it resulted in very buggy behavior.

What this means is in 3.8 you could get some very strange outcomes, which I encourage you to test for yourself:

{
  "length": {
    "base": {
      "type": "dimension",
      "value": "4"
    },
    "11x": {
      "type": "dimension",
      "value": "{length.base}",
      "$extensions": {
        "scale": 11
      }
    }
  }
}

Output 3.8:

:root {
  --length-base: 4;
  --length-11x: var(--length-base)4;
}

So why does it do the above?? Because in 3.8, it would do a regex replace on the resolved value, trying to see if in the resolved value it can find the reference token's value, and if so, replace it with a reference. In this case, we have 44 because 4*11 = 44, and then it tries to find 4 (ref token value) in the 44 (resolved token value) and do this regex replace. Knowing this, you may actually think that the expected outcome would be: var(--length-base)var(--length-base) I don't recall whether the regex replace was using 'g' flag. Maybe it did, maybe it didn't, either way, the output is wrong ;)

https://github.com/tokens-studio/sd-transforms/issues/13 here's an issue that encountered this specific bug, and there's a few more but that one is the main place we discussed about how to fix it. Here's another example https://github.com/tokens-studio/sd-transforms/issues/121

Eventually me and Germain opened an issue on this library repo https://github.com/amzn/style-dictionary/issues/974 and I created a Pull request which was released in 3.9, this fixed the bug but at the tradeoff that transitive transforms "work" is being undone when putting back the reference in the output. So while that can be seen as a regression, it's a tradeoff we felt was worth it to fix the bugs.

The conclusion was that transitive transforms that change the final value of a token compared to how it is in the source, is just not compatible with outputReferences feature.

However, to address this regression, we could add a relatively small fix where instead of undoing the work of transitive transforms, outputReferences will detect the diff between design token value in the source vs after transformations, and just not output a ref for such tokens in the output, while outputting refs for tokens that have not changed. In this way, I believe we still fix the bugs in 3.8 without adding this regression for users like yourself. I'll try to experiment with that a bit this week!

[jorenbroekema]

However, to address this regression, we could add a relatively small fix where instead of undoing the work of transitive transforms, outputReferences will detect the diff between design token value in the source vs after transformations, and just not output a ref for such tokens in the output, while outputting refs for tokens that have not changed.

Okay so I tried doing this and it became apparent that we cannot distinguish whether a value transform was an actual change to the value and thus a reference should not be outputted, or whether a value transform was merely a minor formatting change where you do still want to output the ref, so I don't think my idea for the fix works.

I did write up some ideas at one point that I still want to explore to improve this https://github.com/amzn/style-dictionary/commit/97f26a8f61f10531f6e75f8a063b409482e3bb00 , so I'm not giving up just yet, just not entirely sure if this will make it to v4

[pascalduez]

Hi @jorenbroekema,

thanks a lot for your thorough reply and analysis. This make total sense, and somehow I always knew that this particular transformer and tokens declarations were a bit tinkered and subject to changes. I remember to have tried many ways but couldn't make them work. I guess mostly because in a transformer you don't have access to the whole dictionary, so can't retrieve the base value.

Let me know if that's correct, I'm assuming by your issue that you're using outputReferences: true

That's correct. Although I had to tweak a bit the output (converting px to rem) to circumvent an issue #873. Which really looks like the one you described.

As a result, I would say probably don't spend too much time on this given all the work you already have on the v4. I'll try to scaffold this transformation in another way.

Without even trying maybe:

{
  "length": {
    "base": {
      "type": "dimension",
      "value": "4"
    },
    "1x": {
      "type": "dimension",
      "value": 1,
      "$extensions": {
        "scale": true
      }
    },
    "2x": {
      "type": "dimension",
      "value": 2,
      "$extensions": {
        "scale": true
      }
    },
    "3x": {
      "type": "dimension",
      "value": 3,
      "$extensions": {
        "scale": true
      }
    }
  }
}

'length': {
    type: 'value',
    transitive: true,
    matcher: token =>
      token.type === 'dimension' && Object.hasOwn(token, '$extensions'),
    transformer: token => {
     // Find a way to grab the `scale` value, maybe a platform option after all. Like `basePxFontSize`.  
      return token.value * scale;
  },
},

[pascalduez]

Keep it simple as they say: https://github.com/pascalduez/style-dictionary-transform-repro/pull/1

[jorenbroekema]

It appears I managed to found a pretty neat trick to introduce a fix without the caveat that formatting-only transforms are not considered. https://github.com/amzn/style-dictionary/pull/1154 is the PR that fixes this and the trick I do is diffing between the token value and the resolved refs version of the original value, but since I run resolveReferences on the transformed dictionary, a diff should only occur if there was a transitive transform that made a change to the token value.

Bit complicated but all you need to do to fix your scenario is apply the new outputReferencesTransformed utility function, see docs, but in summary:

import { outputReferencesTransformed } from 'style-dictionary/utils';

export default {
  platforms: {
    css: {
      transformGroup: 'css',
      transforms: ['ts/color/css/hexrgba'],
      files: [
        {
          destination: 'vars.css',
          format: 'css/variables',
          options: {
            outputReferences: outputReferencesTransformed,
          },
        },
      ],
    },
  },
};

where that transform 'ts/color/css/hexrgba' is just an example transitive transform I picked to demo this