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

Official Typescript Enums for Formats and Transforms? #1367

Closed
DarioSoller opened this issue Oct 14, 2024 · 6 comments
Closed

Official Typescript Enums for Formats and Transforms? #1367

DarioSoller opened this issue Oct 14, 2024 · 6 comments

Comments

@DarioSoller
Copy link

DarioSoller commented Oct 14, 2024

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;
};
@DarioSoller DarioSoller changed the title Official Enums for Formats and Transforms? Official Typescript Enums for Formats and Transforms? Oct 14, 2024
@jorenbroekema
Copy link
Collaborator

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
Copy link
Author

DarioSoller commented Oct 21, 2024

@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
Copy link
Author

@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
Copy link
Collaborator

jorenbroekema commented Oct 24, 2024

  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> = 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<ThemeOptions>>>;

// looping over the enum
const options = (Object.keys(ThemeOptions) as Array<keyof ThemeOptionsEnumType>).map((val) => {
	return {
        // can use "val" (keyof ThemeOptionsEnumType) to index the enum now :)!
		value: ThemeOptions[val],
		label: ThemeOptions[val],
		id: val
	};
});
  1. yes
  2. I think so yeah
  3. As far as I can tell yep
  4. 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:

  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
Copy link
Author

  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.

DarioSoller pushed a commit to DarioSoller/style-dictionary that referenced this issue Oct 28, 2024
jorenbroekema pushed a commit to DarioSoller/style-dictionary that referenced this issue Nov 12, 2024
jorenbroekema pushed a commit to DarioSoller/style-dictionary that referenced this issue Nov 25, 2024
jorenbroekema pushed a commit to DarioSoller/style-dictionary that referenced this issue Dec 9, 2024
jorenbroekema pushed a commit to DarioSoller/style-dictionary that referenced this issue Dec 9, 2024
@DarioSoller
Copy link
Author

released with style-dictionary v4.3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants