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

transform/transfromGroup platform option override possibility on file level #1298

Open
DarioSoller opened this issue Aug 5, 2024 · 4 comments
Labels
5.0 Might plan for v5 since it's breaking discuss

Comments

@DarioSoller
Copy link

DarioSoller commented Aug 5, 2024

I was finally trying out Style-Dictionary 4 and once again stumbled over one particular setting. I have searched thru the existing tickets, but could not find anything in this direction, even though I can't imagine being the only one who stumbled over this so far?

  • Search through existing ✅
  • Ensure that you have tested the latest version of the framework: "style-dictionary": "^4.0.1"

Let me explain:
transform and transformGroup settings are part of the platform configs within Style-Dictionary. Commonly and popular used platforms are web, ios and android, like in your mulit-brand-multi-platform example that I used for testing.

Now I was playing with some more output formats and noticed that having the tokens as file configuration of javascript/es6 being part of the web platform generates an invalid Javascript constant name, cause the transformGroup: 'web' has a default naming convention transform of name/kebab:
Bildschirmfoto 2024-08-05 um 16 12 32
Now, in general, Javascript would still be a token output definition which should be part of the web platform IMO.

Therefore the current Style-Dictionary platform and file configuration possibilities seem unintuitive from a DX point of view, because I have to make another platform config that has a naming convention transform of name/camel for a Javascript token output file and need to configure the buildPath to still be part of the web folder. As you usually loop over these settings like in your example, this makes some extra if-conditions for the buildPlatform() calls necessary.

The same applies to iOS, if you need to support old iOS Objective C header files and swift at the same time.

A look in my example code should make things clearer:

import StyleDictionary from 'style-dictionary';

// HAVE THE STYLE DICTIONARY CONFIG DYNAMICALLY GENERATED
const getStyleDictionaryConfig = (brand, platform) => {
  return {
    // Source example tokens coming from: https://github.com/amzn/style-dictionary/tree/main/examples/advanced/multi-brand-multi-platform/tokens
    source: ['tokens/globals/**/*.json', `tokens/brands/${brand}/*.json`, `tokens/platforms/${platform}/*.json`],
    platforms: {
      web: {
        transformGroup: 'web',
        buildPath: `token-package/dist/web/${brand}/`,
        files: [
          {
            destination: 'tokens.css',
            format: 'css/variables',
          },
          {
            destination: 'tokens.scss',
            format: 'scss/variables',
          },
          {
            destination: 'tokens.json',
            format: 'json',
          },
        ],
      },
      js: {
        transformGroup: 'web',
        transforms: ['name/camel'], // needed only for javascript
        buildPath: `token-package/dist/web/${brand}/`,
        files: [
          {
            destination: 'tokens.js',
            format: 'javascript/es6',
          },
        ],
      },
      ios: {
        transformGroup: 'ios',
        buildPath: `token-package/dist/ios/${brand}/`,
        files: [
          {
            destination: 'tokens.h',
            format: 'ios/macros',
          },
        ],
      },
      iosSwift: {
        transformGroup: 'ios-swift',
        buildPath: `token-package/dist/ios/${brand}/`,
        files: [
          {
            destination: 'tokens.swift',
            format: 'ios-swift/any.swift',
          },
        ],
      },
    },
  };
};

// PROCESS THE DESIGN TOKENS FOR THE DIFFERENT BRANDS AND PLATFORMS
await Promise.all(
  // flatMap: This method first maps each element using a mapping function, then flattens the result into a new array. It’s useful when you need to flatten nested arrays. Since flatMap will flatten the nested arrays of promises, the outer function no longer needs to be async.
  ['brand-1', 'brand-2', 'brand-3'].flatMap(brand =>
    // dependent on the tokens/platforms folder structure
    ['web', 'ios', 'android'].map(async platform => {
      const config = getStyleDictionaryConfig(brand, platform);
      const sd = new StyleDictionary(config);
      await sd.hasInitialized;

      // Additionally need buildPlatform() calls, in order to get i.e. the Javascript output file nicely into the web output file folder
      if (platform === 'web') {
        await sd.buildPlatform('js');
      } else if (platform === 'ios') {
        await sd.buildPlatform('iosSwift');
      }

      return sd.buildPlatform(platform);
    }),
  ),
);

Theoretically I would welcome to have the possibility to override transform and even transformGroup settings done on the platform level for specific files in their file configuration settings.

  • Supporting the properties transform and transformGroup on file level, should probably be possible without creating an breaking change for Style-Dictionary, as it only enhances the file config API with optional properties!?
  • I am convinced that this setting would support much cleaner configuration domains and less confusions for certain output file scenarios.
  • Why is that even important? Big enterprises or corporations sometimes do not even know, which teams or vendors have to work with the design tokens, thus providing a huge variety of output file formats are really crucial to support a wholesome design system adoption across such big companies.
  • Such an increased configuration flexibility would much better scale with the rising number of output file formats that will probably come in the future.

With my suggestion the example code above would then look like this:

import StyleDictionary from 'style-dictionary';

// HAVE THE STYLE DICTIONARY CONFIG DYNAMICALLY GENERATED
const getStyleDictionaryConfig = (brand, platform) => {
  return {
    // Source example tokens coming from: https://github.com/amzn/style-dictionary/tree/main/examples/advanced/multi-brand-multi-platform/tokens
    source: ['tokens/globals/**/*.json', `tokens/brands/${brand}/*.json`, `tokens/platforms/${platform}/*.json`],
    platforms: {
      web: {
        transformGroup: 'web',
        buildPath: `token-package/dist/web/${brand}/`,
        files: [
          {
            destination: 'tokens.css',
            format: 'css/variables',
          },
          {
            destination: 'tokens.scss',
            format: 'scss/variables',
          },
          {
            destination: 'tokens.json',
            format: 'json',
          },
          {
            destination: 'tokens.js',
            transforms: ['name/camel'],
            format: 'javascript/es6',
          },
        ],
      },
      ios: {
        buildPath: `token-package/dist/ios/${brand}/`,
        files: [
          {
            destination: 'tokens.h',
            transformGroup: 'ios',
            format: 'ios/macros',
          },
          {
            destination: 'tokens.swift',
            transformGroup: 'ios-swift',
            format: 'ios-swift/any.swift',
          },
        ],
      },
    },
  };
};

console.log('Build started...');

// PROCESS THE DESIGN TOKENS FOR THE DIFFERENT BRANDS AND PLATFORMS
await Promise.all(
  // flatMap: This method first maps each element using a mapping function, then flattens the result into a new array. It’s useful when you need to flatten nested arrays. Since flatMap will flatten the nested arrays of promises, the outer function no longer needs to be async.
  ['brand-1', 'brand-2', 'brand-3'].flatMap(brand =>
    // dependent on the tokens/platforms folder structure
    ['web', 'ios', 'android'].map(async platform => {
      const config = getStyleDictionaryConfig(brand, platform);
      const sd = new StyleDictionary(config);
      await sd.hasInitialized;
      return sd.buildPlatform(platform);
    }),
  ),
);

Complete working Typescript test project is added here as a zip: multi-brand-multi-platform-test-0.1.0.tgz

I would love to discuss on this or maybe I even miss a similar functionality of Style-Dictionary and I have been on the wrong track here? Let me know what you think!

@jorenbroekema
Copy link
Collaborator

jorenbroekema commented Aug 7, 2024

Sorry that it's taken a bit longer for me to respond to this but I think you've touched on a topic that requires a bit more thinking on abstract level about what the platform/transform lifecycle is versus the file/format lifecycle, and why certain token transformations happen on one level or the other.

The way I see it, there are different types of token transformations

Transforms that modify the actual inherent value of a token, usually computing/resolving/modifying, for example:

  • color modifiers (darken, lighten)
  • math computations

Things that transform the token in a way that can be categorized as a formatting-only change:

  • token name (kebab, camel)
  • converting from one unit to another (percentage to decimal, hex to UIColor())

Truth is, right now almost every transform essentially is put under platform/transform, and we can even see in a package like sd-transforms that this causes awkwardness, because we have to register different variants of the tokens-studio transformGroup depending on the chosen platform...

I think we need to go outside of our current way of looking at Style Dictionary and its global - platform - file thinking....
I feel like the computing/resolving/modifying types of transforms are, generally speaking, agnostic of the platform, and the formatting-only types of changes are generally only relevant for the format type, and even when the platform is the same (e.g. web, android or iOS), the outputs can vary significantly (JS vs CSS, Android XML vs Compose, iOS Obj-C vs Swift).

While this idea is maybe a bit wild, what about:

{
  source: [...],
  transformGroup: 'foo', // computing/resolving/modifying types of transforms grouped
  transforms: [...], // additionally/alternatively computing/resolving/modifying types of transforms
  outputs: [
    {
      format: "css/variables", // must be registered, responsible for formatting-only transforms
      files: ["tokens.css"],
      filter: ...,
      fileHeader: ...,
      options: { ... }
    },
    {
      format: "scss/variables",
      files: ["tokens.scss"],
    },
    {
      format: "json",
      files: ["tokens.json"],
    },
    {
      format: "javascript/es6",
      files: ["tokens.js"],
    },
  ]
}

So basically ditching this whole concept of the "platform" level being separate from the "format" level, by putting "actual" transforms on the global level (since they should be irrespective of platform) and putting formatting-only transforms on the format level where they actually belong.

@dbanksdesign I think this one requires your 2 eyes and brain capacity as well. I feel like thinking in this direction would also solve:

  • references resolution happening on a per-platform basis which seems weird and also really bad for performance
  • not allowing folks to combine a transitive transforms (e.g. color darken modifier) with a color format transform e.g. from hex to UIColor. The transitive transform happens later for tokens with refs, meaning the input to the color modifier is "UIColor()" which it wouldn't understand: Transforms that run after all references have been resolved #1063 more context in this issue

The main drawback from this platform-ditching approach is that token transforms (formatting-only) would now be delegated to the "format" level, so that work doesn't really get deduped anymore on platform level e.g. between let's say CSS/SCSS, most of the formatting transforms would overlap, we don't want to repeat those... so some kind of shared abstraction (like "platform" but choosing a better name maybe) for this seems necessary...

Obviously this would be a pretty significant architectural change as well as lots of API breaking changes, so it wouldn't be done until v5

@jorenbroekema jorenbroekema added discuss 5.0 Might plan for v5 since it's breaking labels Aug 7, 2024
@jorenbroekema
Copy link
Collaborator

Btw don't take this as a "I want to head in this direction", I'm just thinking out loud.

I kind of feel like my previous comment also makes a bit of a case for the "platform" abstraction, even if the name is maybe not ideal. I'm just trying to figure out if there's a way to redesign stuff so it makes more sense.

I think a big win would already be to solve #1063 (not breaking) to allow formatting-only transforms to run after value-changing transforms, and to delegate name-transforms to the formats (breaking) by setting a default but allowing to override through for example File.options.casing option shared across all built-in formats.

@DarioSoller the best way forward for you for now I think is to split your web platform into "js"/"css" and using those transformGroups, same for your ios/ios-swift

@DarioSoller
Copy link
Author

I really appreciate and thank you very much for sharing your thoughts on this @jorenbroekema !

I like your idea of thinking in different transform categories. If they still even should be all called transform might be questionable.

On top of your considerations of how best platforms, format or a new outputs could play nicely together, I think that the discussion from the last Tokens Sudio Office Hour, about how important for solving advanced calculations between tokens, it will be to separate a token value from its unit. As the units are also depending on the output file formats, this probably needs to be taken into account here as well!?

Sry for blowing it up and making it even more complicated 🙈 Just try to put all relevant things for a discussion on the table.

@jorenbroekema
Copy link
Collaborator

https://github.com/tokens-studio/sd-transforms/blob/main/test/spec/checkAndEvaluateMath.spec.ts

To an extent our resolve math utility already allows computing expressions with units, but there are some caveats:

  • Mixed units are not possible (with the exception of px since this is basically the same as unitless for web), e.g. 10rem * 8em is not computable because they are different units
  • Addition/subtraction of a relative unit with an absolute unit is not computable, e.g. 20px + 4em cannot be computed because px is absolute and em is relative. 20px * 4em is an exception case that is computable because this would always be 80em no matter the px value of em
  • https://github.com/tokens-studio/sd-transforms/blob/main/test/spec/checkAndEvaluateMath.spec.ts#L33 this case should be resolvable but I've just found that it's not, surprisingly. So that's probably a bug

Summary:
Yes, separating the value from the unit can help for computing math, but you can't always safely do so if the units are mixed or use a combination of relative/absolute. The math util in sd-transforms handles most cases pretty well with the exception of 1 as far as I can tell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.0 Might plan for v5 since it's breaking discuss
Projects
None yet
Development

No branches or pull requests

2 participants