search

amzn / style-dictionary

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

fix: empty token prop in filterTokenObject #1347

Closed mjmonline closed 2 months ago

mjmonline commented 2 months ago

Issue #, if available: My tokens often have a "description" key, if the value of that is null then it results in the following error:

const tokenValue = options.usesDtcg ? token.$value : token.value;
                                                               ^

TypeError: Cannot read properties of null (reading 'value')
    at file:///project-dir/node_modules/style-dictionary/lib/filterTokens.js:51:64
    at async file:///project-dir/node_modules/style-dictionary/lib/filterTokens.js:50:17
    at async filterTokenObject (file:///project-dir/node_modules/style-dictionary/lib/filterTokens.js:49:18)
    at async file:///project-dir/node_modules/style-dictionary/lib/filterTokens.js:65:24
    at async file:///project-dir/node_modules/style-dictionary/lib/filterTokens.js:50:17
    at async file:///project-dir/node_modules/style-dictionary/lib/filterTokens.js:50:17
    at async filterTokenObject (file:///project-dir/node_modules/style-dictionary/lib/filterTokens.js:49:18)
    at async file:///project-dir/node_modules/style-dictionary/lib/filterTokens.js:65:24
    at async filterTokenObject (file:///project-dir/node_modules/style-dictionary/lib/filterTokens.js:49:18)
    at async file:///project-dir/node_modules/style-dictionary/lib/filterTokens.js:65:24

Description of changes: Added a check (if (!token)) right after initializing the acc variable within the reduce function inside filterTokenObject.

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 months ago

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

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

jorenbroekema commented 2 months ago

Could you add a test for this so we can ensure not to create a regression for this bug in the future? Inside __tests__/filterTokens.test.js

mjmonline commented 2 months ago

@jorenbroekema added one of my problematic tokens to the test file. The current tests fails without the fix so I didn't add any new tests. I noticed that it only happens with my shadow tokens and not with any other type of token, I guess it's the structure of those tokens.

I recognise that the plugin I use could export them with a nicer format but nonetheless, no harm in this fix.

jorenbroekema commented 2 months ago

Alright nice, I was trying to make sense of the code and your test and I did get it eventually but I figured I'd make a couple of changes and make it a bit easier to follow the code / test you contributed, https://github.com/amzn/style-dictionary/pull/1354 it's here, I committed it so you are a co-author of it and will still count as a contribution to this project with your name, I hope that's fine!

jorenbroekema commented 2 months ago

closing this and proceeding with https://github.com/amzn/style-dictionary/pull/1354 to fix it