docs(rfc): add api access modifiers style-guide RFC for v9 #23577

Hotell · 2022-06-16T14:28:30Z

This RFC introduces style guide/ common approach for our packages API access modifiers that will be enforced by tooling.

Preview 📖 👀

Tooling implementation PR 🛠

📣 Call to action to all teams

To be able to not delay v9 stable release any further we need all teams to review their packages by the end of Wednesday 22nd of June and apply following:

if you use @internal modifier already, please verify that it is not exposed from package public API. If that's the case please remove it - BREAKING CHANGE in RC
mark your package APIs with @internal modifier where necessary taking into consideration former bullet point

Why is this needed?

As/If we introduce this style-guide/workflow, with aforementioned tooling PR being merged and packages migrated to that new behavior, our packages will trim away @internal.

if we won't catch this now, we will introduce BREAKING CHANGES in stable release which is a NO GO.

fabricteam · 2022-06-16T14:34:02Z

📊 Bundle size report

_{🤖 This report was generated against 02a6a76a3350adfb4fd95cdfc178864801c8e2bf}

codesandbox-ci · 2022-06-16T14:35:52Z

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 121875e:

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration

size-auditor · 2022-06-16T14:42:08Z

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 02a6a76a3350adfb4fd95cdfc178864801c8e2bf (build)

rfcs/shared/build-system/package-public-api.md

ling1726 · 2022-06-16T18:06:20Z

rfcs/shared/build-system/package-public-api.md

+export declare function hello(): void;
+```
+
+#### `@alpha`


I'm not quite sure how useful the prerelease stage modifiers will be since it's harder for partners to be aware of these tags, not all IDEs will render tsdoc the same way. For the scenarios that the prerelease modifiers cover, IMO /unstable entrypoint + _unstable suffix would be more impactful

I'm not quite sure how useful the prerelease stage modifiers will be since it's harder for partners to be aware of these tags, not all IDEs will render tsdoc the same way

good point, lemme provide a visual for the usage

graph TD A["@alpha,@beta"] --> U["@fluentui/pacakge-name/unstable"] B["@public"] --> S["@fluentui/pacakge-name"] C["@internal"] --> E((X))

Loading

these access modifiers will:

enforce us to not expose public/beta/internal by accident as part of public API

guard us from exposing public/beta shipped from package /unstable api to be shipped in consuming package as stable API

additional context: if needed we can allow @beta to be shipped from stable and only @alpha from unstable but I'd leave that to team decisions ( IMO we should not do that ).

Is that more clear now ? If yes I'll add the graphics to RFC. ty!

additional context: if needed we can allow @beta to be shipped from stable and only @Alpha from unstable but I'd leave that to team decisions ( IMO we should not do that ).

As @beta packages could have breaking changes IMO we should not ship them under stable.

To check my understanding: we have following chain for packages alpha -> beta -> rc. If we follow the same for APIs in comments, we should have also @rc comment, right? 👾

does this mean that @fluentui/new-component will only be usable from @fluentui/new-component/unstable while the component is in alpha and beta stages ?

does this mean that @fluentui/new-component will only be usable from @fluentui/new-component/unstable while the component is in alpha and beta stages ?

yes that's the idea (from TS api POV )

ling1726

I'm not fully convinced about the prerelease modifiers, but they are fully usable withour existing practices.

What I love is the ability to completely strip dts for fully internal APIs

rfcs/shared/build-system/package-public-api.md

spmonahan · 2022-06-16T16:03:56Z

rfcs/shared/build-system/package-public-api.md

+It indicates that an API item has been officially released, and is now part of the supported contract for a package. Semver versioning is used and enforced thus the API signature cannot be changed without a MAJOR version increment.
+
+- tooling will NOT trim the declaration from a public API surface.
+- CANNOT be exposed from `/unstable` API surface


This seems like the only reason to use @public. Based on the other examples it seems that everything is implicitly public unless otherwise annotated. Do we need this annotation? When would I use it?

it seems that everything is implicitly public unless otherwise annotated.?

yeah, API without annotation is implicitly considered as public ->

We can enforce it with tooling, so missing annotation will fail on CI/build

Do we need this annotation? When would I use it?

It might feel like unnecessary churn for some devs to add this annotation all the time, on the other hand:

it will force to provide reasonable amount of documentation to public APIs which is what we should be doing anyways

consistency of using modifiers

personally I have no strong opinions on this, besides having consistency

IMO we should enforce it via ESLint to be sure that all APIs are properly annotated.

Thanks for the explanation. That really helped.

As you said, it is an extra bit of typing/work but I like the fact that everything would then always be explicit. We uncovered quite a few accidentally public APIs during bug bash so I think it's worth being strict about.

rfcs/shared/build-system/package-public-api.md

Co-authored-by: ling1726 <[email protected]>

layershifter · 2022-06-17T08:43:21Z

rfcs/shared/build-system/package-public-api.md

+export declare function hello(): void;
+```
+
+#### `@beta`


I don't see any difference between in @alpha & @beta except semantics, is correct? As we export them as under unstable is there a sense to use @unstable instead?

IMO it will reduce maintenance cost as with current proposal we should ensure that @alpha will be changed to @beta when a package will be promoted. And we don't have any automation for it...

I don't see any difference between in @Alpha & @beta except semantics, is correct

correct. I added more context to one of Lings comments
↓

As we export them as under unstable is there a sense to use @unstable instead?

that's great idea ! we would need to get this implemented to api-extractor as it has limited support for access modifiers stripping / or eventually introduce custom tooling

I was looking more into custom annotations and api-extractor support and there might be some possibilities to do this. api-extractor documenter already supports tsdoc that allows custom annotation definitions. similar implementation would be needed for dts stripping mechanism. I'll create issue on Rush repo to add this support.

microsoft/rushstack#3558

GeoffCoxMSFT · 2022-06-20T15:36:48Z

rfcs/shared/build-system/package-public-api.md

+
+### Proposed access modifiers:
+
+#### `@internal`


We could have @public be the modifier and have the tooling make all things @internal by default. This would force us to be explicit in what is exposed. If we forget the attribute, the method or type can't get published.

I actually agree with this 👍 how easy it is to implement might be a differnt story

interesting idea indeed @GeoffCoxMSFT ! that would work only for stable API's though, which is not our use-case as we have various stages of API/packages that are being shipped to users.

how easy it is to implement might be a differnt story

haha YES

Hotell · 2022-07-28T16:16:59Z

rfcs/shared/build-system/package-public-api.md

+export declare function hello(): void;
+```
+
+### Summary


Please check out the summary with Fufure work paragraph - based on our discussion if it makes sense to you and if we can proceed.

@layershifter @spmonahan @ling1726

cheers!

rfcs/shared/build-system/package-public-api.md

Co-authored-by: Oleksandr Fediashov <[email protected]>

rfcs/shared/build-system/package-public-api.md

Co-authored-by: Tristan Watanabe <[email protected]>

spmonahan · 2022-08-17T15:13:28Z

@Hotell In a recent discussion the possibility of an unstable API or feature for an otherwise stable component came up.

As a hypothetical let's say we need to add a new feature to SpinButton. Today SpinButton is stable in v9. We are unsure of either the exact API or the exact behavior (or perhaps both!) of this new feature but we want to be able to ship it so our users can test out the feature and provide feedback. How can we mark this feature as "unstable" so it's clear to users that it should be used with caution but available on an otherwise stable component?

I think this should be essentially the same flow as moving an unstable component to stable, just applied to individual features of a component.

I'm bringing this up now because it may be something we need in the future but I definitely don't think it should block this PR in any way.

docs(rfc): add api acessors rfc

915b546

msft-fluent-ui-bot assigned khmakoto Jun 16, 2022

github-actions bot added this to the June Project Cycle Q2 2022 milestone Jun 16, 2022

github-actions bot added the Type: RFC Request for Feedback label Jun 16, 2022

Hotell changed the title ~~docs(rfc): add api acessors rfc~~ docs(rfc): add api access modifiers style-guide RFC for v9 Jun 16, 2022

Hotell added the Fluent UI react-components (v9) label Jun 16, 2022

Hotell marked this pull request as ready for review June 16, 2022 14:52

miroslavstastny requested a review from a team June 16, 2022 14:59

andrefcdias self-requested a review June 16, 2022 15:15

ling1726 reviewed Jun 16, 2022

View reviewed changes

rfcs/shared/build-system/package-public-api.md Outdated Show resolved Hide resolved

ling1726 reviewed Jun 16, 2022

View reviewed changes

rfcs/shared/build-system/package-public-api.md Show resolved Hide resolved

ling1726 reviewed Jun 16, 2022

View reviewed changes

ling1726 approved these changes Jun 16, 2022

View reviewed changes

spmonahan reviewed Jun 16, 2022

View reviewed changes

Hotell requested review from a team June 17, 2022 08:14

Update rfcs/shared/build-system/package-public-api.md

f138d72

Co-authored-by: ling1726 <[email protected]>

layershifter reviewed Jun 17, 2022

View reviewed changes

layershifter approved these changes Jun 17, 2022

View reviewed changes

spmonahan approved these changes Jun 17, 2022

View reviewed changes

GeoffCoxMSFT reviewed Jun 20, 2022

View reviewed changes

GeoffCoxMSFT approved these changes Jun 20, 2022

View reviewed changes

ling1726 mentioned this pull request Jun 23, 2022

chore: Mark teams-prg owned APIs with @internal #23689

Merged

sopranopillow mentioned this pull request Jun 23, 2022

docs(react-avatar): Adding @internal api access modifier to getInitials #23707

Merged

andrefcdias removed their request for review July 4, 2022 13:01

andrefcdias removed the request for review from a team July 4, 2022 16:52

bsunderhus approved these changes Jul 19, 2022

View reviewed changes

Hotell mentioned this pull request Jul 28, 2022

Add Ability to Annotate Sub-Package Exports As "Internal" #23343

Open

6 tasks

docs: add summary with future plans based on feedback

e2f7505

github-actions bot modified the milestones: June Project Cycle Q2 2022, July Project Cycle Q3 2022 Jul 28, 2022

Hotell commented Jul 28, 2022

View reviewed changes

Hotell requested review from layershifter, spmonahan and ling1726 July 28, 2022 16:18

Hotell mentioned this pull request Jul 28, 2022

[api-extractor]: support custom access modifiers for API stripping microsoft/rushstack#3558

Open

docs: add links to api-extractor issues

dba9b80

spmonahan approved these changes Jul 28, 2022

View reviewed changes

layershifter reviewed Jul 29, 2022

View reviewed changes

rfcs/shared/build-system/package-public-api.md Outdated Show resolved Hide resolved

Update rfcs/shared/build-system/package-public-api.md

d458514

Co-authored-by: Oleksandr Fediashov <[email protected]>

TristanWatanabe approved these changes Aug 11, 2022

View reviewed changes

Apply suggestions from code review

121875e

Co-authored-by: Tristan Watanabe <[email protected]>

Hotell enabled auto-merge (squash) August 17, 2022 15:08

Hotell merged commit 813f873 into microsoft:master Aug 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs(rfc): add api access modifiers style-guide RFC for v9 #23577

docs(rfc): add api access modifiers style-guide RFC for v9 #23577

Hotell commented Jun 16, 2022 •

edited by bsunderhus

Loading

fabricteam commented Jun 16, 2022 •

edited

Loading

codesandbox-ci bot commented Jun 16, 2022 •

edited

Loading

size-auditor bot commented Jun 16, 2022 •

edited

Loading

ling1726 Jun 16, 2022

Hotell Jun 17, 2022 •

edited

Loading

layershifter Jun 17, 2022

ling1726 Jun 17, 2022

Hotell Jun 21, 2022 •

edited

Loading

ling1726 left a comment

spmonahan Jun 16, 2022

Hotell Jun 17, 2022 •

edited

Loading

layershifter Jun 17, 2022

spmonahan Jun 17, 2022

layershifter Jun 17, 2022

Hotell Jun 17, 2022 •

edited

Loading

Hotell Jun 21, 2022

Hotell Jul 28, 2022

GeoffCoxMSFT Jun 20, 2022

ling1726 Jun 20, 2022

Hotell Jun 21, 2022

Hotell Jul 28, 2022

spmonahan commented Aug 17, 2022

docs(rfc): add api access modifiers style-guide RFC for v9 #23577

docs(rfc): add api access modifiers style-guide RFC for v9 #23577

Conversation

Hotell commented Jun 16, 2022 • edited by bsunderhus Loading

📣 Call to action to all teams

fabricteam commented Jun 16, 2022 • edited Loading

📊 Bundle size report

codesandbox-ci bot commented Jun 16, 2022 • edited Loading

size-auditor bot commented Jun 16, 2022 • edited Loading

Asset size changes

Choose a reason for hiding this comment

Hotell Jun 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hotell Jun 21, 2022 • edited Loading

Choose a reason for hiding this comment

ling1726 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hotell Jun 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hotell Jun 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spmonahan commented Aug 17, 2022

Hotell commented Jun 16, 2022 •

edited by bsunderhus

Loading

fabricteam commented Jun 16, 2022 •

edited

Loading

codesandbox-ci bot commented Jun 16, 2022 •

edited

Loading

size-auditor bot commented Jun 16, 2022 •

edited

Loading

Hotell Jun 17, 2022 •

edited

Loading

Hotell Jun 21, 2022 •

edited

Loading

Hotell Jun 17, 2022 •

edited

Loading

Hotell Jun 17, 2022 •

edited

Loading