amzn / style-dictionary

A build system for creating cross-platform styles.
https://amzn.github.io/style-dictionary/#/
Apache License 2.0
3.74k stars 524 forks source link

Cannot override built in transforms #1206

Closed okadurin closed 1 month ago

okadurin commented 1 month ago

In v4 if trying to register a transform with the name which is already registered internally, the custom transform function will never be called. Instead the internal one will be called. In v3 it worked. Either because name/camel was not defined internally or because the library allowed overriding transforms, not sure. Here is example of the code. Transform group is defined at the config object.

  StyleDictionary.registerTransform({
    name: 'name/camel',
    type: 'name',
    transform(token, options) {
       //...
    },
  });

  StyleDictionary.registerTransformGroup({
    name: 'custom/ios',
    transforms: ['name/camel'],
  });

If renaming name/camel it starts working.


This could be a documentation update (if any) or the code could be updated so that if the user registers a transformer with the internal name, then the library allows to override it. Maybe it's not even internal names. Maybe it just ignores registerTransform for the same name after it was registered once. I did not check that. For me I just renamed the transform and the code began to work.

jorenbroekema commented 1 month ago

Yeah this is probably a regression of the v4 restructure of the StyleDictionary object and how hooks are now registered.

I'm guessing the desired behavior when a hook under name X already exists is:

https://github.com/amzn/style-dictionary/blob/v4/lib/Register.js#L100 The code here is what's wrong, it's using deepmerge for registering hooks but merging is not appropriate here is the desired behavior is overriding, instead it should be:

target.hooks = {
  ...target.hooks,
  transforms: {
    ...target.hooks.transforms,
    [name]: {
      type,
      filter,
      transitive: !!transitive,
      transform: transformFn,
    },
  },
});

And repeat that pattern for the other 7 hooks as well within that Register class.

PRs welcome!