search

amzn / style-dictionary

A build system for creating cross-platform styles.
https://amzn.github.io/style-dictionary/#/
Apache License 2.0
3.74k stars 524 forks source link

feat: recursively expand nested composite tokens #1244

Closed Imable closed 2 weeks ago

Imable commented 2 weeks ago

Description of changes

My organization structures design tokens such that design tokens are dynamically linked with a color from our color palette and its foreground color. This requires recursive expansion of composite tokens, which until now was marked // TODO in code.

I reused the expandTokensRecurse function to recursively expand the expanded tokens of a composite token. If the composite token only contains leafs (no composite token), no work is done.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

aws-amplify-us-west-1[bot] commented 2 weeks ago

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-1244.d16eby4ekpss5y.amplifyapp.com

aws-amplify-us-west-1[bot] commented 2 weeks ago

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-1244.d1ouz7xofr5p4l.amplifyapp.com

jorenbroekema commented 2 weeks ago

Hey, thanks for raising this, I made some minor adjustments to simplify it a little and I also adjusted your test to test with a DTCG use case of nested object-values (border -> strokeStyle), your test example is a bit more custom and technically not compliant with DTCG because color values aren't supposed to be object-values (as of now).

That said, I did also run your test locally, ensuring your use case is actually fixed. It does pass with my adjustments, with one small change:

            Background: {
              Surface: {
                Background: {
                  value: '#ffffff', // <-- {Gray.0.Background} in my test because it can remain a ref, due to resolved value being a string value

I found an edge case as well with the strokeStyle test because it has a multi-value dimension (Array type), and I was doing typeof 'object' checks so it would erroneously expand array values, now it checks whether the value is a plain object which fixes that bug.

I also added a changeset ensuring it will be in next release.

Imable commented 2 weeks ago

Awesome @jorenbroekema ✨ The more general test case is a lot more suitable too. Much appreciated!

jorenbroekema commented 2 weeks ago

Thanks for contributing :)!