react
react copied to clipboard
(Paused) ADR 013: Location of experimental react components maintained by Primer
⚠️ No Changeset found
Latest commit: 8be9f4699c03a32dccb2db6c4dcdc7ae1d74067e
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
size-limit report 📦
Path | Size |
---|---|
packages/react/dist/browser.esm.js | 113.71 KB (0%) |
packages/react/dist/browser.umd.js | 114.38 KB (0%) |
First of all, thanks @joshblack! These are great questions!
Feedback Consider a naming scheme like @primer/react-{component}-{stage} versus @primer/react-{stage}-{component} @primer/react (current) @primer/react-{component}-{stage} (proposed)
Yep, that would work well too. Give me your opinions on why that sounds nicer to you?
I like when names read like english "primer/react experimental page header" = @primer/react-experimental-pageheader
, but I do see the benefit of putting the component name first instead of burying it till the end 😅
Answers
- Could you detail the steps of moving a component from experimental to being in stable? How do teams move from an experimental package to a stable package?
For primer/react, moving the component directory into primer/react + moving the doc page out of experimental section
For application teams, ideally all the breaking changes are covered in the experimental package, so it would just mean updating the imports to @primer/react
. When all teams have moved to stable, we can remove the npm package from package.json
- Are we recommending that
@primer/*-experimental
packages are installed in specific workspaces and avoid installing them in theprimer
workspace on dotcom?
Is it possible to have different versions of an experimental package used in different workspaces today? If yes, that could be really helpful in adopting breaking changes (tradeoff: tiny amount of duplication)
- If they are installed in the
primer
workspace like other dependencies, I would imagine we'd run into the same problem identified where a single update breaks multiple workspaces
Yep! That's true. However, it would be an explicit major upgrade with a smaller surface area than primer/react. Workspace-specific upgrades would be perfect, but I imagine some components might end up in primer workspace if multiple teams are using it.
- Will experimental packages be released on a different schedule to
@primer/react
or will they keep the same schedule? (or publish as-needed?)
For the most part, published on an independent schedule whenever needed. But, my guess is that it wouldn't be uncommon for an experimental component to depend on changes in stable primer/react, so we might have to release both together in that case
- Is there an impact on the new primer.style architecture for docs information?
This one's tricky. We have draft + deprecated as well components that need to be housed in the new information architecture. @colebemis, what are your thoughts here?
- What conventions would there be for these components in storybook?
I think we will continue what we do right now with "drafts", which get a section in the sidebar. We would rename it to experimental 😅
- Where would common helpers live that need to be shared between both
@primer/react
and experimental packages?
I had some time to think about this in experimental-react-components.
All experimental components would build on stable components and utils. So, if a helper is already in primer/react, experimental component can import it.
Sharing between experimental components is different though. It's tempting to abstract helpers/utils into primer/react early, but I think we should resist that. If we don't have a stable usage of a helper, we should not upstream it to primer/react yet. Instead, we should duplicate it for the short term.
The hardest bit is when you want to use an experimental package in primer/react (for example, use an experimental implementation of PageHeader or Stack in PageLayout or slots in ActionList). When, both primer/react and dotcom include an experimental component in their dependencies, it's easy to end up with 2 major versions of an experimental package. This should be okay as long as they don't conflict (and themeprovider doesn't get confused) [adding this to ADR under risks]
Thanks for putting this together, @siddharthkp!
In order to help move a decision forward, I'm assigning the following:
ADR decision maker: @colebemis Decision deadline: EOD Friday, 7 April or sooner
Your role as decision maker is to gather feedback, probe for discussion and disagreement, and then make an informed decision based on the feedback, identified risks, and trade-offs about whether to adopt the ADR or not. Your role is not to decide based on your own personally preferred approach.
Keep in mind that ADR decisions can be reversed or changed in the future if new information is exposed that make the ADR worth revisiting.
You can fulfill your decision maker role by doing the following:
- review the ADR, comments, and feedback closely
- seek additional feedback from any necessary parties, as needed; for example, request a review from any individuals or teams that you know would be unduly affected by this ADR, or who have extensive experience in using either the preferred or non-preferred approach and may have perspective to share about it
- follow up on any remaining concerns, questions, or unexamined risks around accepting the proposed approach
- on or before the deadline, make a decision as to whether the ADR is accepted ✅ , which you should signal by approving this PR, or rejected 🚫 , which you should signal by closing this PR with a comment explaining the reason(s)
@joshblack
Is there an impact on the new primer.style architecture for docs information?
I imagine we'd mark these components as "experimental" in the React documentation page, but their experimental status won't affect the top-level navigation.
@lesliecdubs Should we create an issue to track the implementation of this ADR?
@colebemis yes please! Let's add that issue to the "Inbox" on the Primer Roadmap (GitHub staff only) to be prioritized, ideally with a "size" label to give a rough estimate of complexity for the work.
Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.
Pinging for an update on this one. @colebemis would you be willing to open an issue to track the implementation of this ADR as discussed and then we can try to get this merged?
Keeping this open for a few more days as we decide if we want to change our strategy for v36
Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.
Keeping this open for a few more days as we decide if we want to change our strategy for v36
Bumping @siddharthkp, did we have a chance to validate we're happy with this strategy with regards to v36?
Bumping @siddharthkp, did we have a chance to validate we're happy with this strategy with regards to v36?
We decided not to mix this with v36 (it's already too complex) and hold implementation (and decision) until after v36!
Copy that. If we don't come back around to this in Q3, it might be good to get it on the official docket to address in Q4.
Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.
Not stale
Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.
Has this conversation gone as far as it's going to? I don't feel we ever really came to a resolution on this.
Re-opened and converted to draft as I'm not actively working on this right now
Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.