tokens-studio / sd-transforms

Custom transforms for Style-Dictionary, to work with Design Tokens that are exported from Tokens Studio
MIT License
194 stars 28 forks source link

128: Save expanded tokens to separate property #178

Closed yringler closed 8 months ago

yringler commented 1 year ago

Will resolve #128

What do you think of this solution? We move over the expanded tokens to a separate property, keeping the original composite token, which is output as before. If you agree, a simple name transformer can be added to remove the added text from the name.

changeset-bot[bot] commented 1 year ago

⚠️ No Changeset found

Latest commit: 37774ecd1c5979a3b0191300dc64d8bd4c5d6099

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

jorenbroekema commented 1 year ago

Hm I think I would prefer to make keeping the original object that's expanded as an opt-in, so:

{
  "foo": {
    "value": {
      "fontSize": "16px",
      "fontFamily": "Arial",
      "fontWeight": "Bold",
      "lineHeight": "1"
    },
    "type": "typography"
  }
}

becomes:

{
  "foo": {
    "fontSize": {
      "value": "16px",
      "type": "fontSizes"
    },
    "fontFamily": {
      "value": "Arial",
      "type": "fontFamilies"
    },
    "fontWeight": {
      "value": "Bold",
      "type": "fontWeights"
    },
    "lineHeight": {
      "value": "1",
      "type": "lineHeights"
    }
    "__sd_transforms_placeholder__": {
      "value": {
        "fontSize": "16px",
        "fontFamily": "Arial",
        "fontWeight": "Bold",
        "lineHeight": "1"
      },
      "type": "typography"
    }
  }
}

I'm thinking you might need that __sd_transforms_placeholder__ to wrap the "value" inside of, otherwise I fear the individual expanded props may be ignored by style-dictionary, because there's a "value" sibling prop. Not 100% about this, or what's the best way to work with it... but it should then result in:

:root {
  --foo-font-size: 16px;
  --foo-font-family: Arial;
  --foo-font-weight: 700;
  --foo-line-height: 1;
  --foo: 700 16px/1 Arial;
}
yringler commented 1 year ago

Hm I think I would prefer to make keeping the original object that's expanded as an opt-in, so:

I hear - that does end up with a cleaner object model, everything wrapped together nicely. My concern with that is - what if another token references the original composite token? I think it's simplest to keep existing stuff where they are, and add new stuff separately.

I'm thinking you might need that sd_transforms_placeholder to wrap the "value" inside of, otherwise I fear the individual expanded props may be ignored by style-dictionary, because there's a "value" sibling prop. Not 100% about this, or what's the best way to work with it...

I'm getting dizzy 😵 😆 Right, if we could just keep the value as is, that would be great, but I also don't see how that could work.

jorenbroekema commented 1 year ago

My concern with that is - what if another token references the original composite token?

That's actually a really good question and I'm not sure at the moment about whether I added an integration test for that use case. If it turns out there's no clear path for this, then perhaps it makes sense that when expanding composite tokens, the original should always be kept intact for references. Then, it will not be an option, it'll be the default, after all, people could then filter those tokens out with StyleDictionary formats -> filters.

Either way, since you've spent some effort into this, I will make it a priority to dive into this topic as well and come up with a proper solution. The solution isn't that obvious so I want to start with some test cases and outline what the preferred behavior should be (including your case with referencing a composite token), and see if I can adjust the implementation code to align with the preferred behavior.

jorenbroekema commented 9 months ago

This PR kinda got lost a bit because it ended up being a tad more complicated than I foresaw when I first created the issue https://github.com/tokens-studio/sd-transforms/issues/128 , I added a comment that explains the issue and a potential way to solve it, let me know if you still want to work on this PR