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

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

Open
abritinthebay opened this issue Apr 12, 2023 · 4 comments

Comments

@abritinthebay
Copy link

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

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

jorenbroekema commented Oct 24, 2023

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

yinm commented Dec 3, 2023

Can I try it?

@jorenbroekema
Copy link
Collaborator

Can I try it?

#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.

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

4 participants