-
Notifications
You must be signed in to change notification settings - Fork 66
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
Proposal: JSON Schema $ref for aliases (breaking change) #259
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for dtcg-tr ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for dtcg-tr ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
I've literally just read this, so this is a bit of a knee-jerk reaction (perhaps I'll feel differently after sleeping on it :-P), but... I have concerns about this proposal:
Re-reading #166 and thinking about the resolver spec, I wonder whether "referencing tokens in another file" is the right problem to be solving. Clearly, there's a strong demand for being able to split up your design tokens across multiple files - both to keep things manageable when you have a large number of design tokens, and to do stuff like theming by varying which files get included by some mechanism. But that doesn't necesarily require references that point to specific files. In many cases, the intent is to merge all the source files into a single collection of tokens, and then to do stuff with it. This is how tools like Theo and Style Dictionary have been doing it forever and I'd argue the Resolver Spec is another (more advanced) take on that concept too. Resolving design token references doesn't happen until after that file merging process has been done, so there's no need for them to specify a file that a token is in. We can debate whether or not that kind of merge-and-then-resolve-references approach is the "best" way to go, but my point is enabling tokens spread across multiple files can be achieved without changing the current reference syntax. So, considering how big of a breaking change this would be, I wonder if it's worth it. |
@c1rrus great thoughts, thank you! 🙏 I wanted to think on the points you raised for a few days.
This is a great callout, and I think really gets at the mental model of how people think about the DTCG spec now, how people want to think about it, etc. So this is exactly the conversation I wanted to have with this proposal, even if it doesn’t move forward. Specifically “Resolving design token references doesn't happen until after that file merging process has been done” can be preserved with this proposal with no changes. Again, see the comment from the JSON spec:
I interpret this as working exactly as aliases work today—you don’t have to preresolve those preemptively, and you can (and should) preserve all the pointers as actual pointers (not deep clones of the data). I do realize the spec changes as-written are confusing about that, so I should update the examples. But this can work the exact same as aliases do. And leaning on how JSON pointers are used today, the filenames themselves are negligible, again, they can work the same. It just removes the restriction of keeping tokens in files by necessity and moves it to a pattern where you can put any token in any file by choice. You can point to tokens in the same file, or remote files. Up to you.
Yeah performance wasn’t a primary motivator of the change, but this is mostly only true in JavaScript. I agree with you—it is fairly-trivial to parse and typecast on-the-fly. But in other languages that rely more heavily on fixed structures and memory allocation, leaning on structural type discrimination (like serde for Rust) is more ergonomic than string parsing and pattern-matching. Again, not to mention the fact that for JSON As an aside, after writing this I did think about the relation between this and the Resolver Spec more, and I reached the conclusion the Resolver Spec in its current form is sufficient, and whether we accept or reject this proposal won’t change that. The problems I also thought I would hit in the Resolver Spec ended up not being problems at all when I worked through a complex example! So all that to say I’d like to evaluate this solely on the basis of:
And we may come to the conclusion “no” and that’s OK with me. I’m happy whether this is accepted or rejected; so long as we evaluated it. |
As a tool author, it is very difficult to reliably tell if a given spec is a single file vs. multi file with remote refs. I don't work with JSON Schema enough but there's enough prior art that I could find easily find a solution within 5 minutes so this looks pretty good. At the same time, parsing "{" also wasn't that big of a pain point for me. And it does push us farther from human readable but I think that ship has sailed. I also would encourage more breaking changes. As a consumer, I understand the (draft) contract. |
```css | ||
:root { | ||
--color-palette-black: #000000; | ||
--color-text-primary: var(--color-palette-black); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: I also updated this earlier section introducing the term “Alias,” and used CSS variables rather than Sass variables. I think it better preserves this idea we want to keep—of aliases carrying through to runtime better. Plus I think in 2024, CSS variables are more widely-used than Sass variables (completely guessing; I have no data to back this up).
{ | ||
"color": { | ||
"blue": { | ||
"4": { "$value": "#218bff", "$type": "color" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: color syntax is waiting on #257, but can be updated here if necessary. It’s unrelated to this proposal.
That’s a great point, and it is a little bit of a papercut for sure—you don’t always know the depth of schemas that refer to other schemas until you try resolving it. But this is also a common problem that has over a decade of prior art and workarounds, too, so it’s not new. This goes beyond this PR, but in many setups today, people are already using multiple token files for their design system. And they have to manage and describe that “meta” layer that determines how everything fits together. By using JSON pointers ( Of course, there are also more complex usecases with modes, etc., that will be solved by the still-in-progress Resolver proposal (that I have seen a preview, and am a big fan of!). So if we accept JSON |
@drwpow walked me through the PR, and I grilled him on various aspects to ensure we weren't missing anything. I feel comfortable with this proposal, as it helps with adoption and tool creation (JSON ref libraries already exist for several languages). The only possible issue is that we'd no longer support "inline aliases" interpolated in a value. For example: |
Started playing around with an implementation in Terrazzo, and I realized there’s an error with the proposal that needs to be corrected. And it may affect some peoples’ opinions about the approach: 1.
|
Summary
This makes a breaking change to aliases, changing the syntax:
This provides a solution for #166, as well as aligns the DTCG format closer to common syntax that address similar problems.
Reasoning
DTCG users have kept their files separately for a long time, and have asked for the ability to reference tokens in other files (#166). Since we were already borrowing from JSON Schema in places (primarily the
$
character to mark reserved keys), this brings JSON Schema’s$ref
keyword to the concept of token aliasing.The
$ref
keyword (aka “JSON pointers”) comes both from JSON Schema and the OpenAPI specification (previously Swagger), and has been in use for over a decade. It is syntactically already consistent with the DTCG format, and functionally consistent as well. Most languages have lots of existing tooling to parse and resolve these (e.g. JavaScript).For the DTCG format, it is a way to keep all the functionality of aliases while extending it for future needs, borrowing from prior art where this syntax is known, used, and well-defined. For simplicity, this proposal aims to replace the previous alias syntax, rather than keep 2 conflicting ways to accomplish the same thing (which will be expanded on in points below).
What’s gained
What’s lost
Pros
This greatly expands what’s possible with the DTCG format, now that any part of the document (or remote documents, or partial remote documents) may be reused.
$extensions
if they wanted to, while selectively applying overridesWhile this is flagged as a breaking syntax change, it’s a backwards compatible functionality upgrade to aliases. There is no end-behavior nor features that should be lost with this change.
Figma Styles and Variables use the
/
character in names, so now token names will map 1:1This is backwards compatible for token names. Tokens can still use
/
and#
characters in their names; they just have to be escaped with~0
and~1
respectively ONLY INSIDE$ref
, according to the spec:For DTCG parsers, this simplifies parsing/handling of aliases. Consider the old syntax:
When a schema isn’t distinguishable by structure, tool makers have to do additional work string matching to do basic typecasting and discrimination.
Tool makers also benefit from over a decade of prior art and tooling for handling this syntax (even if they may not have used it directly before)
Cons
"{my.token.alias"}
) to an object ({ "$ref": "#/my/token/alias" }
). Again, though long-term it will be easier to work with, short-term will impose migration pains.$value
e.g.:$value
of one token points to another token’s$value
(which means aliasing the entire token object itself, or a group, means aliases are created)Alternatives
We could extend the current
{…}
alias syntax to support remote files. But I think realistically, it would follow the JSON Pointer spec underneath. But that would require extensive documentation to outline this minor detail, and would still leave toolmakers without the ability to use any of the myriad tools available for JSON pointers. If we’re adhering to those principles underneath, why not just use the syntax directly?Those familiar with JSON Schema will also be familiar with its counterpart
$defs
, which allows you to declare reusable parts of a document that can be$ref
’d anywhere (but by default are ignored/not parsed). $defs are NOT being proposed here, because it doesn’t solve an immediate problem, and introduces more complexity than necessary (but could be an additive followup)An alternate idea is to keep aliases the same
{color.blue.05}
, and introduce this new concept as a reference (and distinguish between the terms). Aliases would just be the “legacy” way to declare token aliases. I didn’t initially propose this because:There were no advantages I could find. Even the automatic
$type
inheritance is possible with$ref
, but$ref
carries more benefitsI couldn’t think of a sensible usecase of distinguishing “aliases” from “references” after reading this note in the JSON Schema spec (2019-09:
In other words, even the concept of
$ref
feels spiritally identical to the existing DTCG alias syntax—there are times when you do want to preserve and reference those values later, and they are significant in some ways (even including overrides).However, I think we could define some sort of deprecation strategy where the old syntax is supported for some time before switching over.
Notes
color.blue.05
orcolor/blue/05
?.
is allowed in token names again?2020-12
DOES allow$ref
to have sibling keys (“overrides”), which is IMPORTANT! Without this, I believe this would lose some functionality of aliases—specifically their ability to inherit$type
automatically.Edit: an earlier version referenced the upcoming Resolver Spec proposal, but I’ve realized the two aren’t related, and Resolver Spec isn’t a motivator for this. Mentions have been removed.