Closed ilikescience closed 2 years ago
Coming at this from the perspective of working on a parser for this spec (and have built parsers for OpenAPI previously), IMO this doesn’t make parsing any easier on its own. So if this were pursued it should be for other benefits.
The reason this doesn’t help parsing is that value
isn’t unique to groups; it’s also found on all tokens. Further, it’s impossible to infer the type from any value
, as composite types will have object values.
If we wanted to make parsing groups easier, I’d expect taking the JSON Schema approach of enforcing a unique type
on every node (in this case, type: "group"
). I think that’s been proposed before and it was decided against (unless I’m mistaken)? The value
would help in tandem with type
, but again, type: "group"
would be doing the heavy lifting being the unique identifier that says “this node is a group and could not be anything else.”
It’s worth pointing out adding value
would open up namespacing to allow more names, so in that regard this seems like an alternate proposal to #72.
I'm dabbling with building a DTCG file parser myself (written in TypeScript) and I'm also not convinced this change would make parsing easier.
In my case, the goal is to construct a tree of Group
and DesignToken
objects that represents the file's contents and can then be inspected and/or manipulated via an API that those objects expose. It's analogous to how an HTML or XML file is parsed into a DOM. I'm building a TOM (Token Object Model), if you will 😜
My naïve parsing algorthim is like so:
JSON.parse()
$value
property. If it does, instantiate a Token
instance from the data.$...
props and instantiate a Group
instance from those. Then iterate over the remaining properties of that object and for each one:
Group
or DesignToken
from that property's data as needed)Group
or DesignToken
. So, in my case, having "$type": "group"
be the thing that differentiates a group from a token (and having the child items under $value
) wouldn't really make much difference. I'd still be looping over all the child items regardless.
I'm sure other tools/parsers might use different approaches, so if someone has a case where the syntax being proposed in the OP is beneficial, I'd love to hear about it.
In the absence of that, I'm inclined to agree with @drwpow though: If we were to adopt this, it should be for other reasons.
With the recent decision to use $
as a prefix for "special" format properties, I feel we've got a good solution for future spec versions to add new properties without clashing with people's group and token names. So while this proposal could have been an alternative to that, I think that ship has sailed.
Could there be other benefits to this?
I suppose it makes the syntax for groups and tokens similar, which some may find nice. Then again it also makes the files more verbose. Either way, it's quite subjective though. Maybe we need to have a vote?
Style Dictionary-format token files also just consider anything with value
a token and everything else a group, and that convention has worked fine for me so far in everything I've done with tokens. I don't see a reason why checking for node.$type == "group"
is any easier than "value" in node
when writing a parser. So since the "$type": "group"
option is more verbose, my vote goes to leaving it out.
(Plus, requiring "$type": "group"
seems like it would completely block off the possibility of having tokens and groups with the same name as in #97, though having a special token name that's ignored when exporting would still work.)
Reviewed by the format editors on 2022-2-22
The format editors discussed this issue and feel that adding value
to a group would complicate the spec and make it harder to read and write. We don't believe there would be performance gains for parsers or translators.
Decision We’re closing this issue for now, but may reopen it in the future if needed.
In conversations about groups and tokens sharing a name (#97), I had a few ideas about group format.
The current spec has us write the tokens in a group as properties of that group. This affords a high level of flexibility in writing and reading tokens, but results in a lot of extra work on the parsing side[^1]. And that's a totally fine tradeoff to make, but I thought it might be worth exploring ways to get some more concrete definition/formatting in place to help parsers, without losing human read/write-friendliness.
So, an example group according to the current spec (not using prefixes currently being discussed in #61):
My suggestion (but by no means the only possibility) is that groups, like tokens, should have a "value" property.
I don't write a parser/translator (hopefully the folks who do can weigh in), but I imagine that being able to expect every object to have a "value" property, regardless of it being a group or token, allows for some performance gains (and code maintenance gains).
Additionally, having all the group's tokens in a
value
property make it faster/easier to access them.Finally, and this is just a personal aesthetic preference, but it makes me happy to have a very concise and consistent grammar associated with the spec; every object has a "value" property!
What do y'all think?
[^1]: in that getting the tokens in a group requires enumerating all the properties of the object, then removing any properties that are reserved words