Handling deprecation in Kaizen

[ActuallyACat]

Problem

The need to deprecate things is a side effect of a healthy design system. As we learn more about how our components are used we may want to make improvements, or move in another direction and remove the component from the system. This issue covers the latter case.

More specifically:

• How do we indicate a prop is deprecated?
• How do we indicate a component is deprecated?
• How do we indicate a Sass mixins/variables are deprecated?
• How do we show that a component is deprecated in Storybook? At what point do we remove the story?

Extra discussion:

• When should the deprecated thing be removed from the system?

[ActuallyACat]

Here's my 2c:

How do we deprecate props?

Deprecating a property of the type/interface

The interface for tabs previously contained the property direction (with the String Literal Type values of "row" | "column" | undefined) which controlled whether the tabs would render horizontally or vertically. We decided that a better name would instead be orientation with the values ("horizontal" | "vertical" | undefined).

interface Props {
  /**
   * Renders the tab in a column or a row
   * @deprecated Use orientation property instead
   */ 
  direction?: "column" | "row"
  /**
   * Renders the tab in a horizontal or vertical orientation
   */
  orientation?: "horizontal" | "vertical"
}

Deprecating some options of a property

Not too sure about this one. We could potentially still use the @deprecated annotation, but specify which options are no longer supported?

interface Button { 
  /** 
   * Describes the appearance of a button
   * @deprecated The "yuzu" type is no longer supported
   */
  variant: "default" | "primary" | "yuzu"
}

How do we deprecate components?

We can also use the @deprecated annotation for components too:

/** 
 * @deprecated Use NewButton instead
 */ 
export const Button = () => {}

Depending on how yell-y we want to be, also pop a console.warn when the component is mounted.

How do we deprecate Sass mixins and variables?

mixins

Use warn or Sass Deprecate

@mixin some-example {
   @warn "some-example is deprecated, use component instead"
}

Variables

I haven't tried this, but Sass Deprecate might work for variables?

How do we show that a component is deprecated in Storybook?

Add (deprecated) to the story title so that it is easily searchable. Here's an example.

At what point do we remove the deprecated component/prop/mixin?

¯_(ツ)_/¯

[sebpearce]

These are great 👍

I looked for whether there are plans to add JSDoc hints to TS for deprecated union values but can't see anything like that yet :(

One question I have is: at what point do we remove a deprecated component from Storybook?

It's theoretically possible that a particular repo might be a straggler and still be using Deprecated Component X in 5 years' time – but if we were to keep its story in Storybook for 5 years, it'd be a mess of deprecated components.

Maybe there's an argument to be made for removing the story, say, 3 months after the component's been marked as deprecated. That might lead to discoveries like "why isn't that component I want to use in Storybook?" which is an opportunity to spread awareness.

[ActuallyACat]

at what point do we remove a deprecated component from Storybook?

This could get messy really quickly. I'm happy to remove the stories a little sooner (say, within 2 weeks of it becoming deprecated) to build up a bigger barrier of using deprecated components. I wonder if this will frustrate engineers though.

[mbylstra]

Depending on how yell-y we want to be, also pop a console.warn when the component is mounted.

I think this is a good idea (as long as it doesn't generated hundreds of warnings per page). To me the most important thing is to know when a component is deprecated so they don't keep using it.

I'd say that a common way people use Kaizen components is by copying existing examples of usages. If the existing examples use a deprecated component, they won't know, and it will continue to spread. People will often copy an example, and if it works, they are done - no need to look at storybook or read the Kaizen source code, so I think console warnings might be the only way they can find out. A good example is the legacy Text component. The @deprecated tag is great (I haven't tried, but I assume people will see a warning in their text editor, which is ideal), but unfortunately there's still a majority of code out there that isn't linted through typescript.

[lijuenc]

I also think using console.warns where possible is a good idea.

I'm not sure where the text from @deprecated would show up. From the looks of microsoft/TypeScript#38523, it's a visual-only indicator in the editor. Is this editor dependant or does the TS compiler actually error or warn when it comes across a usage of one of these?

If it doesn't, I think we would need to have something that applied regardless of which editor someone used. With some help from TypeScript and generics I think we could create a set of helpers that could be applied to components, props, or specific values of props.

Decorators would probably be more pure for the purpose of annotating things but that's a class-only thing.

[ActuallyACat]

Yeah, that's my understanding of @deprecated. It will be an editor hint and won't result in a Typescript error or warning or anything like that. Definitely a little limiting...

Spitballing - we could also write an Eslint rule that checks for deprecated components, and spit out a warning. All the repos using frontend-ops would receive the rule from that, so it could be a straightforward way to manage them.

[dmisdm]

Just a thought, maybe it's bikeshedding, we could also use the link/see tags in JSDocs to link to the alternative in a bit of a nicer way.
https://jsdoc.app/tags-see.html

[dmisdm]

Also an idea, maybe we can advise the use of this eslint plugin?
https://github.com/gund/eslint-plugin-deprecation

[mbylstra]

I haven't tried this, but Sass Deprecate might work for variables?

I looked into it, but Sass Deprecate is just some helpers on top of scss's built in @deprecate mixins, so it can't support variables. So, I don't think there is any way to add deprecation warnings for variables.

The main thing it does extra is conditionally show the warning depending on whether there has been a breaking/major release or not. I don't know if this is useful to us, because I don't think we treat breaking changes that same way they do (a major release wouldn't necessarily mean that all deprecated features have been removed).

[lloydstubber]

Going to leave this discussion open (and remove the tag) as it's quite large and don't want to split this content. @ActuallyACat thoughts on this?

[github-actions]

This issue hasn't seen activity in two months! Has it been fixed, decided not to be worked on or needs more information? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in a fortnight.

[github-actions]

This issue was closed due to 2 months of inactivity. Feel free to reopen it if still relevant.