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: Add API to exclude tokens from JSON output #53

Closed
wants to merge 1 commit into from

Conversation

fralongo
Copy link
Member

We are currently including all exposed tokens in the JSON output, but some of them (e.g. motion keyframes) only makes sense in a CSS context due to their dependency with CSS custom variables. This API allows to exclude them, and will be used in the main components package.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@fralongo fralongo requested a review from a team as a code owner July 12, 2023 10:42
@fralongo fralongo requested review from Al-Dani and removed request for a team July 12, 2023 10:42
@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (b8caa85) 96.81% compared to head (c170063) 96.83%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #53      +/-   ##
==========================================
+ Coverage   96.81%   96.83%   +0.01%     
==========================================
  Files          42       42              
  Lines         974      978       +4     
  Branches      114      116       +2     
==========================================
+ Hits          943      947       +4     
  Misses         29       29              
  Partials        2        2              
Impacted Files Coverage Δ
src/__fixtures__/common.ts 100.00% <100.00%> (ø)
src/build/internal.ts 100.00% <100.00%> (ø)
src/build/tasks/public-tokens.ts 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@just-boris
Copy link
Member

Can people ignore these tokens on their end? There could be different use-cases, are we going to create one more JSON, if these tokens become useful for somebody else?

@fralongo
Copy link
Member Author

Can people ignore these tokens on their end? There could be different use-cases, are we going to create one more JSON, if these tokens become useful for somebody else?

I think that it is safer to remove them now. Adding them in the future is backward compatible.

@just-boris
Copy link
Member

We have a defined list of public tokens. Having exclusions from this list seem like a system design concern for me

@fralongo
Copy link
Member Author

We have a defined list of public tokens. Having exclusions from this list seem like a system design concern for me

I will add documentation about it in the design tokens page.

@fralongo
Copy link
Member Author

Closing this PR as we prefer clarity and cleanliness to the risk of publishing tokens that might be unusable in some tech stacks.

@fralongo fralongo closed this Jul 12, 2023
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.

2 participants