amzn / style-dictionary

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

Platform in transformer is not optional anymore #991

Closed lukasoppermann closed 6 months ago

lukasoppermann commented 1 year ago

Hey @dbanksdesign,

in this commit: https://github.com/amzn/style-dictionary/commit/c43263bfd5223a7f24525f20aa87958aeedab812# you changed Platform to be required in transformer. Is this on purpose or was that an accident and it should stay optional?

pascalduez commented 1 year ago

Hey,

also in this commit, the options argument for a Transform['transformer'] is becoming required. Whereas it's not if I scan the code and all the docs, it's completely fine to solely use a token param.

It now errors when trying to reuse a core transformer such as:

const pxToRem = token =>
  StyleDictionary.transform['size/pxToRem'].transformer(token);
Transform.d.ts(23, 5): An argument for 'options' was not provided.

There's a piece of reply in https://github.com/amzn/style-dictionary/issues/975#issuecomment-1569135741 but for Platform.

in the meantime I will:

const pxToRem = (token, options = {}) =>
  StyleDictionary.transform['size/pxToRem'].transformer(token, options);
jorenbroekema commented 6 months ago

I think this is intended, many of our built-in transforms rely on those options being passed and in general our consumers that create transforms should be able to assume they are passed. If you call the transformer callback yourself imperatively (I kinda consider it private API), you should adhere to that signature