Skip to content

Create deprecating-components.md #2733

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.

Sign up for GitHub

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

Merged
broccolinisoup merged 9 commits into from
Feb 17, 2023
Merged

Create deprecating-components.md #2733

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
aac770b
Create deprecating-components.md
broccolinisoup Jan 3, 2023
5ee2013
Apply suggestions from code review
broccolinisoup Jan 4, 2023
a05470e
Merge branch 'main' into bs/deprecating-comp-docs
joshblack Jan 25, 2023
d857286
Merge branch 'main' into bs/deprecating-comp-docs
broccolinisoup Jan 27, 2023
be4d425
update the docs
broccolinisoup Jan 27, 2023
56b0650
Merge branch 'main' into bs/deprecating-comp-docs
broccolinisoup Jan 27, 2023
2811c88
Merge branch 'main' into bs/deprecating-comp-docs
broccolinisoup Jan 31, 2023
e97505e
rewording deprecation
broccolinisoup Jan 31, 2023
e6b6bb2
Merge branch 'main' into bs/deprecating-comp-docs
broccolinisoup Feb 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions contributor-docs/deprecating-components.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
As we work on the maturity of our components, we will sometimes need to build components in a parallel track/bundle without breaking existing components. Eventually, the new components would replace the old ones in the main bundle and this document explains the process around it.

## Developing a draft component

Start building the new component outside of the main bundle. This includes the source code of the component being under the `src/drafts` folder and the component being exported in the draft bundle (`src/drafts/index.ts`). That way, folks who
Copy link
Member Author

@broccolinisoup broccolinisoup Jan 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see some of the drafts component under the src/drafts and some not. Would that be true that we eventually want to have the practise to create the drafts components under the src/drafts?

Copy link
Member

@lesliecdubs lesliecdubs Jan 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@siddharthkp instead of recommending these components go into src/drafts, do you think we should be encouraging folks to build directly in the new experimental components repo instead? https://github.com/github/experimental-react-components

Copy link
Member Author

@broccolinisoup broccolinisoup Jan 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh super keen to hear about this 👀

Copy link
Member

@siddharthkp siddharthkp Jan 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes!

But given the change in direction for experimental-components, i'm not sure if it should be github/experimental-react-components or our own primer/react-experimental, another ADR? 😅

Copy link
Member

@lesliecdubs lesliecdubs Jan 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, probably. @justinbyo is going to be helping us figure out ownership models for shared React components looking forward.

In the meantime, are we comfortable shipping these docs with the direction to build direct in experimental-react-components for now, and we can always update if/when we make a change? We don't seem to deprecate components super often so I'm thinking that changing this guidance later wouldn't be a huge problem.

Copy link
Member

@lesliecdubs lesliecdubs Jan 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point @broccolinisoup. In most cases we should have similar standards for components (including unit tests and Storybook) in the experimental-react-components repo as we do in primer/react. I'm admittedly not as familiar with all of the items in the checklist and whether they would apply exactly the same in experimental-react-components; what do you think @siddharthkp?

Copy link
Member Author

@broccolinisoup broccolinisoup Jan 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most cases we should have similar standards for components (including unit tests and Storybook) in the experimental-react-components repo as we do in primer/react

I am a fan of this!

I wasn't sure how much we can "influence" experimental-react-components to follow our Primer component standards.
From my observation, experimental-react-components differ from PRC in the way of documenting components and they don't include some util unit test functions as PRC do.

Super keen to hear @siddharthkp's thoughts.

Copy link
Member

@siddharthkp siddharthkp Jan 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋 thanks for DMing me! i've lost the fight against notifications.

I wasn't sure how much we can "influence" experimental-react-components to follow our Primer component standards.

Oooh, we definitely should influence it a lot. We can even assist (and enforce) practices by baking them into the react toolchain.

However, we also keep the barrier for contribution pretty low. Experimental components are meant to be pre-alpha in the lifecycle, more context in ADR 008

From my observation, experimental-react-components differ from PRC in the way of documenting components and they don't include some util unit test functions as PRC do.

Some of this, like the docs, is intentional, we chose storybook docs over doctocat to make it easier (and less elaborate) to document than primer. The lack of test functions isn't part of the strategy, that's just because we've only had one iteration of this till now :)

Copy link
Member Author

@broccolinisoup broccolinisoup Jan 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oooh, we definitely should influence it a lot. We can even assist (and enforce) practices by baking them into the react toolchain.

Awesome to hear!

However, we also keep the barrier for contribution pretty low. Experimental components are meant to be pre-alpha in the lifecycle, more context in ADR 008

This is a pretty good point!

Some of this, like the docs, is intentional, we chose storybook docs over doctocat to make it easier (and less elaborate) to document than primer. The lack of test functions isn't part of the strategy, that's just because we've only had one iteration of this till now :)

Gotcha!

It feels to me that moving experimental components from react-experimental-components is a slightly different process than deprecating components, especially around the practises.

Another question - if an existing component was to be re-written by a Primer engineer (following all Primer standards), would we still start in the experimental-react-components repo or in our own repos?

cc @lesliecdubs

Copy link
Member Author

@broccolinisoup broccolinisoup Jan 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello! @siddharthkp and @lesliecdubs

Seems like we still have questions regarding to experimental-react-components repo's practises and how they will make their way to Primer. Would it be okay to go ahead with this section as is (creating the components in the drafts folder) until we have a clear practise for experimental-react-components? At least we will treat this document as the current practise and we can update as we go?

who would like to try the draft version of the component can export it from the draft bundle.

```
import { ActionList } from "@primer/react/drafts"
```

If it is a 1:1 replacement, it's useful to keep the component name the same for consumers. Internally, we would want to, for example, call the folder name `ActionList2` and call the docs `ActionMenu v2`.

## Deprecating the component

Once the component is ready to be graduated from the drafts bundle to the main one, we first plan to see in which major release the component will go into deprecation and prepare for the code changes and the communications around it.

### Code side of the deprecation

Here is a checklist for developers to smoothen the deprecation process.

#### Source Code

- [ ] Move the old component's source code under the `src/deprecated` folder.
- [ ] Remove the old component export in the main bundle to be added to the deprecated one.
- [ ] Export the old component in the deprecated bundle `src/deprecated/index.ts`.
- [ ] Rename the new component to its neutral version if it includes `2` or `v2`. I.e. `ActionList2` -> `ActionList`.
- [ ] Move the new component's source code to the `src` folder if it is under `src/drafts`.
- [ ] Export the new component in the main bundle `src/index.ts`.

#### Storybook

- [ ] Update the new component's storybook title from `Drafts/Components/<Component-Name>` to `Components/<Component-Name>`.
- [ ] Move the old component's stories under `src/stories/deprecated`.

#### Tests

- [ ] Move the old component's tests under `src/__tests__/deprecated`.
- [ ] Update `script/generate-e2e-tests.js` to reflect the new component's component ID is generated by the storybook title.
- [ ] Update the snapshots using the command `npm run test -- -u`.

#### Docs

- [ ] Move the deprecated component's docs to the `src/docs/deprecated` folder and update the title by adding `(legacy)`, status as well as the storybook and the source code link.
- [ ] Make sure to update `jsx live` -> `jsx live deprecated`
- [ ] Make sure to update the import code block. I.e. `import { ActionList } from "@primer/react/deprecated"`.
- [ ] Add a `Deprecation` section with the link of the newly developed component and provide a diff. See [deprecated ActionList docs](https://primer.style/react/deprecated/ActionList#deprecation) as an example.
- [ ] Move the new components docs from draft folder to the main docs folder `docs/content/` and update the title by removing `v2`, status as well as the storybook and the source code link.
- [ ] Make sure to update `jsx live drafts` -> `jsx live`
- [ ] Make sure to update the import code block. I.e. `import { ActionList } from "@primer/react"`.
- [ ] Update the navigation on `docs/src/@primer/gatsby-theme-doctocat/nav.yml` accordingly.

### Comms side of the deprecation

When we know if the newly developed component is ready to go in the main bundle and the old component is good to be deprecates in the next major release, we announce the deprecation and the promotion in the Primer changelog under the `Coming soon` section.

Once the major release is out, we provide support on how to switch using the newly develop component as well as how to continue to use the deprecated component for folks who prefer to migrate later on.

We also write codemods to deprecate the old component and use the new component under [our migration repo](https://github.com/primer/react-migrate#readme).
Copy link
Member Author

@broccolinisoup broccolinisoup Jan 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we provide any other support for teams except proving these codemods? Do they actually go and update the usages themselves or do we do it?

Also, is there any other practise we do or would like to introduce to encourage teams switching using the new components?


### Accessibility Scorecards

As teams begin to adopt the [Accessibility scorecard](https://github.com/github/engineering/discussions/2443), we have an opportunity to encourage teams to update/remove deprecated components and adopt new ones.

To get these items on the scorecard, we should be reaching out to #accessibility:

- When we deprecate a component
- When we release a new component that we want to enforce usage around

## Removing the deprecated component

After about 6 months of living in the deprecated bundle, a component is retired/deleted from the codebase. This is also a breaking change and we release it in the upcoming major release.
At this point, consumers are expected to plan migration work and we annouce the deletion in Primer Changelog as we do for the deprecation.

## FAQ

- Does the deprecated component accept new feature request?
- No, because we have a single bundle for all components, you can not pick the components you want to upgrade. This can result in additional unrelated work.
Copy link
Member

@joshblack joshblack Jan 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be helpful to highlight that, once deprecated, the component is likely in a "maintenance mode" and will only receive bug fixes. Maybe something like:

Suggested change
- No, because we have a single bundle for all components, you can not pick the components you want to upgrade. This can result in additional unrelated work.
- No, once a component is deprecated the only changes it will receive are bug fixes.

Copy link
Member

@lesliecdubs lesliecdubs Jan 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do y'all think: should this be will receive or may receive? Not sure how dedicated we've been to handling all bugfixes in deprecated components, or just major functional bugs 🤔

Copy link
Member Author

@broccolinisoup broccolinisoup Jan 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"may receive" sounds good to me. I like that it removes the certainty.

I updated to be "may receive" but please let me know if anyone has a concern 🙌🏻