Moves group properties into a metadata property

[c1rrus]

Updates the spec as per the discussion in issues #61 & #72 to move the description and type group properties into a metadata object to minimise naming clashes with group items now and in the future if more group properties are added.

[netlify]

✔️ Deploy Preview for dtcg-tr ready!

🔨 Explore the source changes: 96d1d7b

🔍 Inspect the deploy log: https://app.netlify.com/sites/dtcg-tr/deploys/61dcc40742f470000a23364a

😎 Browse the preview: https://deploy-preview-89--dtcg-tr.netlify.app

[dbanksdesign]

LGTM!

[dbanksdesign]

Nice!

[ChucKN0risK]

I’ve reviewed this and have nothing to add/trust the others for the outcomes.

[drwpow]

I like this. This allows someone to both set a default type for this group, as well as also create a type subgroup, which wouldn’t be out-of-the-question for typography tokens.

[c1rrus]

Good point! i hadn't actually considered that before.

In effect, this means that only value and metadata need to be reserved words. The reason is that value is the only required prop in a token and therefore the way in which parsers can differentiate between a group and a token.

I.e. if an object has a property called value, it must be a token. If not, then it must be a group. Consequently, groups cannot contain items called value.

[jorenbroekema]

Yeah, name clashing crossed my mind when I read this spec today, I like this change as well!

[drwpow]

To expand on this: I’m currently working on a parser for design tokens and the type key is a really important part of the schema, both for parsers as well as type systems like TypeScript to infer which is which. Having both a group node and token node possess a top-level type: "color" key would make everything exponentially harder/slower to reason about. So for that reason, I do like it being metadata on the group.

If group did have type, IMO it should be consistent with tokens and be self-identifying (type: "group"). But I’m not in favor of that as it would probably cause confusion with metadata.type, it’d be annoying to write, and it’s not that helpful for parsers. I’m in favor of separating self-identification with identifying children, which this PR does via type / metadata.type (edit: clarity)

[kaelig]

A quick question and a suggestion!

[kaelig]

The idea behind having description at the same level as value was to make it easily accessible, and ideally nudge authors toward documenting tokens.

Nesting the description field deeper could have a negative impact on this.

Curious if that's something that was brought up when deciding about this change?

[c1rrus]

I don't recall that specific point being brought up, so thanks for raising it.

Bear in mind that this change only affects groups, not tokens. In tokens, value, description and other properties all remain top-level. That's because, as per the spec, no other properties are allowed within a token.

On groups, properties are used as the names for items within that group, so having "special" properties like description mixed in amongst them could be problematic if we ever added new group properties in future spec versions as there'd be the risk of naming collisions with item names people might be using. Moving all group properties under metadata avoids this.

As far as nudging folks to add descriptions to groups goes, not having description as a top level property on groups is of course less convenient to type. However, I doubt it's that much of a chore that it would put people off describing their group at all. With a bit of luck, IDEs may one day offer auto-completion (a la Intellisense) for editing DTCG files which would further aleviate the issue. And GUI tools can hide the underlying file structure altogether - they could just display an editable decription text area near a group's name for instance.

Arguably, having group description and other properties all collated into an object of their own may actually aid readability as they might otherwise be randomly mixed in amongst the group's item names.

I'm therefore still in favour of making this change. What do others think?

[kaelig]

Thank you for this great context, this makes more sense now 👍🏻

And my apologies, I hadn't realized this was only impacting the schema of groups: my assumption was that if group descriptions were under metadata.description, for consistency they'd also have been nested in that location for tokens themselves.

This then raises the point: could this nesting inconsistency make the format harder to learn?

To illustrate my question, here's an explanation:

{
  amazingGroup: {
    // group description (nested under metadata object)
    metadata: { description: "my group" },

    // token description (direct child of the token)
    fantasticToken: { value: 15, description: "my token" },

    // expectation possibly set by group.metadata.description
    fantasticToken: {
      value: 15,
      metadata: { description: "my token" },
    }
  }
}

[donnavitan]

I like the idea of having consistent formatting whenever possible. And looking at this example now I can see the very subtle difference. Would having metadata on the group and the item create the same issue as having description at the top love?

[drwpow]

From a schema design perspective, it’s good to think in terms of what will break if a reserved word is added/removed/renamed. Namespacing (metadata.*) is a great way to mitigate against this.

Tokens are “closed,” meaning they only accept few reserved words (type, value, description). The potential for name changes won’t break much, as a user probably shouldn’t have any arbitrary keys on tokens. So in this scenario, metadata would just add length to everything when typing it out.

But groups are “open” in that a user may use any key they like. So any time a reserved word is added/removed/renamed, it may break all users’ schemas. Namespacing all reserved words into metadata (as the PR/Issue discuss) solves this.

All that to say it might seem inconsistent on the surface to have metadata on one and not the other, but IMHO isn’t inconsistent in practice because tokens and groups behave differently and have different constraints, and it’s a reflection of that.

[c1rrus]

Thanks @drwpow , I couldn't have said it better myself :-)

[c1rrus]

For what it's worth, there was an earlier suggestion in #61 by @ivnmaksimovic to do the inverse: All group's top-level properties are "closed" - just like they are for tokens - but there is a special tokens property, which then contains just the child items. For example:

{
  "group-1": {
    "description": "Description for group-1",
    "tokens": {
      "token-1": { "value": "..." },
      "token-2": { "value": "..." },
      "nested-group": {
        "tokens": {
          "deep-token-1": { "value": "..." }
        }
      }
    }
  }
}

I suppose you could argue it's more consistent from a stylistic point of view. But the trade-off is that it would require every group to always contain a tokens property (regardless of whether or not is also uses any other special group properties like description or type). It also makes your JSON files nest more deeply which could be a bit inconvenient when hand-editing files.

There's pros and cons either way, but my personal preference is still to go ahead with metadata for groups only. I'm keen to hear the other format editor's views though.

[ilikescience]

FWIW, I really like @ivnmaksimovic's proposal. My initial reaction to the metadata keyword was "isn't this all metadata?" ... which is a little naive, but I can definitely see a user trying to put metadata on an individual token.

Regardless, my preference would be that "description" goes in the same place on a group, single token, composite token, alias, etc. So that might mean 'open' metadata like this:

{
    "group-1": {
        "type": "group",
        "description": "Description for group-1",
        "value": {
            "token-1": {
                "type": "color",
                "description:" "Description for token-1",
                "value": "#ffff00"
            }
        }
    }
}

Or 'closed' metadata like this:

{
    "group-1": {
        "metadata": {
            "type": "group",
            "description": "Description for group-1",
        }, 
        "value": {
            "token-1": {
                "metadata": {
                    "type": "color",
                    "description:" "Description for token-1",
                },
                "value": "#ffff00"
            }
        }
    }
}

To me the latter feels like if javascript objects had something like Array.properties.length and Array.methods.push() instead of just Array.length and Array.push() ... but either way consistency means it's easy to reason about.

[c1rrus]

I've added a comment in the "reserved names" issue (#61) mentioning that we decided at the last format editors' meeting to debate this further before deciding how to proceed.

As I've mentioned there, I'm now wondering if we should reconsider prefixing all "special" properites in our spec (both for groups and for tokens). That also avoids naming clashes but keeps the syntax for groups and tokens similar.