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 json output format #1513

Merged
merged 5 commits into from
Jul 25, 2023
Merged

feat: add json output format #1513

merged 5 commits into from
Jul 25, 2023

Conversation

dstaley
Copy link
Contributor

@dstaley dstaley commented Jul 19, 2023

πŸ“Œ Summary

Adds a new output format that generates a JSON file that can be re-consumed by other Style Dictionary instances.

πŸ› οΈ Detailed description

As part of marketing's adoption of HDS, we want to generate a set of tokens that build upon the existing tokens of HDS. There are a handful of instances where we take a color value and perform modifications on it (such as lighten/darken) that are only possible if we have access to the underlying token value. For example:

{
  "color": {
    "vault": {
      "button": {
        // ...
        "background-color-hover": {
          "type": "color",
          "value": "{color.vault.brand}",
          "modify": [
            { "type": "mix", "args": ["black", 0.06, { "space": "srgb" }] }
          ]
        },
        // ...
        "border-color-focus": {
          "type": "color",
          "value": "{color.vault.foreground}",
          "alpha": "0.1"
        }
      }
    }
  }
}

Currently, we maintain an entire fork of the tokens package to accomplish this, but have realized that if we could pass the HDS tokens into another instance of Style Dictionary, then we could reduce our fork to just our custom tokens.

This PR adds a third output format using the built-in json format. For simplicity, it doesn't configure a custom formatter to remove unnecessary properties, but that's a weak opinion loosely held.

This JSON file can then be passed into Style Dictionary like so:

const config: Config = {
  source: ['src/marketing/**/*.json'],
  include: [
    require.resolve(
      '@hashicorp/design-system-tokens/dist/products/tokens.json',
    ),
  ],
  platforms: {
    'web/css-variables': {
      transformGroup: 'marketing/web',
      buildPath: 'dist/marketing/css/',
      prefix: 'hdsplus',
      basePxFontSize: 16,
      files: [
        {
          destination: 'tokens.css',
          format: 'css/variables',
          filter(token: DesignToken) {
            return token.isSource
          },
        },
      ],
    },
  },
}

This configuration will generate a new set of tokens that have access to the underlying token values from HDS, but won't overwrite them given it uses a different prefix.

πŸ“Έ Screenshots

N/A

πŸ”— External links

HashiCorp Slack link: https://hashicorp.slack.com/archives/C05CBQ2J6J3/p1689625431482409


πŸ‘€ Reviewer's checklist:

  • +1 Percy if applicable
  • Confirm that PR has a changelog update via Changesets if needed
  • Confirm that A11y tests have been run locally for this component

πŸ’¬ Please consider using conventional comments when reviewing this PR.

@vercel
Copy link

vercel bot commented Jul 19, 2023

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Updated (UTC)
hds-showcase βœ… Ready (Inspect) Visit Preview Jul 21, 2023 5:18pm
hds-website βœ… Ready (Inspect) Visit Preview Jul 21, 2023 5:18pm

@didoo
Copy link
Contributor

didoo commented Jul 21, 2023

@dstaley I had a quick look at the changes, and I have some thoughts:

  • I am not super keen in adding formats to the other "targets" when they're not used (products, devdot) when technically what we want is to add a new export type only for marketing
  • I think there is an opportunity to add marketing as a "target", and explicitly set which components you want to use (import later) and potentially even include "base" tokens for marketing if makes sense (something to discuss later, probably)
  • I have did a quick test, to make sure what I have in mind actually works (πŸ˜€) so I am going to send you the git patch in DM so you can play with it and see if makes sense to you too, and maybe even clean up the code/logic (it's been done quickly as a test)

@dstaley
Copy link
Contributor Author

dstaley commented Jul 21, 2023

@didoo Great points! I've refactored the implementation to incorporate your feedback and the patch you sent over (which was super helpful, so thank you!). This new approach adds a platforms configuration option to the target config which allows us to specify the output platforms for each target. products and devdot have ['web/css-variables', 'docs/json'] whereas marketing has ['web/css-variables', 'json']. This PR also enables the marketing target.

@didoo
Copy link
Contributor

didoo commented Jul 21, 2023

@dstaley I love your approach, more neat and the code flows better. Let me review the PR "officially" (I've left already a couple of comments in your last commit).

Copy link
Contributor

@didoo didoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good for me. Great work! πŸ™Œ

🚒

.changeset/wild-trains-decide.md Outdated Show resolved Hide resolved
Co-authored-by: Cristiano Rastelli <cristiano.rastelli@hashicorp.com>
@didoo
Copy link
Contributor

didoo commented Jul 25, 2023

@dstaley when you want, you can merge the PR (you don't need more approvals, right? or are there other blockers I am not aware of?)

@dstaley dstaley merged commit f8e15bb into main Jul 25, 2023
12 checks passed
@dstaley dstaley deleted the ds.json-format branch July 25, 2023 17:24
@hashibot-hds hashibot-hds mentioned this pull request Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants