RFC: communicating package convergence status #16772
Conversation
ecraig12345
requested review from
xugao,
Hotell,
levithomason,
khmakoto,
paulgildea and
JustSlone
ecraig12345
changed the title
ecraig12345
force-pushed
the
rfc-communicating-status
branch
from
8efb2dd to
a5ea76d
Compare
ecraig12345
force-pushed
the
rfc-communicating-status
branch
from
a5ea76d to
6bab3a1
Compare
ecraig12345
force-pushed
the
rfc-communicating-status
branch
from
6bab3a1 to
d821715
Compare
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 10f294e:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 6b02081f476ecf005a9cee37ba0cad7ac3ff1190 (build) |
Perf AnalysisNo significant results to display. All results
Perf Analysis (Fluent)Perf comparison
Perf tests with no regressions
|
|
|
||
| <!-- Optional section, but useful for first drafts. Use this section to track open issues on unanswered questions regarding the design or proposal. --> | ||
|
|
||
| ### Should we change anything about `react-cards`? |
There was a problem hiding this comment.
@khmakoto I'd appreciate your input on this section in particular
There was a problem hiding this comment.
I think we should delete it from version 8. To my understanding there was one partner using it and they knew it wasn't officially released. We could still provide support in the 7.0 branch and, honestly, there's not much in terms of influx of Card related issues.
|
Quick question, @ecraig12345 how much of the proposed changes here will block V8 release? Will this push out V8 release date? (that would be quite unfortunate) |
@JustSlone Any major renames or code moves would block the release. I don't think it will push out the date. |
| - Get rid of `react-internal` | ||
| - `react-internal` components move back to `react` | ||
| - To break a cycle: `react-date-time` components move into `react`, and `react-date-time` re-exports them | ||
| - Open question: should we make code location or naming changes to any other `react-*` packages which still contain legacy components? |
There was a problem hiding this comment.
I would say it would be ideal if the only react-* packages were converged components, but I don't think it's a high priority. I would certainly not suggest we delay v8 release for moving additional react-* packages.
There was a problem hiding this comment.
Keep this because we will have a variety of other platforms (inside and outside our repo) under @fluentui scope.
Would be nice if it could mean "converged" but that's not realistic.
|
|
||
| Another sneaky way out would be moving the new card to a new package `react-card` (singular), which is a more standard name but could definitely lead to confusion having both. | ||
|
|
||
| ### Should `@fluentui/react-*` mean converged? (remove `react-` prefix from old packages?) |
There was a problem hiding this comment.
Wasn't part of the reason to prefix with react- to make it clear these packages were associated with @fluentui/react instead of northstar or web-components? Would we be losing that if we go back?
I don't have a strong preference, but wanted to make sure we aren't forgetting the reasons we prefixed with react-.
There was a problem hiding this comment.
suggestion:
I'd go with complete isolated approach in terms of folder structure (if we don't wanna do it in new repo/next branch - very common in OSS)
/packages
/react-next (>= v9)
/react - `name: @fluentui/react; version: 9.0.0-alpha.1`
-> ```
bundle which re-exports everything
export * from '@fluentui/react-button'
export * from '@fluentui/react-avatar'
....
```
/button - `name: @fluentui/react-button; version: 9.0.0-alpha.1`
/avatar- `name: @fluentui/react-avatar; version: 9.0.0-alpha.1`
/text- `name: @fluentui/react-text; version: 9.0.0-alpha.1`
/theme- `name: @fluentui/theme; version: 9.0.0-alpha.1`
/theme-provider - `name: @fluentui/theme-provider; version: 9.0.0-alpha.1`
/react (v8)
/react-button (v8)
/react-avatar (v8)
/fluentui (v0)
/web-components
There was a problem hiding this comment.
hah just realised that this wouldn't allow users to use both 8/next in hybrid mode ...
Anyways I'd still keep it "isolated" from folder structure perspective:
question/thought:
What about following?:
- consumers are using v7/8
- they have in their deps(package.json)
"@fluentui/react": "7.x" - we have converged Button -> publish
"@fluentui/react-button": "9.0.0-alpha.1" - consumer want to use converged button
- he adds
"@fluentui/react-button": "9.0.0-alpha.1" - now he can use both buttons:
import {Button} from '@fluentui/react'
import {Button as ConvergedButton} from '@fluentui/react-button'
- convergence continues
- some initial feature set of components is converged
- we publish -
"@fluentui/react": "9.0.0-alpha.1", "dist-tag": "next" - consumer continues migration/hybrid mode
- consumer comes to state that he is not using any v7/8 anymore (convergence continues)
- we have new converged components -> publish -
"@fluentui/react": "9.0.0-alpha.2", "dist-tag": "next" - consumer installs new converged suite, and removes atomic converged packages
-"@fluentui/react": "7.x"
+"@fluentui/react": "9.0.0-alpha.2"
-"@fluentui/react-button": "9.0.0-alpha.2"
-"@fluentui/react-avatar": "9.0.0-alpha.2"import {Button} from '@fluentui/react'
- import {Button as ConvergedButton} from '@fluentui/react-button'- initial convergence feature set is stable
- we release
"@fluentui/react": "9.0.0", "dist-tag": "latest"
Summary:
Pros:
- no need for temporary name for fluentui react barrel
- "less" confusion to consumers what is what
- explicit folder and single source of truth for devs working on convergence
Cons:
- Until consumer have not fully migrated to
v9they won't be able to use just whole barrel rather than granular installs.- I wouldn't say it's a con necessarily rather than more "verbose" syntax to consumers
WDYT?
There was a problem hiding this comment.
The reasoning we went with at the time of the name changes was literally "any package which uses React should have react- prefix" (not association with a particular parent package). But we can revisit that reasoning if it makes sense.
There was a problem hiding this comment.
@Hotell I think that will work at the broad strokes level.
Here's some useful context from later last year: [RFC] Increasing focus and iteration speed in convergence - Lean the Towers v2.0
To summarize what I think will work, and what I think you're saying here
- Ship
react-button 9.0-alpha(could it also bereact-button 0.0.x-alpha?) - Customers use v7/v8/v0 and
[email protected] - Later we roll
react-buttoninto@fluentui/[email protected]- Addition: when we do this I think we need to update components in
@fluentui/reactthat use button to usereact-button 9.0
- Addition: when we do this I think we need to update components in
- Eventually
@fluentui/reactis mostly converged components- At this point we could choose one of this options:
- Make minimal updates to legacy components to use tokens/styling making them "converged enough"
- Spin out remaining legacy components to their own packages, making
@fluentui/react100% converged - Some other great idea we come up with later
- At this point we could choose one of this options:
Regarding:
/packages/react-next/button (>= v9)
react-next is a folder name that will only be accurate for a period of time, which means we'll have to move the components out of this folder at some point. Do we really want to force ourselves to do file moves in the repo? I'd recommend we name this something more durable such as: react-components which could eventually become a place where we put all the component packages for the @fluentui/react library. We could then re-export these wherever we need to.
Another question on packages/react-next/button wouldn't this make it harder to find the folder which exports @fluentui/react-button? I think that's one of the benefits of having a flat packages/* folder there is a 1:1 mapping between released packages and folders in the repo. I could be wrong though.
Regarding:
consumer comes to state that he is not using any v7/8 anymore (convergence continues)
The point at which consumers are not using any v7/v8/v0 components is probably a long distance in the future. The goal for now is to build out a baseline set of converged components such that the converged components cover the common components needed to build an application. While the desire is to eventually deprecate all 80 legacy components, the time frame on that is likely exceedingly long, and the number of technical challenges that must be solved to do that is daunting.
Another thing to consider is that the cost of doing a major upgrade of @fluentui/react for both us and our partners (though mostly partners) is fairly high. I would suggest we plan on v9.0 being much later and mix-matching the converged packages with both @fluentui/[email protected] and @fluentui/react-northstar for some time.
I honestly think it wouldn't be a bad idea to keep our options open about how specifically components fold back into @fluentui/react (as long as we have multiple workable options). The number one priority we have right now is getting to a useful set of baseline components. We have partners looking to build entirely on top of converged components right now. This is a great opportunity to test out our converged model in real use cases and iterate on it quickly. Folding back into @fluentui/react is of less importance right now. As we go forward we will learn more which likely will lead us to a particular implementation.
There was a problem hiding this comment.
Another question on packages/react-next/button wouldn't this make it harder to find the folder which exports @fluentui/react-button?
I don't think so:
- you can directly point user from npm overview to implementation in any particular folder
- whole convergence is gonna be powered by TS path aliases, one click away to be redirected to right direction
- the sandboxing/separation is explicit by next folder
With that a question may arise:
- So how will we able to "gradually" add converged component to "v8"
reactsuite? -> Well only option that I see here is to specify it within direct dependencies -> thus consuming it from npm (as we'll have different versions of same pkg name in same branch) or leveraging "old" build upfront. I'll need to investigate this when the right time comes. (other thing that comes to my mind is to extendreacttsconfig with newtsconifg.base.json...)
There was a problem hiding this comment.
I don't think so:
- you can directly point user from npm overview to implementation in any particular folder
- whole convergence is gonna be powered by TS path aliases, one click away to be redirected to right direction
- the sandboxing/separation is explicit by next folder
Sounds good. I can see how a subfolder to organize things would go a long way to cleaning things up (just today someone contributing changes to slider changed packages/react-slider rather than packages/react-internal/src/components/slider, I'm not surprised they fell into that trap).
There was a problem hiding this comment.
For purposes of reducing further scope creep of this RFC, I think any repo structure changes like subfolders that don't affect package naming should be considered separately (I tried to call this out at the beginning of the doc). Totally agree that it's a possible helpful tool for clarifying to contributors what's what, but it's a whole separate discussion for another RFC. Also it can be changed at any time without impact on consumers, so it doesn't block version 8.
There was a problem hiding this comment.
The naming of a future suite package for consuming only converged components is also intended to be covered by #16577, not this RFC.
| - Possible sub-badge: [-green>)](https://github.com/microsoft/fluentui/wiki/Package-status) | ||
| - [](https://github.com/microsoft/fluentui/wiki/Package-status) | ||
|
|
||
| We might also want separate badges for category/convergence/??? (naming suggestions welcome) |
There was a problem hiding this comment.
I prefer the separate badges for convergence (if we need them).
There was a problem hiding this comment.
suggestion: I'd add explicit annotations to public APIS as well - ideally have automation that would take that as source of truth and generate those badges.
eg:
/**
* @experimental
*/
export function Button(props){....}
/**
* @stable
*/
export function Avatar(props){....}There was a problem hiding this comment.
I was going to suggest using the @packageDocumentation comment for this purpose and including a status annotation in the comment, but at least according to the API Extractor docs that's explicitly not supported. 🙄
On individual APIs we could use release tags but that has to be done on each API individually. While this is definitely something we should consider using, I'm not sure offhand how you'd set up automation to determine the overall release status of a particular package based on individual API tags.
Another possibility for communicating release status is to just use semver, like staying on a prerelease (-dev followed by -beta?) until it's ready. I guess in theory 0.x is also supposed to communicate this but it's a bit less explicit.
There was a problem hiding this comment.
yeah those annotations will definitely not "solve" the general status. but it's always a good idea to include them for particular apis to explicitly communicate.
I'm all for semver and prerelase tags 👍
There was a problem hiding this comment.
We may be too quick to "release" unstable/semi-stable things to partners--okay during iteration but we should keep the heaviest churn internal.
@alpha/@beta tags on individual APIs could be good if we have unstable APIs within an overall stable package, but having to add annotations on every API in a new package is less useful.
There was a problem hiding this comment.
Should ensure that before we "ship" anything, need agreement from relevant parties on both sub-teams. Don't repeat what we did with react-compose where we release too soon and not all parties are aware.
Consider quarterly planning for breaks and new releases for converged stuff only (separate RFC?). Won't affect v0 or v8.
There was a problem hiding this comment.
Make an RFC or something discussing previous issues we hit with versioning a suite package.
| - Possible sub-badge: [-green>)](https://github.com/microsoft/fluentui/wiki/Package-status) | ||
| - [](https://github.com/microsoft/fluentui/wiki/Package-status) | ||
|
|
||
| We might also want separate badges for category/convergence/??? (naming suggestions welcome) |
There was a problem hiding this comment.
suggestion: I'd add explicit annotations to public APIS as well - ideally have automation that would take that as source of truth and generate those badges.
eg:
/**
* @experimental
*/
export function Button(props){....}
/**
* @stable
*/
export function Avatar(props){....}|
|
||
| Another sneaky way out would be moving the new card to a new package `react-card` (singular), which is a more standard name but could definitely lead to confusion having both. | ||
|
|
||
| ### Should `@fluentui/react-*` mean converged? (remove `react-` prefix from old packages?) |
There was a problem hiding this comment.
suggestion:
I'd go with complete isolated approach in terms of folder structure (if we don't wanna do it in new repo/next branch - very common in OSS)
/packages
/react-next (>= v9)
/react - `name: @fluentui/react; version: 9.0.0-alpha.1`
-> ```
bundle which re-exports everything
export * from '@fluentui/react-button'
export * from '@fluentui/react-avatar'
....
```
/button - `name: @fluentui/react-button; version: 9.0.0-alpha.1`
/avatar- `name: @fluentui/react-avatar; version: 9.0.0-alpha.1`
/text- `name: @fluentui/react-text; version: 9.0.0-alpha.1`
/theme- `name: @fluentui/theme; version: 9.0.0-alpha.1`
/theme-provider - `name: @fluentui/theme-provider; version: 9.0.0-alpha.1`
/react (v8)
/react-button (v8)
/react-avatar (v8)
/fluentui (v0)
/web-components
| | `@fluentui/react-button` | Was previously considered ready for release (and therefore ready to export from `@fluentui/react`), but we've decided that while it's ready for partners to try out and should be stable within this major release, there will be some significant changes in the next major release. | | ||
| | `@fluentui/react-cards` | Weird case since there's an old version (built on `@uifabric/foundation` `createComponent` patterns) but it never officially graduated from experimental status and isn't exported from the suite. It also has a `next` folder where the converged version is currently being worked on. | | ||
| | `@fluentui/react-date-time` | Previously `@uifabric/date-time`. It's a rewrite (mostly by OWA folks, started awhile ago) of the original OUFR Calendar and DatePicker, but still using older patterns. New for v8, the Calendar and DatePicker from this package are exported from `@fluentui/react` by default. | | ||
| | `@fluentui/react-focus` | Current version of FocusZone with most of the work needed to converge with v0's FocusZone. Still differs in are some default values and things that are pulled from the theme. | |
There was a problem hiding this comment.
nit:
| | `@fluentui/react-focus` | Current version of FocusZone with most of the work needed to converge with v0's FocusZone. Still differs in are some default values and things that are pulled from the theme. | | |
| | `@fluentui/react-focus` | Current version of FocusZone with most of the work needed to converge with v0's FocusZone. Still differs in some default values and things that are pulled from the theme. | |
|
|
||
| <!-- Optional section, but useful for first drafts. Use this section to track open issues on unanswered questions regarding the design or proposal. --> | ||
|
|
||
| ### Should we change anything about `react-cards`? |
There was a problem hiding this comment.
I think we should delete it from version 8. To my understanding there was one partner using it and they knew it wasn't officially released. We could still provide support in the 7.0 branch and, honestly, there's not much in terms of influx of Card related issues.
|
|
||
| ### Use semver to help communicate status | ||
|
|
||
| New packages should be created with version `0.1.0`. While the open source community (and even this repo) is extremely inconsistent in its use of `0.x` versions, that is in theory the "standard" way to indicate that a package is not production-ready. |
There was a problem hiding this comment.
add:
0.x means not ready. Badges will help clarify this due to inconsistent usage of 0.x in ecosystem.
When things are close to ready, we should release 1.0.0-alpha (may break, frequent revisions). -beta once it's closer (close to stable, time to try out). Release once it's final.
Start everything at 1.0.0-alpha. Disadvantage is you can't go back to 0.x. Advantage is it can be consistent across all packages. Communicates that we're actively iterating and you should not use this.
Maybe: Go to -beta when it's almost stable. It's still NOT ready to go in your production pipeline and be pushed out to customers, but it's ready to try out in a branch to verify that the component generally works. Still need more consideration of whether this phase should exist.
Doesn't have to be perfect to be 1.0. Major release is more about when we're ready to commit to it on an initial version.
Larger versioning discussion: how to major bump in a monorepo, and how hard it is
|
Because this pull request has not had activity for over 150 days, we're automatically closing it for house-keeping purposes. The pull request will still be available for reference. If it's still relevant to merge at some point, you can reopen or make a new version based on the latest code. |
msft-fluent-ui-bot
added
the
Resolution: Soft Close
|
Leaving this closed since it's a mix of implemented and abandoned. The badge idea might be worth picking up again, but not essential immediately (and we improved the state of that using prerelease tags). |
Splitting out parts of #16577 since the discussion got very long. The split is a bit awkward but having everything in the one RFC was becoming unwieldy.
The unifying theme of this RFC is essentially steps that we should take in the very near future to clearly communicate to consumers which of our packages are old vs. converged. (Open to suggestions on naming and/or better ways to split.)
See an HTML rendered version of the RFC here.
Points this RFC covers:
@fluentui/react(removingreact-internal)react-cardsreact-*means converged?