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

Remove JSON derived types from the specification #120

Closed
romainmenke opened this issue Mar 25, 2022 · 16 comments · Fixed by #201
Closed

Remove JSON derived types from the specification #120

romainmenke opened this issue Mar 25, 2022 · 16 comments · Fixed by #201
Assignees

Comments

@romainmenke
Copy link
Contributor

romainmenke commented Mar 25, 2022

https://design-tokens.github.io/community-group/format/#types

This part of the specification assumes that implementers will have an inherent understanding of certain types and how to handle them.

This implicit typing will lead to interop issues.
It's better to have exact definitions for each type supported in the schema.

An obvious example is String.
When converting a value to CSS it is ambiguous if it should become an ident token or a string token.

{
  "$value": "center"
}
.foo {
  content: "center";
}

or

.foo {
  text-align: center;
}

You also can't do this in the tokens file :

{
  "$value": "\"center\""
}

Because that dictates how strings work in any consuming context.

@valhead valhead added dtcg-format All issues related to the format specification. To be reviewed by editors Issues that need to be reviewed in an upcoming meeting between editors. labels Jul 13, 2022
@kevinmpowell kevinmpowell added this to the Next Draft Priority milestone Oct 3, 2022
@kevinmpowell kevinmpowell removed dtcg-format All issues related to the format specification. To be reviewed by editors Issues that need to be reviewed in an upcoming meeting between editors. labels Oct 3, 2022
@c1rrus
Copy link
Member

c1rrus commented Oct 14, 2022

I've been making certain assumptions about how tools could / should handle certain scenarios. I'll detail them below for discussion. Whether we accept these or something else, I do agree that these kinds of things should be made explicit in the spec.

1. Tools don't necessarily need to use every type of token value. It is acceptable for tools to just ignore types that are not relevant to them

For example, when importing a design tokens file into a colour palette editing tool, I'd argue that it's quite reasonable for that tool to pluck out only the color tokens and present those to its users. Any other type of token could just be ignored. (It might however make sense to require tools that can import and export design token files to at least preserve any tokens they don't use themselves - see discussion in #157)

Note that for this to work it is necessary for every tool to implement the type resolution algorithm defined in the spec. However once that has been done (presumably while parsing the imported file), the tool can then choose to operate on whatever subset of tokens are relevant to it.

Therefore, if we retain the basic JSON types like string, boolean, etc., that doesn't mean tools that have no use for them would be forced to do anything with them.

2. The list of types supported by the spec will grow over time

As time goes by, I'm sure new use-cases for design tokens will arise and that is likely to necessitate more types being added to our spec.

I think your CSS text-align example has actually highlighted the need for a new type we don't currently have. Perhaps we should add a textAlignment type, whose value must be one of "left", "center", "right" etc. Just like it does for the other types, the spec would then articulate what the meaning of those values are, so that any tool that wants to work with textAlignment tokens can do so in a consistent way.

For example, a translation tool that wants to export them as CSS code could then output the corresponding CSS keywords without quotes.

3. The basic JSON types can be useful for extensibility

I'd argue that thenumber type can be useful in the context of UI design & development - e.g. for z-indexes, unitless lineheights, aspect ratios (e.g 16:9 = 1.7777777), etc. However, the string, boolean, null, object and array types probably don't have obvious applications in UI design.

However, used together with $extensions, they might enable teams and products to add new, proprietary types. If any of those start to gain traction in the community, they could help inform future versions of the spec where we can add new types to replace them.

The value of $type has to be one of the types defined in the spec, so you can't do this:

{
  "token name": {
    "$type": "myFancyType", // <-- not allowed!
    "$value": ...
  }
}

However, you can do something like this:

{
  "token name": {
    "$type": "...", // <-- whichever type is the "closest" to what you need
    "$value": ... , // <-- value that is valid for the chosen type
    "$extensions": {
      // Tell your tool to interpret this token's value in a special way
      "com.your-tool.custom-type": "myFancyType"
    }
  }
}

Now the "closest" type you pick here could technically be any of the spec's types - one of the more DS-oriented ones like color, duration, etc. or one the JSON ones string, array, etc.. However, since our DS-oriented types each have very specific rules for their corresponding values, they probably don't lend themselves to be extended so much. The JSON types on the other hand are semantically meaningless (a bit like <div> and <span> in HTML), so they probably are useful for this sort of use-case.

While I hope that using extensions & the JSON types in this way to create new, proprietary token types will be relatively rare (I'm certainly not trying to advocate for every tool to go and invent lots of proprietary types!), I do think it's useful to retain this option. As I mentioned earlier, this may enable "cowpaths" to emerge which future spec iterations can "pave". And, one of the DTCG principles is "Focused, yet extensible" :-)


If you agree with my assumptions above, then I'd argue we should keep the number, string, boolean, null, object and array types, because:

  • We're not forcing tools to have to do anything special with them (because they're allowed to just ignore tokens of that type if they wish), so keeping them shouldn't make life more difficult for tool makers
  • They enable teams and tools to create proprietary types, which can help inform future spec iterations and (as per my 2nd point) I do expect more types to be needed in the future

And, aside from all of that, I think we should consider adding something like a textAlignment type to the spec!

@CITguy
Copy link

CITguy commented Oct 15, 2022

1. Tools don't necessarily need to use every type of token value. It is acceptable for tools to just ignore types that are not relevant to them

The value of $type has to be one of the types defined in the spec, so you can't do this:

{
  "token name": {
    "$type": "myFancyType", // <-- not allowed!
    "$value": ...
  }
}

If tools can ignore any type they wish, then why wouldn't they also ignore non-standard "$type": "myFancyType"?

Tooling that I'm currently building relies on custom token $types to basically extend where the spec falls short. If you have to define your custom type via $extensions, then the $type resolution algorithm cannot be fulfilled.

@TravisSpomer
Copy link

I think that the biggest concern there would be this:

  1. you start using a non-standard textAlignment type with values left, right, and center
  2. a year from now a textAlignment type is added to the spec with values start, end, and middle
  3. now your tooling barfs on standard token files that use the word start because your tooling expected left

That's a bit contrived though. And if you named your non-standard textAlignment property custom.textAlignment or the like, that problem goes away entirely.


On the topic of this issue: for what it's worth, I was definitely against the JSON primitive types being allowed as valid token types before reading James's post today, but I have successfully been converted to the pro-primitive-types side with that $extensions example. If we assume that most tools will simply ignore tokens with those types, they suddenly become very useful.

@romainmenke
Copy link
Contributor Author

romainmenke commented Oct 15, 2022

I don't really understand the arguments for the JSON derived types.

To me this argument looks like this :

do the work to define base types do not do this work
interop issues between tools unlikely likely
compatibility issues between spec versions unlikely likely

Not doing the work and hoping that the type system of the carrier format (JSON) perfectly aligns with this specification is just not a good idea in my opinion.

The main difference is that unknown/unexpected things become invalid things.
This makes error handling vastly more powerful and improves interop between tools and compatibility between versions.

It might sound weird that errors are desirable but it is much better to have a hard error that allows ignoring a token.

see : https://www.w3.org/TR/css-syntax-3/#error-handling


The specification discourages using $extension for typing.

https://tr.designtokens.org/format/#extensions

In order to maintain interoperability between tools that support this format, teams and tools SHOULD restrict their usage of extension data to optional meta-data that is not crucial to understanding that token's value.

I think this was the right call and it is a mistake to suddenly suggest the opposite in this issue.

That does not mean that implementors can not experiment with non-standard types.
They just can not do so within the specification.

Implementers can do a spec+ approach like Sass or go for vendor prefixes.


And, aside from all of that, I think we should consider adding something like a textAlignment type to the spec!

It is the opposite that needs solving :)

What is something with type string in JSON and without any $type.

  • unknown, should be ignored by tools, might get special meaning through $extension
  • a keyword (left, auto, inherit, ...)
  • text ("some text")

What is something with type string in JSON and with "$type" : "string".

  • a keyword (left, auto, inherit, ...)
  • text ("some text")

How do we express what is left?

@kevinmpowell
Copy link
Contributor

Branching the text-alignment/CSS Keyword concerns into a separate issue for further discussion: #177

Keeping this issue focused on keeping/removing the JSON types from the specification.

@ChucKN0risK
Copy link
Contributor

After reviewing this issue, the editors decided to keep JSON derived types in the specification.

Why
Keeping JSON derived types is beneficial for many reasons:

  1. It makes the spec flexible enough to support a whole variety of use cases we can't guess.
  2. It removes the burden of thinking about all types possible and thus it helps us maintain and release the spec as easily as possible.

JSON derived types enable teams and tools to create proprietary types, which can help inform future spec iterations and we do expect more types to be needed in the future.

@romainmenke
Copy link
Contributor Author

romainmenke commented Oct 31, 2022

@ChucKN0risK This is really unexpected.
I truly can not understand the reasoning here combined with the core concept of this specification.

This specification exists to remove interop issues between tools.
If anyone needs a type that doesn't exist in the specification they should open an issue here so that it can be specified and a new version can be published.

This is in direct conflict with :

It makes the spec flexible enough to support a whole variety of use cases we can't guess.

JSON derived types enable teams and tools to create proprietary types, which can help inform future spec iterations and we do expect more types to be needed in the future.

If teams or tools need to store and communicate bits of information that are not part of this specification they can use $extensions or use a separate file.


It removes the burden of thinking about all types possible and thus it helps us maintain and release the spec as easily as possible.

It can be as simple as a reference to the JSON specification and "$type": "string"
This extra effort does not way up against the ambiguity in the current specification.

@romainmenke
Copy link
Contributor Author

If type prototyping is a concern it would be more interesting to have something like this :

{
  "$value": [{"anything": "you need"}],
  "$type": "com.toolprovider.custom-type"
}

Any tool that doesn't recognise this type will leave it as-is.
And toolprovider can experiment with their idea while simultaneously submitting an issue here for feedback.

This creates a conflict free mechanic/space for experimentation.
It does not require implicit fallbacks to JSON types.

@romainmenke
Copy link
Contributor Author

@kevinmpowell @ChucKN0risK @c1rrus This question was also unanswered :

#120 (comment)

What is something with type string in JSON and without any $type.

unknown, should be ignored by tools, might get special meaning through $extension
a keyword (left, auto, inherit, ...)
text ("some text")

@TravisSpomer
Copy link

Whether the format allows custom $type values or use the primitive types plus an $extension enables roughly the same set of scenarios.

Pros of using JSON primitive types and $extension:

  • Allows a v2 of the token format to define custom composite token $types right in the token file
  • The list of valid $type values stays small (though it's not clear if that matters since tools should probably ignore types they don't explode when reading files in v2 of the format)

Pros of using a custom $type value:

  • Easier to understand what's going on
  • Less typing
  • We probably could remove the confusing basic JSON types from the spec at that point

The way I see it, the better of the two solutions thus depends on how important that "custom composite token types right in the token file" scenario is to you. I'm not sold on the importance of that—I'm struggling to come up with a scenario where you'd want to be able to store a composite token in the JSON but wouldn't need your tools to understand anything about those tokens other than their schema. In all of those cases, it seems like a custom $type would be a somewhat better solution anyway.

If we can say that custom $types would cover all of those scenarios that people are thinking they'd need custom composite types for, then that seems superior to me. But we'd still need the "basic data plus extra stuff in $extensions" pattern for other types of data, so eliminating the pattern for this one case might not matter that much.

@CITguy
Copy link

CITguy commented Nov 18, 2022

Recently, I was thinking about how HTML remains forward compatible with attribute and element names, and I wonder if it would be helpful to take a page from their book and do something like the following.

Specific to $type

  • all DTCG supported types will have camelCase spelling
    • all compliant tooling should support built-in types anyway, so there's no additional responsibility requirements on downstream tools
  • custom types MUST be hyphenated
    • similar to HTML custom element naming requirements (e.g., <my-input>, <some-button>, <our-fancy-thing>)
  • the DTCG spec should provide a means of defining document-level metadata (maybe #/$types) that would allow token authors to register a custom type via a { "$types": { "<type>": "<schema>" } } configuration
  • when a tool loads a tokens.json file, it should fetch the custom type schemas such that it would augment the base set of built-in token types
    • after registering the custom types internally, the tool can then use that information to validate, create, and modify custom token types

Related

  • Similar to HTML data-* attributes, I believe $extensions fulfills the role of defining syntactically-inert data for custom functionality.

@romainmenke
Copy link
Contributor Author

I don't think the specification needs another convention to indicate that something is custom and I also don't think that that solves this issue.

The specification already defines a good naming format for extensions that can be reused to indicate that a type is user/tool defined.

this could be : "$type": "com.toolprovider.custom-type"


What are the practical benefits of having a schema for user/tool defined typed?
I don't see how any tool can provide any functionality for a token type it doesn't understand without having a full specification for how to define these schema's and how to act on them.


Both aspects don't really have anything to do with the point of this issue.
Which is that the JSON derived types are problematic because they are underspecified.

After @ChucKN0risK stated that the purpose is to allow custom types I think they are vastly more problematic.

JSON derived types enable teams and tools to create proprietary types, which can help inform future spec iterations and we do expect more types to be needed in the future.

User/tool defined types should be clearly marked as such, they should not be a side effect of vagueness in the specification.

That again leaves JSON derived types without any purpose, which means they should be removed from the specification.

@c1rrus
Copy link
Member

c1rrus commented Nov 29, 2022

Thanks all for the additional comments. The editors have re-reviewed this issue and come to the conclusion that, yes, we should remove the JSON types after all! :-)

  • As others have pointed out, most of the raw JSON types don't have obvious applications in UI design
    • Possble exceptions to this are number (e.g. for unitless line heights) and string (e.g. for microcopy like company names, slogans, etc.). We will therefore separately explore adding new types into the spec to cover those kinds of things.
  • Since we've also decided to no longer fallback on JSON types when no $type property is present on the token or its parent groups, they're no longer needed for that
  • As per the comments above, using JSON types together with $extensions as a means of creating proprietary types is problematic. Since they'd still be "valid" tokens, other tools may have interpreted them in unintended ways if they didn't also understand the $extension.
    • We do however think it's useful for the spec to provide a means of defining proprietary types1, so I'll open a separate issue to discuss that (FWIW, something along the lines of what @romainmenke and @CITguy have proposed above like $type values that contain periods - e.g. "$type": "com.example.custom-type" - looks promising for this)

I'll leave this issue open until we've made and merged a PR that updates the spec accordingly.

Footnotes

  1. Though, similar to $extensions, usage of such a feature should be for exceptional cases only. We intend to add some guidance into the spec's wording to make that clear(er).

@CITguy
Copy link

CITguy commented Dec 5, 2022

FYI: another use for "$type": "number" is z-index/elevation level

@c1rrus
Copy link
Member

c1rrus commented Jan 3, 2023

I've just opened a PR to make the required changes: #201 :-)

@CITguy it retains the number type as it was needed by the gradient type. As you say, there are other uses for that type too such as z-index / elevation. I've left out string or any other potential candidates for (re-)inclusion in the spec in order to keep the PR more focused.

It'll need to be approved by other editors before it can be merged. But you're all very welcome to take a look and add review comments. Thanks!

@romainmenke
Copy link
Contributor Author

Thank you everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants