opensearch-project / oui

OpenSearch UI Framework
Apache License 2.0
36 stars 69 forks source link

Deprecation Guideline Proposal #647

Open BSFishy opened 1 year ago

BSFishy commented 1 year ago

Right now, we don't have guidelines on deprecation in OUI. It is important that we establish them so that we have a consistent and streamlined approach in the future.

There are 4 main things we might be deprecating: components, component props, utilities, and icons. Here is my proposal for guidance on deprecating both.

These are just drafts on what this guidance should be, feel free to drop any questions, comments, or suggestions.

Components

606 establishes a phenomenal precedent on deprecating components. Namely, doing these things:

  1. Wrap the component in @opensearch-project/oui/utils/deprecated to log a message when a deprecated component is being used (helpful for consumers to know when they're using deprecated components)
  2. Add a doc comment on the component saying the component is deprecated (helpful for consumers to see an IDE deprecation message when writing code)
  3. Update the docsite page to reflect that the component has been deprecated

The first point helps consumers refactor code that has already been written using deprecated components and the second point helps prevent consumers from writing new code that uses deprecated components.

Component Props

This is new water, so it may go through a few iterations before we land on guidance that we stick with. I think there are a few steps that should be taken, then a few that we may or may not add to the guidance:

  1. Add a doc comment to the prop saying the prop is deprecated (helpful for consumers to see an IDE deprecation message when writing code)
  2. Reformat code to use both old and new prop, if there is a new replacement prop (helpful for consumers for compatibility)
  3. Add a console log message whenever the deprecated prop is used (helpful for consumers to know when they're using deprecated props)
  4. Add a section to the docsite (if one doesn't already exist) that tells users the prop is deprecated

Utilities

  1. Add a doc comment on the utility saying it is deprecated (helpful for consumers to see an IDE deprecation message when writing code)
  2. Update the docsite page to reflect that the utility has been deprecated
  3. Add a single call function (a function that sets itself to undefined once it has been called. Another utility could be created for this, like @opensearch-project/oui/utils/deprecated) that logs a message when the deprecated utility is used

Icons

Since icons are passed as a parameter, their deprecation needs to be similar to Utilities. This will need some additional set up because we don't want to have to do a bunch of work in the icon component for each icon we want to deprecate.

Ideally, we simply have a list of IconTypes that are deprecated. When an icon is created, a the component checks if the icon is in the list of deprecated icons. If it is, a log message is sent. Additionally, a docsite section could be dedicated to icons that are currently deprecated and pending removal. This way, whenever we want to deprecate an icon, we simply need to add it to the list of deprecated icons.

ashwin-pc commented 1 year ago
  1. Will adding the deprecated JSDoc annotation show up in the docsite? Since that is what users will refer to when they build using OUI?
  2. The same goes for components. In the case of #606 we dont want it to exist on the docsite for obvious reasons, but that will not be true in future. In future, even component deprecations need to show up in the docsite since it has to be an accurate reflection of the state of the library's public contract.
KrooshalUX commented 1 year ago

@ashwin-pc you are correct that in the ideal situation, we would deprecate something and have the deprecation notice also available on the docs site in sync with the deprecation notice within source code. 100% aligned, lets be sure that OUI docs site is called out explicitly in all of these guidelines. In regards to prop deprecations, I don't want to bury it in the props tab, I think its important enough to call out at the component level in addition to the props tab. This way no one has to go digging for information and its immediately clear to designers and builders what has changed.

Just to give some context as to why we didn't follow that in the case of OuiLoadingElastic and OuiLoadingKibana it was simply for compliance reasons and to get out ahead of a flood of "there are references to Kibana and Elastic on the OUI docs site" issues filed by viewers and to not block the go-live of the documentation.

KrooshalUX commented 1 year ago

Some additional thoughts about buckets outside of "component" and "component props":

BSFishy commented 1 year ago
  1. Will adding the deprecated JSDoc annotation show up in the docsite? Since that is what users will refer to when they build using OUI?
  2. The same goes for components. In the case of [CCI] Add deprecation comments to OuiLoadingElastic and OuiLoadingKibana #606 we dont want it to exist on the docsite for obvious reasons, but that will not be true in future. In future, even component deprecations need to show up in the docsite since it has to be an accurate reflection of the state of the library's public contract.
  1. The JSDoc won't show up on the docsite, added another point to update the docsite
  2. Same
BSFishy commented 1 year ago

Some additional thoughts about buckets outside of "component" and "component props":

  • Icons - we are about to deprecate a ton more of these after (Eng Research) Investiage the use of Logos & App Icons  #249, and we also already hid a number of icons from the docs site launch but have not yet "deprecated" the icons from a code perspective. This was also to achieve compliance (ex: removing the elastic logo from the page so it is not referenced by designers and engineers)
  • Themes?
  • Sass variables?
  • Utilities - do these count as "components" or are they treated differently?
ashwin-pc commented 1 year ago

@KrooshalUX yeah i understand why we had to do it this way this time, but was just calling out that thats the exception and not the norm :)

ashwin-pc commented 1 year ago

I would also like to know some of the options you considered for this proposal. Deprecation strategies for UI systems is an already solved problem. Many mature UI systems already do this all the time. Are we following the same pattern? If we arent, why are we deviating?

SergeyMyssak commented 1 year ago

I think this proposal is very well written. As an addition to this proposal, I assume that we need to create a separate page that describes the steps to migrate from one version to another. Here are examples of how this has been done in other popular ui libraries: 1) material-ui (85.9k stars) https://v5-0-6.mui.com/guides/migration-v4/#introduction 2) ant-design (85.5k stars) https://ant.design/docs/react/migration-v5 3) react-bootstrap (21.7k stars) - https://react-bootstrap.github.io/migrating/

It would be good to summarise all this and create action items to implement this deprecation guideline.