Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: recursively expand nested composite tokens #1244

Merged
merged 3 commits into from
Jun 17, 2024

Conversation

Imable
Copy link

@Imable Imable commented Jun 17, 2024

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.

@Imable Imable requested a review from a team as a code owner June 17, 2024 08:00
Copy link

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

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

Copy link

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

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

@jorenbroekema
Copy link
Collaborator

jorenbroekema commented Jun 17, 2024

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
Copy link
Author

Imable commented Jun 17, 2024

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

@jorenbroekema jorenbroekema merged commit 8450a45 into amzn:v4 Jun 17, 2024
5 checks passed
@jorenbroekema
Copy link
Collaborator

Thanks for contributing :)!

jorenbroekema added a commit that referenced this pull request Jun 28, 2024
* feat: recursively expand nested composite tokens

* fix: expand object type check, maintain ref if possible

* fix: handle multi-value object tokens like shadows

---------

Co-authored-by: Abel van Beek <abel.van.beek@tromsfylke.no>
Co-authored-by: jorenbroekema <joren.broekema@gmail.com>
dbanksdesign pushed a commit that referenced this pull request Jun 28, 2024
For a full list of changes see the CHANGELOG.md or the [migration guide](https://styledictionary.com/version-4/migration/) from version 3.x.x to version 4.0.0

* BREAKING: make style-dictionary ESM-only and browser-compatible (#1014)
* Update docs (#1051)
* chore: upgrade glob to latest, set posix:true, verify & fix Windows (#1070)
* chore: fix glob usage in script (#1074)
* fix: glob dependency (#1075)
* chore: release (prerelease) (#1076)
* feat: improve and test ref utils error handling (#1077)
* feat: expose flattenTokens utility (#1079)
* fix: resolved filepath, async parser await (#1083)
* feat: allow deferring transitive transforms from the transformer (#1069)
* feat: support w3c special properties (#1081)
* feat: expose typeW3CDelegate util function (#1087)
* fix: small bug with w3c type delegate util (#1088)
* fix: only run postinstall scripts in non- prod-mode (#1091)
* fix: publish missing script file to npm (#1093)
* fix: dev scripts only run in sd cwd (#1095)
* fix: escape double quotes for ts outputStringLiterals (#1097)
* W3C Syntax refactor to allow value & type token names/groups (#1100)
* feat: support async for all hooks (#1106)
* fix: dtcg option not passed in some places (#1116)
* fix: outputReferenceFallbacks for css/variables formatter (#1113)
* fix: missing type imports (#1118)
* feat: allow transforms to be added on top of transformGroup (#1120)
* chore: fix ref docs mistakes, add DTCG option (#1122)
* feat: remove reliance on CTI, add token type (#1123)
* feat: redesign logging (#1125)
* fix: outputReferences for DTCG spec tokens (#1127)
* feat: allow passing Filesystem volumes to SD instance / extend method (#1128)
* chore: update docs volumes (#1130)
* fix: absolute paths in Node env (#1134)
* feat: add warnings disabled option, fix logging issues (#1136)
* fix(types): add missing type keyword for type export from index.d.ts (#1139)
* Revamp docs (#1141)
* feat: allow outputReferences function, publish filter util for it (#1145)
* feat: move formatHelpers to utils entrypoint (breaking) (#1147)
* chore: add migration docs (#1148)
* Some small docs fixes, and removing docsify (#1149)
* fix: change merging behavior for tokens to override (#1151)
* fix: asset url handling in CSS format (#1153)
* feat: add outputReferencesTransformed utility function (#1154)
* fix: outputReferencesTransformed util handling object-value tokens, unfiltered (#1155)
* chore: fix some docs links (#1159)
* fix: logging to be in sequence per platform (#1161)
* fix: get rid of lodash, refactor templates to ES6 (#1160)
* Format options improvements (#1165)
* feat: turn off generated timestamp by default in file headers (#1174)
* Expand composites (#1169)
* Align Hooks APIs (#1185)
* feat: move fileHeader global prop to hooks.fileHeaders (#1177)
* feat: move filter global prop to hooks.filters, rename matcher to filter (#1179)
* feat: move action global prop to hooks.actions (#1180)
* feat: move parsers global prop to hooks.parsers, apply explicitly (#1181)
* feat: move transformGroup global prop to hooks.transformGroups (#1182)
* feat: move transforms global prop to hooks.transforms, align api (#1183)
* feat: move formats global prop to hooks.formats, align api (#1184)
* fix: color filter now skips invalid colors (#995) (#1188)
* chore: docs mentions extend twice, duplicate (#1192)
* feat: add object-value CSS shorthand transforms (#1194)
* feat: pass options to fileheaders and filters, add unfilteredAllTokens (#1205)
* fix: pass options to transforms filters, live demo main page (#1208)
* chore: allow preselecting playground file (#1209)
* fix: typeDtcgDelegate  at any position, size/rem apply to unitless values (#1210)
* feat: add convert to DTCG spec utilities, file converter docs site (#1200)
* chore: version inject for sd-playground on build (#1216)
* docs(DTCG): Update DTCG docs with latest link and fix full name (#1215)
* chore: add mermaid integration to docs, create architecture (#1221)
* feat: add formatPlatform/formatAllPlatforms methods (#1222)
* fix: allow register hooks override existing (#1223)
* feat: pass dictionary options to preprocessors (#1224)
* feat: pass ref path in getReferences util output (#1226)
* chore: add missing changeset for SD format platform methods (#1227)
* timingFunction and fontFamily props, optional border props (#1229)
* fix: expand on platform level to adjust path prop (#1231)
* fix: fix CLI, update basic example, fix android templates DTCG (#1235)
* feat: allow not throwing on broken refs (#1240)
* feat: recursively expand nested composite tokens (#1244)


---------

Co-authored-by: Andreas Nordahl <andnorda@gmail.com>
Co-authored-by: tkgroot <tkuppensgroo@uos.de>
Co-authored-by: Lukas Oppermann <lukasoppermann@github.com>
Co-authored-by: Lukas Oppermann <oppermann.lukas@gmail.com>
Co-authored-by: Joren Broekema <joren@Jorens-MacBook-Pro.local>
Co-authored-by: Tom Pietrosanti <Rykus0@users.noreply.github.com>
Co-authored-by: Kaelig Deloumeau-Prigent <kaelig@users.noreply.github.com>
Co-authored-by: Daan Vos de Wael <daanvosdewael@users.noreply.github.com>
Co-authored-by: Abel van Beek <abel.van.beek@tromsfylke.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants