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

Feature request: let createPropertyFormatter resolve chained references when using options.outputReferences #960

Open abritinthebay opened 1 year ago

abritinthebay commented 1 year ago

Use case: I'd love to use the property formatter, but I want full resolution (to avoid duplication) for my custom format. Unfortunately it's not possible without rolling my own version of createPropertyFormatter.

Simply put - a chain of..

a.b.value = "1";
a.c.value = "{foo.bar.value}";
a.d.value = "{foo.baz.value}";

will mean that resolving the a.d.value reference will get you a.c.value not a.b.value. I can absolutely see this use case--especially with CSS variable scoping--but given that resolving the value would get you 1 it's odd to not be able to do the reverse at all.

Wanting this behavior but not having it in the default createPropertyFormatter means having to write my own exact copy... just with a simple addition of recursion of the reference resolution (I pulled out the code into a local resolveReferences function) inside the refs.forEach

      if (dictionary.usesReference(ref.original.value)) {
        ref = resolveReferences(ref, dictionary);
      }

Obviously you could also modify in place, etc. Lots of options, but the point is to make it possible to recursively resolve the references up the chain. Given the current behavior is certainly wanted, likely as a default, putting this behind an option flag like recursiveReferences or something would make sense.

chazzmoney commented 1 year ago

This seems like a good change. If you would be up for writing a PR to modify the existing function and add some documentation about the new option, we can get this in.

jorenbroekema commented 11 months ago

Since we have outputReferences and outputReferencesFallback already, it might make sense to go to this API:

{
  "outputReferences": {
    "enabled": true,
    "fallbacks": true,
    "recursive": true
  }
}

Which would be a breaking change but I'm happy to pull this topic into v4 major, I think outputReferences in general could do with a bit of redesign anyways.

yinm commented 10 months ago

Can I try it?

jorenbroekema commented 10 months ago

Can I try it?

https://github.com/amzn/style-dictionary/issues/1032 Might make sense to follow this issue, I expanded a little bit more on this there. I haven't started on trying to implement this yet because I first want to have a chat with the other maintainers since it's quite a big restructure, which is hopefully tomorrow.

Feel free to reach out to us in our Slack -> style-dictionary-v4 channel! This is also where I announce any new prereleases or ask for feedback, so it's a good one to follow the progress.