-
Notifications
You must be signed in to change notification settings - Fork 616
ADR 010: Merge drafts status into experimental #2856
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
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
# ADR 010: Merge drafts status into experimental | ||
|
||
## Status | ||
|
||
Draft | ||
|
||
## Context | ||
|
||
`Drafts` were introduced in [ADR-006](./adrs/adr-006-drafts.md) as a way of creating components outside of the root bundle of `@primer/react`. This helps us create, test and maintain multiple versions of the same component in parallel. | ||
|
||
Example: | ||
|
||
```js | ||
// default version of component exported from root of package | ||
import {UnderlineNav} from '@primer/react' | ||
|
||
// new version of component exported from /drafts | ||
import {UnderlineNav} from '@primer/react/drafts' | ||
``` | ||
|
||
We have also used this method for components that are still a work in progress and we don't want developers to start using them in production without collaborating with us first. | ||
|
||
Example: | ||
|
||
```js | ||
// this component only exists in drafts and not in the roof of the package | ||
import {TreeView} from '@primer/react/drafts' | ||
``` | ||
|
||
### Why do we not reuse `experimental` status? | ||
|
||
The reason we do not use `experimental` label for these components is because `drafts` conveys a different message than maturity on the [component lifecycle](https://primer.style/contribute/component-lifecycle). | ||
|
||
A `draft` component can be further along on the component lifecycle than `experimental`, but still not be the recommended version of the component until the next major release (to plan breaking changes better). For example, at the time of writing, the [new UnderlineNav2 in drafts](https://primer.style/react/drafts/UnderlineNav2) has been reviewed for accessibility and is almost at `Beta` maturity. We can call it a `draft` component with `beta` maturity. | ||
|
||
### Problem | ||
|
||
This is very confusing. While the above separation is useful for maintainers of primer, it is less useful to consumers. | ||
|
||
1. Consumers of primer are often confused to not find `draft` maturity on the component lifecycle. | ||
2. It is similar to `experimental` label in the most important characteristic to consumers - The component is not ready for production usage. | ||
|
||
### Decision | ||
|
||
We should drop the `draft` label and use `experimental` instead. | ||
|
||
While not completely accurate (UnderlineNav2 is much more polished than `experimental` maturity), the lack of confusion makes it a good trade-off. | ||
|
||
Fine-grained information about component maturity, useful for maintainers, is also communicated by the [status checklist in the component documentation](https://primer.style/react/drafts/UnderlineNav2#status). | ||
|
||
### Side effects | ||
|
||
We should create a new parallel import path for `@primer/react/experimental`. | ||
|
||
```diff | ||
// new version of component exported from /experimental | ||
- import { UnderlineNav } from '@primer/react/drafts' | ||
+ import { UnderlineNav } from '@primer/react/experimental' | ||
``` | ||
|
||
We would need to keep `@primer/react/drafts` in the short term as well for backwards compatibility. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
As an alternative, would love to explore other ways of indicating API stability (like through an
unstable_*
prefix through the main bundle). It would be great to limit the number of entrypoints that we ship, especially if the intent is to have them move to the main entrypoint over time.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.
I am loving this proposal! 💖 Even as a maintainer of PRC, it took me a while to wrap my head around this ~ not sure how the experience was for consumer both Hubbers and non Hubber consumers.