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

$deprecated property? #118

Open romainmenke opened 2 years ago

romainmenke commented 2 years ago

When a specific design token should no longer be used the current schema only gives the option to remove it.

This can easily block consumption of future changes until all uses have caught up with the removal.

This is a communication issue. Design token files will eventually have breaking content changes.

This can be partly handled by using a distribution method that supports some form of versioning. That would allow consumers to stay on a version they are compatible with until they are ready to update. This is outside the scope of my request.

Having a $deprecated field would allow tools to give warnings and guidance about upcoming changes.

{
  "Button background": {
    "$value": "#777777",
    "$description": "The background color for buttons in their normal state.",
    "$deprecated": "Button styles have been moved to a separate design tokens file. Import 'buttons.tokens'"
  }
}
CITguy commented 2 years ago

Why not a $status field instead?

I can see this being useful to describe a token throughout the entirety of its lifespan (not just during deprecation).

Could start with just a few supported values...

crude, partial JSON Schema doc to illustrate my idea...

{
  "$schema": "http://json-schema.org/draft-07/schema",
  "$id": "token",
  "type": "object",
  "properties": {
    "$value": {},
    "$type": { "type": "string" },
    "$extensions": { "type": "object" },
    "$state": {
      "type": "string",
      "default": "stable",
      "enum": [
        "experimental",
        "stable",
        "deprecated"
      ]
    }
  },
  "required": ["$value"]
}
romainmenke commented 2 years ago

I like this! But it will only work as long as no two statuses need to be assigned at the same time.

Something can be both deprecated and experimental.

danieldelcore commented 2 years ago

I think having a $state property will be really beneficial to the spec as well.

In our implementation, we have a state property that can take one of the following states:

Experimental → Active → Deprecated → Sunset → Deleted

Our tooling leverages this along with a replacement property to perform automatic or controlled migrations via tooling such as eslint, stylelint, figma etc.

For example, a deprecated token with a replacement can be warned against and autofixed by eslint. A sunset token will produce an error and can be autofixed and so on.

{
  link: {
    pressed: {
      attributes: {
        group: 'color',
        state: 'deleted',
        replacement: 'color.link.pressed',
        description: 'Use for links in a pressed state',
      },
  },
}

We've had to build these tools ourselves, but the great thing about having an industry-wide spec like this one is that both community and proprietary tooling can leverage this concept to make token lifecycles a standard and consistent behaviour. Particularly helpful for the maintenance and evolution of tokens over time.

ajsecord commented 2 years ago

For communication in long-running projects, deprecated is so useful as to be required, I think. It gives the authors the flexibility to change things without breaking all downstream users immediately.

We also use a replacement-like field both for concise communication (in addition to the deprecation message) and tooling support.

Re: encoding something like Experimental → Active → Deprecated → Sunset → Deleted in the spec: this is brittle because different teams have (sometimes very) different ideas of what the right life cycle states should be. If we go this way, the states will need to be configurable and not fixed.

romainmenke commented 2 years ago

Re: encoding something like Experimental → Active → Deprecated → Sunset → Deleted in the spec: this is brittle because different teams have (sometimes very) different ideas of what the right life cycle states should be. If we go this way, the states will need to be configurable and not fixed.

I agree. But this this is only problematic if you can only have one state at a time. If state is just a grouping of properties and deprecated is part of that I think it can work for everyone.

Standardising around a set states will be required as this in turn makes it possible for tools to hook into these.

Some of these might be problematic for the standard :

Ideally these states are unambiguous so that it's easy for everyone to agree on what a state means and how it should be applied.

danieldelcore commented 2 years ago

Those are correct observations. Just to clarify deleted doesn't actually exist. It marks when a token is physically removed.

Also, sunset is just deprecated but louder. The difference is how our tools react, for our specific implementation and needs deprecated results in a warning and sunset an error. It means for us that when we finally move to delete the token most of our consumers should have already moved away from sunset tokens.

Not proposing this be adopted verbatim, sharing as an example 👍

WanaByte commented 2 years ago

To me, deprecation seems like a fairly standard part of the software lifecycle. I would suggest a $deprecated object rather than a string.

With an object, $deprecated could then resemble other objects in the spec.

For example

{
  "Button background": {
    "$type": "...",
    "$value": "...",
    "$deprecated": {
      "$description": "Very important deprecation message",
      "$extensions": {
          "com.my-co.lifecycle": "sunset",
    }
  }
}

The replacement mentioned by @danieldelcore could be an extension, or could be part of the deprecated object itself.

romainmenke commented 2 years ago

With an object, $deprecated could then resemble other objects in the spec.

This I do not really follow :) Why would com.my-co.lifecycle have the need to set sunset?

This would just be a vanity alias in this case if the spec only allows $deprecated property per token.


A $description is interesting but I think it might be easier to implement in tools if $description is used (for now) to write extra information like this.

I do agree that an object might be better than a simple flag as this allows for future additions.

{
  "Button background": {
    "$type": "...",
    "$value": "...",
    "$description": "Bright pink background for buttons.\nThis has been deprecated.\nSee : <some link> for more info."
    "$deprecated": {
      "$value": true,
      "$future-thing": "..."
    }
  }
}

For now I think it would be best to keep this a bit more simple and keep $deprecated or a more general $state as flags.

Any extra API will also need corresponding UI in design tools so that people can enter data and will also need to be surfaced by all tools that consume tokens.

WanaByte commented 2 years ago

With an object, $deprecated could then resemble other objects in the spec.

This I do not really follow :) Why would com.my-co.lifecycle have the need to set sunset?

The argument is for $deprecated to be an object with an $extensions property. com.my-co.lifecycle is an example only.

The specifics within $extensions are a response to your comment where $deprecated could take a $status and to @danieldelcore's comment using states Experimental → Active → Deprecated → Sunset → Deleted.

A $description is interesting but I think it might be easier to implement in tools if $description is used (for now) to write extra information like this.

{
  "Button background": {
    "$type": "...",
    "$value": "...",
    "$description": "Bright pink background for buttons.\nThis has been deprecated.\nSee : <some link> for more info."
    "$deprecated": {
      "$value": true,
      "$future-thing": "..."
    }
  }
}

A description and a deprecation notice are separate concepts, IMO. Consider @deprecated in Javadoc, @available attribute in Swift, the [[deprecated]] attribute in C++`, etc.

For those examples, the deprecation message is separate from the implementation. IMO, a separate deprecation message lends toward a better version control history.

romainmenke commented 2 years ago

A description and a deprecation notice are separate concepts, IMO. Consider [@deprecated in Javadoc]

True. Instead of a boolean flag, $deprecated could just use a string value? If it's there, the token is deprecated and the string contents is the deprecation message.


So to summarise we have expanded from a need to set a deprecated boolean flag to these concerns/requirements :

WanaByte commented 2 years ago

Instead of a boolean flag, $deprecated could just use a string value?

The reason for $deprecated to be an object is for forward compatibility and for machine readability.

One possibility to allow simplicity is for $deprecated to be true or be an object. Angular's config files work this way. I do not like this approach, but prior art does exist. It is simple to parse in Javascript, but much harder in languages with strong, static types.

If $deprecated is an object, it can be evolved with the spec. If $deprecated is a string, there is no way to evolve the field. We'd either have to add more fields, for example, $deprecated-seeOther, or re-work deprecation into an object.

CITguy commented 2 years ago

Having $deprecated be an object makes sense, but I'd argue that a $value prop seems both redundant and conflicts with the definition of a Token. If $deprecated is present, it's assumed to be truthy, so there's no need for a $value that would always be set to true. Tokens are defined as JSON objects containing a $value property, so it'd be confusing if seen within deprecation metadata.

Having a deprecation message in isolation of the token description also makes sense, but I wonder if using $description would overload the term. I'd be more inclined to use $message, but this is just my personal opinion.

Lastly, I agree that there should be some degree of flexibility in allowing additional metadata for various use cases (e.g., documenting a potential replacement). However, $extensions within $deprecated seems overloaded. The way I interpreted the spec in regards to extension metadata seems like it's focused more on providing information on how to transform a token value, rather than how to document token metadata. $deprecated provides documentation metadata, not transformation metadata (regardless of its presence, it won't affect the transformed value of a token).

If the problem is the need to isolate transformation metadata (i.e., extensions) from documentation metadata, why not be explicit about it and have a $docs or $info Token property as a sibling to $extensions?

Example

{
  "gray": {
    "$value": "#808080",
    "$description": "Neutral 50% Gray"
  },
  "grey": {
    "$value": "{gray}",
    "$description": "An optional description of the token, 'grey'.",

    // Transformation metadata (free-form props)
    // This data is used specifically to configure how to transform a token's value.
    "$extensions": {
      "com.foo.bar": {
        // if language supports it, define by reference, not by value
        //  (Sass) $grey: $gray;
        //  (JS) const grey = gray;
        //  etc.
        "retainAlias": true
      }
    },

    // Documentation metadata (free-form props)
    // This data has no affect on transformed value.
    "$info": {
      // Might be preferred, to keep all docs-specific data organized within a single object.
      // Overrides #/grey/$description
      "description": "Optional description of token 'grey'.",

      /* Deprecation Examples */
      // FORMAT 1: boolean
      // (Answers nothing other than "Is it deprecated?")
      "deprecated": true,
      // FORMAT 2: string
      // (Extends format 1, by providing a human-readable message about the deprecation)
      "deprecated": "'grey' has been deprecated.",
      // FORMAT 3: object
      // (Further extends format 2 by providing a "next of kin" replacement.)
      "deprecated": {
        "message": "'grey' has been deprecated.",
        // An alias is necessary in order to document the resolved, token name based on the transformation language.
        "replacement": "{gray}"
      }
    }
  }
}

Alternatively, you could just isolate transform vs documentation metadata within #/grey/$extensions.com.foo.bar...

{
  "gray": {
    "$value": "#808080",
    "$description": "Neutral 50% Gray"
  },
  "grey": {
    "$value": "{gray}",
    "$description": "An optional description of the token, 'grey'.",

    // Transformation metadata (free-form props)
    // This data is used specifically to configure how to transform a token's value.
    "$extensions": {
      "com.foo.bar": {
        "transform": {
          // if language supports it, define by reference, not by value
          //  (Sass) $grey: $gray;
          //  (JS) const grey = gray;
          //  etc.
          "retainAlias": true
        },
        "docs": {
          // Might be preferred, to keep all docs-specific data organized within a single object.
          // Overrides #/grey/$description
          "description": "Optional description of token 'grey'.",

          /* Deprecation Examples */
          // FORMAT 1: boolean
          // (Answers nothing other than "Is it deprecated?")
          "deprecated": true,
          // FORMAT 2: string
          // (Extends format 1, by providing a human-readable message about the deprecation)
          "deprecated": "'grey' has been deprecated.",
          // FORMAT 3: object
          // (Further extends format 2 by providing a "next of kin" replacement.)
          "deprecated": {
            "message": "'grey' has been deprecated.",
            // An alias is necessary in order to document the resolved, token name based on the transformation language.
            "replacement": "{gray}"
          }
        }
      }
    }
  }
}
hereinthehive commented 2 years ago

This is something I'd been thinking about before joining. A state/status object feels extensible and more descriptive. I'd also argue that the deleted state is valid as you might want to capture why a value was deleted.

With concepts like this, perhaps we're touching on whether tokens should have their own version of an ADR so there's an audit/history around choices/decisions made?

{
  "colors": {
    "action": {
      "$value": "#ff0000",
      "$type": "color",
      "$status": [{
        "$state": "deprecated",
        "$description": "We'll be replacing this soon, please check out how the replacement value works for you",
        "$value": "#00ff00",
        "$timestamp": "20220707111111",
        "$note": "We changed the color based on user research",
        "$toolstamp": "Figma",
        "$author": {
          "$name": "Dan Donald",
          "$email": "dan@zeroheight.com"
        }
      },
      {
        "$state": "created",
        "$timestamp": "20220128093348",
        "$author": {
          "$name": "Dan Donald",
          "$email": "dan@zeroheight.com"
        }
      }]
    }
  }
}

The rationale here is that as tokens when used to their full potential are a real business asset, it's useful to know when and why a decision was made. Seeing a simple history allows us to have context for a value but also allow any given tool to write to it and have that shared understanding in a way we haven't to date. Arguably the notion of 'toolstamp' is a naive way of suggesting there should be a reference to what tool was used as part of the audit.

Really curious to hear your thoughts!

I think having a $state property will be really beneficial to the spec as well.

In our implementation, we have a state property that can take one of the following states:

Experimental → Active → Deprecated → Sunset → Deleted

  • Experimental: For introducing tokens for internal validation and trail purposes
  • Active: This token is currently actively being used. Tooling will help to ensure tokens are used.
  • Deprecated: This token has been flagged for future deletion and should no longer be used.
  • Sunset: This token has been marked for upcoming deletion and strictly should not be used.
  • Deleted (hard): This token has been fully removed from the tokens API

Our tooling leverages this along with a replacement property to perform automatic or controlled migrations via tooling such as eslint, stylelint, figma etc.

For example, a deprecated token with a replacement can be warned against and autofixed by eslint. A sunset token will produce an error and can be autofixed and so on.

{
  link: {
    pressed: {
      attributes: {
        group: 'color',
        state: 'deleted',
        replacement: 'color.link.pressed',
        description: 'Use for links in a pressed state',
      },
  },
}

We've had to build these tools ourselves, but the great thing about having an industry-wide spec like this one is that both community and proprietary tooling can leverage this concept to make token lifecycles a standard and consistent behaviour. Particularly helpful for the maintenance and evolution of tokens over time.

romainmenke commented 2 years ago

it's useful to know when and why a decision was made. Seeing a simple history allows us to have context for a value but also allow any given tool to write to it and have that shared understanding in a way we haven't to date.

I see a connection with this issue : https://github.com/design-tokens/community-group/issues/157

This change log could include changes made by tools. If a tool must change a value to make it processable, it could record this.


At the same time I wonder if tracking changes should be done externally? git exists and a service like GitHub is really successful in providing a nicer UX to expose a git history for source code.

Someone can make a service to provide something similar for design tokens.


If a change log should be part of the specification I still think it should be separate from the deprecated/state property.

A change log should encompass all aspects of a token (including deprecated/state). @hereinthehive Maybe you can open a separate issue for this concept?

hereinthehive commented 2 years ago

Sure, happy to!

I guess the issue is that as you can write the proposed format by hand, as well as using any number of tools Git may or may not be a part of the workflow or only have a partial view of the history.

I'll split it out and see what people think

CITguy commented 2 years ago

Given that there are much better and more mature tools to accomplish changelog / history / audit trails, I'd highly recommend against trying to wedge it into the spec. Additionally, that information provides no useful data for transforming the token itself and only adds bloat to the JSON.

lukasoppermann commented 3 months ago

What is the current state of this idea?

I do think at least having a $deprecated: true property or $state: "deprecated" or however it can be encoded is very useful. For example, to use it to compile a json file with deprecated tokens only which can be used in linting.

Style dictionary currently encourages the use of deprecated and deprecated_comment. Another aspect we find important is to have a replacement token (if applicable).

I think the idea of @CITguy is pretty solid:

      "deprecated": true,
      // FORMAT 2: string
      // (Extends format 1, by providing a human-readable message about the deprecation)
      "deprecated": "'grey' has been deprecated.",
      // FORMAT 3: object
      // (Further extends format 2 by providing a "next of kin" replacement.)
      "deprecated": {
        "message": "'grey' has been deprecated.",
        // An alias is necessary in order to document the resolved, token name based on the transformation language.
        "replacement": "{gray}"
      }

This at least covers all cases that I have come across.

drwpow commented 3 months ago

What is the current state of this idea?

I’m actually going through this issue, and a few others, where there’s been demonstrated interest and it’s not too controversial of a topic (not like color and modes/transforms at least) and creating proposals to make final decisions on. I’m working on some minor changes to $type: dimension and $type: shadow this week. After those, this one is on my list to summarize into a proposal (will get to it within the month). Look forward to a linked PR that will welcome more feedback/thoughts 🙂

lukasoppermann commented 1 month ago

Look forward to a linked PR that will welcome more feedback/thoughts 🙂

@drwpow any news in this?

drwpow commented 1 month ago

Look forward to a linked PR that will welcome more feedback/thoughts 🙂

@drwpow any news in this?

Thanks for the ping. We ended up postponing a few meetings, but are picking back up this week. Will put together a PR for this. What happens when you promise a date 😅

drwpow commented 1 month ago

A proposal has been added here: https://github.com/design-tokens/community-group/pull/255. Would love thoughts / feedback!

And yes this is only for the $deprecated property, NOT the Status Object offshoot.