Skip to content

ADR: Parallel drafts track & plan for deprecated components #1722

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
merged 15 commits into from
Jun 20, 2022
Merged

ADR: Parallel drafts track & plan for deprecated components #1722

Changes from 3 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
2aeacf3
propose migration plan
siddharthkp Dec 13, 2021
3e7a92e
Update adr-005.md
siddharthkp Dec 13, 2021
287c236
Update adr-005.md
siddharthkp Dec 20, 2021
a5b65fa
Merge branch 'main' into siddharthkp/migration-adr
mperrotti Jan 10, 2022
10b76ca
move SelectMenu to the first list
siddharthkp Jan 12, 2022
7fe1927
Merge branch 'main' into siddharthkp/migration-adr
siddharthkp Jan 12, 2022
bbc285c
Update with parallel track talk
siddharthkp Jan 19, 2022
5d3eef8
Update adr-005.md
siddharthkp Jan 19, 2022
37419da
rename back to drafts
siddharthkp Jan 19, 2022
f71c2a8
Update contributor-docs/adrs/adr-005.md
siddharthkp Jan 20, 2022
8c5b1e4
Apply suggestions from code review
siddharthkp May 25, 2022
fdfff6b
Remove component list from ADR
siddharthkp May 25, 2022
14f420b
Merge branch 'main' into siddharthkp/migration-adr
siddharthkp May 30, 2022
f1396e2
Merge branch 'main' into siddharthkp/migration-adr
siddharthkp Jun 20, 2022
3c6fdca
Merge branch 'main' into siddharthkp/migration-adr
siddharthkp Jun 20, 2022
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
65 changes: 65 additions & 0 deletions contributor-docs/adrs/adr-005.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# ADR 5: Migration plan for deprecated components

## Status

Proposed

## Context

As we work on maturity of our components, we will sometimes need to deprecate components that are replaced with new components.

## Here are the 3 proposed stages:

### Stage 1

Start new component outside the main bundle. Folks who want to try it, need to explicitly import it from the `drafts` bundle.

`import { ActionMenu} from '@primer/react/drafts'`

The contract with consumers is - you are opting into a rewrite of the old component that might not cover all the cases of the old component yet. If you are using both the old and new version of the component in different pages, you are paying some additional bundlesize cost.

Note: If it is a 1:1 replacement, it's useful to keep the component name the same for consumers. Internally, we would want to call the filename `ActionMenu2.tsx` and call it `ActionMenu2 v2` in the docs.
Copy link
Contributor

@rezrah rezrah Jan 12, 2022

Choose a reason for hiding this comment

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

After stage 2, where the component gets moved to /deprecated, do we rename this back to ActionMenu? Even if it's a breaking change in major version, it feels cleaner to take v2 out of the naming.

Copy link
Member Author

@siddharthkp siddharthkp Jan 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Sooo, the import name for consumers is always ActionMenu but the import path would change.

- import { ActionMenu } from '@primer/react/lib-esm/drafts'
+ import { ActionMenu } from '@primer/react'

render(<ActionMenu>)

The filename internally and the docs would drop the v2 suffix


### Stage 2
Copy link
Contributor

@colebemis colebemis May 13, 2022

Choose a reason for hiding this comment

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

[Non-blocking] We may want to consider a @primer/react/next bundle where we put replacement components when we're "confident" in the API. This will allow consumers to migrate to the new components on their own schedule ahead of the breaking release described in Stage 2

Copy link
Member Author

@siddharthkp siddharthkp May 25, 2022

Choose a reason for hiding this comment

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

Is that separate from @primer/react/next? 😅

Copy link
Contributor

@colebemis colebemis Jun 1, 2022

Choose a reason for hiding this comment

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

Not sure I understand the question 😅


After we have the confidence that this component is better than the old version of it, we swap the components and move the old component out of the main bundle.

This is a breaking change! It’s recommended that you start using the new component, but if you’d like to push that effort to the future, we give you an easy way out -

`import { ActionMenu } from '@primer/react/deprecated'`

The deprecated component does not accept new features requests.

Reason: Because we have a single bundle for all components, you can not pick the components you want to upgrade. This can results in additional unrelated work.

### Stage 3

After 3 months of living in the `deprecated` bundle, a component is retired/deleted from the codebase.

At this point, consumers are expected to plan migration work.
Copy link
Contributor

@colebemis colebemis May 13, 2022

Choose a reason for hiding this comment

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

Some additional notes for this section:

  • The deprecated component should be marked as deprecated on the documentation page with information about how to migrate to the recommended component
  • The link to the deprecated component should be moved to the Deprecated section of the navigation
  • The title of the page should change to ComponentName (legacy) if we replaced it with a component of the same name

Copy link
Member Author

@siddharthkp siddharthkp May 25, 2022

Choose a reason for hiding this comment

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

There's a long list for "deprecating a component" 😅 Should we add that to the contribution docs instead of ADR?

Copy link
Contributor

@colebemis colebemis Jun 1, 2022

Choose a reason for hiding this comment

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

Should we add that to the contribution docs instead of ADR?

Yup, makes sense to me 👍


## Relevant components:
Copy link
Contributor

@colebemis colebemis May 13, 2022

Choose a reason for hiding this comment

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

Not sure if this section makes sense to include in an ADR


Deprecated in favor of Box, ready for stage 2:

1. BorderBox
2. Flex
3. Grid
4. Position

Replaced with newer components

5. Dialog - replaced with Dialog2, ready for stage 2
6. Dialog2 - ready for Stage 2 with rename, already in main bundle
7. Dropdown - replaced with DropdownMenu, ready for stage 2
8. SelectMenu - replaced with SelectPanel?, ready for stage 2
9. SelectPanel - unclear if it should be renamed, already in main bundle
Copy link
Member Author

@siddharthkp siddharthkp Dec 13, 2021

Choose a reason for hiding this comment

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

This is a TODO: Should we rename it? It's already in the main bundle, used in production and doesn't seem like a 1:1 successor to SelectMenu. Maybe, it doesn't belong to this list.

Copy link
Contributor

@mperrotti mperrotti Dec 16, 2021

Choose a reason for hiding this comment

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

SelectPanel seems different from SelectMenu. It's just unfortunate that the names are so similar.

Copy link
Contributor

@rezrah rezrah Jan 12, 2022

Choose a reason for hiding this comment

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

Shall we move these into an issue instead? I can imagine this changing very frequently so doesn't feel like it should be in an ADR?


Future deprecations, replaced with newer components

10. ActionList - replaced with ActionList2, need to plan for stage 2
11. ActionList v2 - in drafts bundle, need to plan for stage 2
12. ActionMenu - replaced with ActionList2, need to plan for stage 2
13. ActionList v2 - in drafts bundle, need to plan for stage 2
14. Button - replaced with NewButton, need to plan for stage 2
15. NewButton - in drafts bundle, need to plan for stage 2 with rename