design-tokens / community-group

This is the official DTCG repository for the design tokens specification.
https://tr.designtokens.org
Other
1.56k stars 63 forks source link

Optional token $type #173

Closed honzatmn closed 2 years ago

honzatmn commented 2 years ago

Since we plan to extend our token format in Supernova, we're also checking Design Tokens Format Module specification and trying to figure out differences in what is defined in the format vs. how we work with tokens in Supernova. We'd like to eventually support also import based on DT Format, and I was surprised by the decision to have $type property for tokens and parent group as optional. Especially because the promise of $type in this specification is that tools can reliably interpret their value.

I found older discussions in #63 and #72, but it seems that it might result in situation where the token type is not defined at all. In that case, this applies:

Otherwise, the token's type is whichever of the basic JSON types (string, number, boolean, object, array or null) its value is.

However, if the type is based on the basic JSON type, we often get just a string. In these cases, we'll need to guess what is the type of the token based on the value. This is in direct conflict with another paragraph in the Types section:

Tools MUST NOT attempt to guess the type of a token by inspecting the contents of its value.

So I'm a bit confused about how should vendors who implement the Design Tokens Format approach this.

Would the group please consider making the $type mandatory either for a token, or a group in which the token lives? It would really make it easier for vendors to implement the format reliably.

Thanks a lot for considering.

romainmenke commented 2 years ago

Also see : https://github.com/design-tokens/community-group/issues/120

The fallback to the JSON type is a source of ambiguity and in my opinion it is logical to explicitly type every value.

c1rrus commented 2 years ago

The way I think about is:

I'd argue that

{
  "my token": {
    "$value": [1,2,3]
  }
}

is just a shorthand way of saying:

{
  "my token": {
    "$value": [1,2,3],
    "$type": "array"
  }
}

Those examples are exactly equivalent - they define a token called "my token" whose type is array. The first one is just a bit more terse.

Now, whether or not an array token is actually useful in the context of UI design is debatable. Perhaps not.

However, I don't think that means we can assume tokens that use one of the basic JSON types are always mistakes. As I've just commented over in #120, I think there are potentially good uses for those basic JSON types, which is why I'm advocating to keep them in the spec. And, if we keep them, then we may as well also keep the current type resolution algorithm which means $type properties can be optional.

However, if the type is based on the basic JSON type, we often get just a string. In these cases, we'll need to guess what is the type of the token based on the value.

Is your actual concern here that people and/or tools will accidentally forget to set a $type in their token files when they actually meant for them to be one of the non-JSON types our spec defines (e.g. a color or dimension)?

While I can certainly see how that is possible, it's hard to tell how much that will actually happen in the wild. (Though, if you have any real world usage stats, I'd love to hear them. You mention that you "often get just a string". Are Supernova users already trying to import files they've authored based on the draft spec perhaps?)

I can see how mandating a $type property either on a token or one of its parent groups could mitigate the risk of accidentally forgetting to type your tokens as you had intended. It would force authors to be explicit about their tokens' types. So, even if they choose to use something unusual like boolean as the type, they'd be explicitly stating that somewhere in the file which makes it clear that it's intentional.

Thinking about it, maybe that would be better. I suppose any tokens that do not have or inherit a $type or reference a token that does, would need to be considered invalid. Tools would then be expected to skip such invalid tokens while parsing (and, ideally, show a suitable warning or error message to the user).

What do others think?

TravisSpomer commented 2 years ago

Your comment in #120 has successfully convinced me that the JSON primitive types are actually good and useful! 🙂 But it does still seem likely to me that the vast majority of tokens without a $type explicitly present will be mistakes—in fact, your comment has convinced me of that even more. And allowing $type on groups to cascade down to the tokens within makes reading token files more complicated and annoying for both humans and code. What we get in exchange is saving a person hand-writing token files a few characters of typing, and if the main use of those primitive types is for extensions (which we hope are not too common), then those savings would be both small and rare.

So then it seems like there are two major cons to $type being optional and one minor pro. That's a bad tradeoff so I'm still strongly in favor of $type being required always.

romainmenke commented 2 years ago

All of what @TravisSpomer said!

A benefit of requiring $type is that it reserves optionality for future use. See the CSS error handling concepts for background : https://www.w3.org/TR/css-syntax-3/#error-handling

By requiring $type now it is possible to make it optional in the future. It will then be an informed choice based on real world cases within this specification.

If we make it optional now we can never take that back.

donnavitan commented 2 years ago

Based on the discussion leaning towards the benefits of $type being more helpful towards interop rather than the convenience of typing less, I also like that it provides explicit clarity for the token.

kevinmpowell commented 2 years ago

In my experience, having an explicit $type has been beneficial - especially when working with cross-platform systems. I agree with making $type required for all tokens (acknowledging that it can be inherited from a group's $type definition).

kevinmpowell commented 2 years ago

Closing this issue, in favor of the longer thread on #139 (which is a duplicate topic). The format editors will reference both of these issues when discussing and coming to a resolution.