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

Alias $type inheritance? #236

Open
drwpow opened this issue Apr 9, 2024 · 4 comments
Open

Alias $type inheritance? #236

drwpow opened this issue Apr 9, 2024 · 4 comments

Comments

@drwpow
Copy link

drwpow commented Apr 9, 2024

Was digging into alias behavior a bit, and I had a few question about alias $types. What are peoples’ thoughts on whether any of the following are valid or invalid? Also I may have missed a previous conversation or part of the spec that applies to this.

Omitting $type

Is this valid?

{
  "color": {
    "a": { "$type": "color", "$value": "#336699" },
    "b": { "$value": "{color.a}" }
}

$type mismatch

Will color.b be a color, and be validated as such (sort of an extension of the previous one)?

{
  "color": {
    "a": { "$type": "color", "$value": "#336699" },
    "b": { "$type": "dimension", "$value": "{color.a}" }
}

And an even bigger question: is declaring ANY $type on an alias invalid?

$type inheritance

Will the inherited $type also apply to the alias? (and override any possible $type the alias may have)?

{
  "base": {
    "$type": "color",
    "a": { "$value": "#336699" }
  },
  "semantic": { "$value": "{base.a}" }
}

In all of these, I’d like to reduce error as much as possible, and I can’t think of a good reason for even declaring $type on aliases because there’s at best there’s duplication; at worst there’s the possibility of a conflict. And wondering when it’s helpful to err, when it’s helpful to warn, and when it’s helpful to do nothing (silently work)

@whoisryosuke
Copy link

I think the $type alias is to allow for parsers just to know to check an object and its properties. Otherwise how would it know to check for colors? It’d have to assume a token structure (like all colors are under top level color property).

It also helps explicitly define the type of the token, since you might want to mix tokens to achieve another (like combining opacity number and RGB color to make a RGBA new color). Or for example, if you multiply fontSize and a spacing token, what is the type of the result? Might get weird for parsers.

I do think there you have a great idea of not including it every time though. It seems verbose and makes it difficult to manually parse JSON without a tool to strip the noise. Not sure how to practically achieve that without an explicit structure or some other system to accommodate for the inference of types.

Also this issue emphasizes the need for a section of the docs on parsing, and expectations the spec has - to create some consistency between tooling.

@drwpow
Copy link
Author

drwpow commented Apr 12, 2024

I think the $type alias is to allow for parsers just to know to check an object and its properties. Otherwise how would it know to check for colors? It’d have to assume a token structure (like all colors are under top level color property).

Well that’s just the thing though—parsers actually have to validate aliases currently. Aliases have to be non-circular. And non-aliases have to be valid tokens. By just accepting an invalid schema at face-value, the user will get errors with no clear action on how to fix it. So parsers actually do have to scan the alias and know it’s referring to a color anyway, and there’s no way for them to avoid having that work (or they’d be bad, hard-to-use-tools). But I was more interested in exploring whether that’s an error to show to the user “fix this!” or if it just silently works

Also this issue emphasizes the need for a section of the docs on parsing, and expectations the spec has - to create some consistency between tooling.

💯!

@ilikescience
Copy link

ilikescience commented May 1, 2024

The spec lists out the following resolution order:

A token's type can be specified by the optional $type property. If the $type property is not set on a token, then the token's type MUST be determined as follows:

  • If the token's value is a reference, then its type is the type of the token being referenced.
  • Otherwise, if any of the token's parent groups have a $type property, then the token's type is inherited from the closest parent group with a $type property.
  • Otherwise, the token's type is whichever of the basic JSON types (string, number, boolean, object, array or null) its value is.

So, in your examples

{
  "color": {
    "a": { "$type": "color", "$value": "#336699" },
    "b": { "$value": "{color.a}" }
}

This is valid, and color.b has a type of color.

{
  "color": {
    "a": { "$type": "color", "$value": "#336699" },
    "b": { "$type": "dimension", "$value": "{color.a}" }
}

This is invalid; the spec says that a token with the type dimension must be "a string containing a number (either integer or floating-point) followed by either a "px" or "rem" unit". Since {color.a} doesn't match this, this is an invalid token.

{
  "base": {
    "$type": "color",
    "a": { "$value": "#336699" }
  },
  "semantic": { "$value": "{base.a}" }
}

This is valid, and semantic would get resolved to having the type of color (since base.a has the type of color).

The spec is pretty unambiguous about these, though it might need a bit more language to acknowledged that a referenced token might inherit its type, too, either from its group or another reference.

I can’t think of a good reason for even declaring $type on aliases

Yes, almost any type on a token with a value like {base.a} will result in an invalid token. There's probably some weird edge cases where you can have a token with a fontFamily type which, according to the spec, would be valid, resulting in css like font-family: '{base.a}' ... but resolving this would probably create substantially more edge cases than it would solve.

@drwpow
Copy link
Author

drwpow commented May 2, 2024

There's probably some weird edge cases where you can have a token with a fontFamily type which, according to the spec, would be valid, resulting in css like font-family: '{base.a}' ... but resolving this would probably create substantially more edge cases than it would solve.

Oh I hadn’t even thought of this, but you’re right just evaluating a token’s $type by parsing its value could lead to many errors. Especially if string tokens ever were accepted (not that they are in the DTCG now, but mainly thinking of Figma variables’ current implementation where string types can refer to colors or typography values).

The spec is pretty unambiguous about these, though it might need a bit more language to acknowledged that a referenced token might inherit its type, too, either from its group or another reference.

Agreed—I think clarifying this would help. The resolution definition you provided earlier implies this, but doesn’t outright explicate it, and it could be clearer.

Thanks for clarifying! This seems like it’s not really ambiguous, then, how the spec is interpreted. More just a possible documentation TODO of “should alias types get a little more specific regarding this behavior or not”.

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

3 participants