amzn / style-dictionary

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

Official Typescript Enums for Formats and Transforms? #1367

Open DarioSoller opened 1 month ago

DarioSoller commented 1 month ago

I think it would be a favourable improvement, if you could centrally provide Typescript Enums for a few things, rather than having everyone writing their own. This would create a smoother DX for anyone using Style-Dictionary in a Typescript project, because a single source of truth (SSOT) for these things also make future update paths easier, cause Typescript errors can guide you for identifying breaking changes in the values for the formats or transfroms.

While I do acknowledge that enums are controversial in some scenarios, I think that they would really shine here for having more explicitly defined format and transform options. As enums can be used as a type but also as a constant, I see some unused benefits for within the Style-Dictionary code base itself and for anyone using Style-Dictionary in a Typescript project.

My proposed two enums that I created myself look like the following: sd-predefined-transforms.enum.ts

// extracted from https://github.com/amzn/style-dictionary/blob/main/lib/common/transforms.js
export enum SdPredefinedTransforms {
  attributeCti = 'attribute/cti',
  attributeColor = 'attribute/color',
  nameHuman = 'name/human',
  nameCamel = 'name/camel',
  nameKebab = 'name/kebab',
  nameSnake = 'name/snake',
  nameConstant = 'name/constant',
  namePascal = 'name/pascal',
  colorRgb = 'color/rgb',
  colorHsl = 'color/hsl',
  colorHsl4 = 'color/hsl-4',
  colorHex = 'color/hex',
  colorHex8 = 'color/hex8',
  colorHex8android = 'color/hex8android',
  colorComposeColor = 'color/composeColor',
  colorUIColor = 'color/UIColor',
  colorUIColorSwift = 'color/UIColorSwift',
  colorColorSwiftUI = 'color/ColorSwiftUI',
  colorCss = 'color/css',
  colorSketch = 'color/sketch',
  sizeSp = 'size/sp',
  sizeDp = 'size/dp',
  sizeObject = 'size/object',
  sizeRemToSp = 'size/remToSp',
  sizeRemToDp = 'size/remToDp',
  sizePx = 'size/px',
  sizeRem = 'size/rem',
  sizeRemToPt = 'size/remToPt',
  sizeComposeRemToSp = 'size/compose/remToSp',
  sizeComposeRemToDp = 'size/compose/remToDp',
  sizeComposeEm = 'size/compose/em',
  sizeSwiftRemToCGFloat = 'size/swift/remToCGFloat',
  sizeRemToPx = 'size/remToPx',
  sizePxToRem = 'size/pxToRem',
  htmlIcon = 'html/icon',
  contentQuote = 'content/quote',
  contentObjCLiteral = 'content/objC/literal',
  contentSwiftLiteral = 'content/swift/literal',
  timeSeconds = 'time/seconds',
  fontFamilyCss = 'fontFamily/css',
  cubicBezierCss = 'cubicBezier/css',
  strokeStyleCssShorthand = 'strokeStyle/css/shorthand',
  borderCssShorthand = 'border/css/shorthand',
  typographyCssShorthand = 'typography/css/shorthand',
  transitionCssShorthand = 'transition/css/shorthand',
  shadowCssShorthand = 'shadow/css/shorthand',
  assetUrl = 'asset/url',
  assetBase64 = 'asset/base64',
  assetPath = 'asset/path',
  assetObjCLiteral = 'asset/objC/literal',
  assetSwiftLiteral = 'asset/swift/literal',
  colorHex8flutter = 'color/hex8flutter',
  contentFlutterLiteral = 'content/flutter/literal',
  assetFlutterLiteral = 'asset/flutter/literal',
  sizeFlutterRemToDouble = 'size/flutter/remToDouble',
}

sd-file-formats.enum.ts

// Extracted from https://github.com/amzn/style-dictionary/blob/main/lib/common/formats.js
export enum SdFileFormats {
  androidColors = 'android/colors',
  androidDimens = 'android/dimens',
  androidFontDimens = 'android/fontDimens',
  androidIntegers = 'android/integers',
  androidResources = 'android/resources',
  androidStrings = 'android/strings',
  composeObject = 'compose/object',
  cssVariables = 'css/variables',
  flutterClassDart = 'flutter/class.dart',
  iosColorsH = 'ios/colors.h',
  iosColorsM = 'ios/colors.m',
  iosMacros = 'ios/macros',
  iosPlist = 'ios/plist',
  iosSingletonH = 'ios/singleton.h',
  iosSingletonM = 'ios/singleton.m',
  iosStaticH = 'ios/static.h',
  iosStaticM = 'ios/static.m',
  iosStringsH = 'ios/strings.h',
  iosStringsM = 'ios/strings.m',
  iosSwiftAnySwift = 'ios-swift/any.swift',
  iosSwiftClassSwift = 'ios-swift/class.swift',
  iosSwiftEnumSwift = 'ios-swift/enum.swift',
  javascriptEs6 = 'javascript/es6',
  javascriptModule = 'javascript/module',
  javascriptModuleFlat = 'javascript/module-flat',
  javascriptObject = 'javascript/object',
  javascriptUmd = 'javascript/umd',
  json = 'json',
  lessIcons = 'less/icons',
  lessVariables = 'less/variables',
  scssIcons = 'scss/icons',
  scssMapDeep = 'scss/map-deep',
  scssMapFlat = 'scss/map-flat',
  scssVariables = 'scss/variables',
  stylusVariables = 'stylus/variables',
  typescriptEs6Declarations = 'typescript/es6-declarations',
  typescriptModuleDeclarations = 'typescript/module-declarations',
}

Usage example:

import { File } from 'style-dictionary';
import { SdFileFormats } from '../../enums/sd-file-formats.enum'; // Would be cool if this would also come from the Style-Dictionary import above

export const cssOutputFileConfig = (): Array<File> => {
  const files: Array<File> = [
    {
      destination: 'tokens.css',
      format: SdFileFormats.cssVariables,
    },
    {
      destination: 'tokens-refs.css',
      format: SdFileFormats.cssVariables,
      options: {
        outputReferences: true,
      },
    },
  ];
  return files;
};
jorenbroekema commented 1 month ago

I think this would be a good idea, we'll also need it for the built in actions and transformGroups.

I would also suggest using CONSTANT_CASE for the keys, seems to be a convention for string constants quite often in JS projects.

DarioSoller commented 1 month ago

@jorenbroekema you're right that this constant naming convention is often used, but there are also trends in Javascript against this. Some people say that UPPER_SNAKE_CASE for constants is old fashioned and comes from times, where IDEs have not been capable to inform a developer about a variable being a constant and read-only. Angular for example does not follow this principle and they used to have it explicitly in their coding style guides, the way I did it. Couldn't find it on a quick search though, but they still stick to this rule.

Anyhow, I would prefer a little bit more modern naming approach, but I do not have strong feelings about it, as I am aware, that UPPER_SNAKE_CASE is a well known naming pattern.

I would love to bring this to a PR. Let me have a look into this and then I might need a few more answers, bevor I can provide a first solution for it.

DarioSoller commented 1 month ago

@jorenbroekema can you answer me the following questions, before I start adding things to a PR:

  1. Do I see that right, that the types a separately maintained right now? So there is nothing generated directly out of the lib folder, if I am seeing this correctly? This would mean that we can't make use of the enums in the JS source right now. Are you planing to migrate the whole Style-Dictionary source code to Typescript one day?
  2. There are two predefined actions at the moment: 'android/copyImages' and copy_assets?
  3. These are the predefined TransformGroups: web, js, scss, css, less, html, android, compose, ios, 'ios-swift', 'ios-swift-separate', assets, flutter, 'flutter-separate', 'react-native'
  4. FileFormats and Transforms as listed above in my original description are correct?
  5. Any dependencies of the TS types into the tests or anything else, I have to keep an eye on?
jorenbroekema commented 4 weeks ago
  1. yes correct, only Typescript users of this library will benefit from enums, as JavaScript files do not allow importing types as values. No I'm not planning to move it to TypeScript any time soon, the tooling overhead is a headache, I prefer buildless, especially for libraries. I recognize that this is rough with regards to using enums, but we can definitely get the same type safety benefits by exporting a JavaScript object for format/transform/transformGroup/action keys instead of using TypeScript enums. I think enums are tough to work with anyways and you need to know about the trickery required to do stuff like this:
    
    export enum ThemeOptions {
    EXCLUDED = 'excluded',
    INCLUDED = 'included'
    }

type ValueOf = T[keyof T]; // using Partial so the props are not required // using Record because that's basically what the enum is here type ThemeOptionsEnumType = Partial<Record<keyof typeof ThemeOptions, ValueOf>>;

// looping over the enum const options = (Object.keys(ThemeOptions) as Array).map((val) => { return { // can use "val" (keyof ThemeOptionsEnumType) to index the enum now :)! value: ThemeOptions[val], label: ThemeOptions[val], id: val }; });

2. yes
3. I think so yeah
4. As far as I can tell yep
5. Yeah there's probably tests or other places where we use the hard-coded strings

As for the naming thing, I went looking if we have some convention already in our source code for this and I stumbled upon the logger utility (`lib/utils/groupMessages.js`:
```js
  this.GROUP = {
      PropertyReferenceWarnings: 'Property Reference Errors',
      PropertyValueCollisions: 'Property Value Collisions',
      TemplateDeprecationWarnings: 'Template Deprecation Warnings',
      RegisterTemplateDeprecationWarnings: 'Register Template Deprecation Warnings',
      SassMapFormatDeprecationWarnings: 'Sass Map Format Deprecation Warnings',
      MissingRegisterTransformErrors: 'Missing Register Transform Errors',
      PropertyNameCollisionWarnings: 'Property Name Collision Warnings',
      FilteredOutputReferences: 'Filtered Output Reference Warnings',
      UnknownCSSFontProperties: 'Unknown CSS Font Shorthand Properties',
    };

which is later used as:

const PROPERTY_VALUE_COLLISIONS = GroupMessages.GROUP.PropertyValueCollisions;
const PROPERTY_REFERENCE_WARNINGS = GroupMessages.GROUP.PropertyReferenceWarnings;
const UNKNOWN_CSS_FONT_PROPS_WARNINGS = GroupMessages.GROUP.UnknownCSSFontProperties;
const FILTER_WARNINGS = GroupMessages.GROUP.FilteredOutputReferences;

Kinda confusing coz it's mixing CONSTANT_CASE with PascalCase 😓 . I'm not really sure what's best here, I'm leaning to just converting it all to camelCase and keeping things simple. These mixed casings seem a bit outdated indeed. So let's just go with camelCase and we can bring the consistency to the other places in another PR someday.

Instead of enums, we could do this instead: image

So just exporting a key value pair JS object, for each hook, and then adding an entrypoint to StyleDictionary so people can import like so:

import { formats, transforms } from 'style-dictionary/enums';

// optional, if you want to ensure that it's read-only and the members cannot be assigned to, to make it similar to enums
const formatsReadOnly = formats as const;

More info: https://www.typescriptlang.org/docs/handbook/enums.html#objects-vs-enums

DarioSoller commented 4 weeks ago
  1. Gotcha and agree with going for JS objects in this case
  2. 👍
  3. 👍
  4. 👍
  5. as const is the way to go!

I think I can start on a PR now! Thanks for your clarifications with the details.