-
Notifications
You must be signed in to change notification settings - Fork 520
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.
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
ADR: Parallel drafts track & plan for deprecated components #1722
Changes from 8 commits
2aeacf3
3e7a92e
287c236
a5b65fa
10b76ca
7fe1927
bbc285c
5d3eef8
37419da
f71c2a8
8c5b1e4
fdfff6b
14f420b
f1396e2
3c6fdca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
# ADR 5: Parallel experimental track & plan for deprecated components | ||
|
||
## Status | ||
|
||
Proposed | ||
|
||
## Context | ||
|
||
As we work on 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. | ||
|
||
## 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 `experimental` bundle. | ||
|
||
`import { ActionMenu} from '@primer/react/experimental'` | ||
|
||
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. | ||
siddharthkp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### Stage 2 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Non-blocking] We may want to consider a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is that separate from There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
siddharthkp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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 - | ||
siddharthkp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
`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. | ||
siddharthkp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### Stage 3 | ||
|
||
After 3 months of living in the `deprecated` bundle, a component is retired/deleted from the codebase. | ||
siddharthkp marked this conversation as resolved.
Show resolved
Hide resolved
siddharthkp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
At this point, consumers are expected to plan migration work. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some additional notes for this section:
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yup, makes sense to me 👍 |
||
|
||
## Suggested changes: | ||
|
||
We should detangle "experimental" from component lifecycle. | ||
|
||
1. Rename `Experimental` to `Candidate` in [Component maturity lifecycle](https://primer.style/contribute/component-lifecycle): Candidate, Alpha, Beta and Stable. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough, makes a lot of a sense. I guess we can keep |
||
2. Rename `drafts` to `experimental`. A component can have `beta` maturity in the `experimental` bundle. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Major ➕ |
||
3. Experimental components should not be collocated with "main bundle" components in the documentation or status page. They should have their own section, as experimental components are not recommended as an alternative yet. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense, but it makes discovery and IA much more challenging. I think it's a wider discussion, so happy to go with this for now. This may make more sense if we can create a dedicated Labs section in our docs. |
||
|
||
## Relevant components: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
5. SelectMenu | ||
|
||
Replaced with newer components | ||
|
||
6. Dialog - replaced with Dialog2, ready for stage 2 | ||
7. Dialog2 - ready for Stage 2 with rename, already in main bundle | ||
8. Dropdown - replaced with DropdownMenu, ready for stage 2 | ||
9. SelectPanel - unclear if it should be renamed, already in main bundle | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
siddharthkp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
11. ActionList v2 - in experimental bundle, need to plan for stage 2 | ||
12. ActionMenu - replaced with ActionList2, need to plan for stage 2 | ||
13. ActionList v2 - in experimental bundle, need to plan for stage 2 | ||
14. Button - replaced with Button v2, need to plan for stage 2 | ||
15. Button v2 - in experimental bundle, need to plan for stage 2 |
There was a problem hiding this comment.
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 toActionMenu
? Even if it's a breaking change in major version, it feels cleaner to takev2
out of the naming.There was a problem hiding this comment.
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.The filename internally and the docs would drop the
v2
suffix