RFC: Global context registration for v9 by ling1726 · Pull Request #22300 · microsoft/fluentui

ling1726 · 2022-04-04T07:13:21Z

This RFC proposes a solution for #21338 that involves registering one
single context version per major in the global namespace to be consumed
within Fluent.

PREVIEW

This RFC proposes a solution for microsoft#21338 that involves registering one single context version per major in the global namespace to be consumed within Fluent.

codesandbox-ci · 2022-04-04T07:18:49Z

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 c04e7db:

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

fabricteam · 2022-04-04T07:21:41Z

📊 Bundle size report

_{🤖 This report was generated against 7940614c4a775722266127c3fec0706896506c71}

size-auditor · 2022-04-04T07:29:22Z

Asset size changes

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

Baseline commit: f2fd9407493b438898c6027fde432a1c7fefd344 (build)

Hotell · 2022-04-06T09:11:35Z

+- One single major version context regardless of duplicate dependencies
+- Not too different from `React.createContext` - we still use React contexts
+- Global contexts are 'obfuscated' on window/node global with `Symbol`
+- Functionality can be explicitly enabled


I guess this was supposed to be "Opt in functionality" ?

yeah it is supposed to be opt-in, I just didn't do it in the prototypes I made

Hotell · 2022-04-06T09:13:28Z

+
+## Detailed Design or Proposal
+
+Prototypes can be found in the following repos for create-react-app and next.js SSR respectively:


suggestion: can we inline that prototype implementation (code) or at least pseudocode to this RFC ? I mean those repos can be removed by any time, thus we would loose reference for future ourselves.

In this case I can do it easily - the prototype implem is not that big.

Co-authored-by: Martin Hochel <[email protected]>

Hotell

This is good enough start. we can have more focus talk about specifics during implementation phase in a PR. ty for looking into this !

…ntui into rfc/global-context

miroslavstastny · 2022-04-13T16:35:52Z

+Many of our partners have complex dependency trees. The complexity in dependency management means that duplicate
+packages can exist in `node_modules` folders. Although duplicate depedencies should generally be avoided, this still
+happens quite often due to these factors:


Do we actually want to support multiple versions in the bundle or avoid them? We can use similar approach (or just a dumb global variable) to detect that there are multiple versions of the contexts used in the application and throw in that case.

I like that idea. I am just not optimistic that it will be enough. From what I've seen partners will currently tolerate multiple versions of Fluent in the library anyway.

layershifter · 2022-05-04T13:54:24Z

+  const fooContextDefaultValue: FooContextValue = {
+    foo: 'xxx',
+    bar: 'yyy',
+  };


This should be defined outside a hook to be the same reference to mimic React's behavior.

hmm not sure that it could work since once we have duplicate modules we have the same problem as contexts not matching no ?

otherwise we'd need some code to set defaults explicitly in the hook 🤔🤔

Let's that's true... As we don't export the context it might be not a big issue.

layershifter · 2022-05-04T13:57:04Z

+- Global contexts are 'obfuscated' on window/node global with `Symbol`
+- Functionality can be explicitly enabled
+- Functionality can be applied with post processing so that we still use `React.createContext` internally
+- Not exporting the actual context is good API encapsulation


It's not related to that proposal technically. We can avoid context exports even without this change

layershifter · 2022-05-04T13:58:29Z

+
+## Problem statement
+
+Many of our partners have complex dependency trees. The complexity in dependency management means that duplicate


Is @fluentui/react-components the single package that has such issues?

no, but an opt-in solution (that is discouraged) would mean that there is some kind of workaround

layershifter · 2022-05-04T14:06:32Z

+- Custom magic when creating contexts in Fluent
+- Generally using window/node globals is an antipattern
+- Enforces stricter practices for using contexts
+


Suggested change

- Bundle size increase for any context (+ 18.95 kB minified)

I measured it based on the prototype that you have. The main increase comes from semver package.

Thanks, I'll try to remove semver from the prototype

In order to provider proper encapsulation and allow us to change internals more easily, this PR removes exports of context objects from react-components. In cases where contexts were removed, the export was replaced with a specific provider component and hook to use the context value. Follows from microsoft#22300

* BREAKING: Remove context exports from react-components In order to provider proper encapsulation and allow us to change internals more easily, this PR removes exports of context objects from react-components. In cases where contexts were removed, the export was replaced with a specific provider component and hook to use the context value. Follows from #22300 * changefile * stop exporting menu contexts * changefile * update md Co-authored-by: Sean Monahan <[email protected]>

* react-button: Removing deprecated block prop from all button components (#23071) * react-button: Removing deprecated block prop from all button components. * Removing block stories. * Adding changefile. * Removing block prop from migration guide and vr tests. * fix(react-text): Update typography wrappers to latest naming (#22934) * fix: change Headline to Subtitle1 * fix: change Subheadline to Subtitle2 * fix: change Body to Body1 * fix: change Caption to Caption1 * feat: add Caption2 * chore: use better vr story name * chore: add change files * chore: update api * fix: rename components in missing story * feat: removed unused `caption2ClassName` * chore: disable classname conformance test * chore: remove deprecated className * chore: add conformance test exception * BREAKING: Removing deprecated `<componentName>ClassName` exports from v9 packages (#23026) * removing single static classnames * api files * change files * disabling component-has-static-classname-exported test in react-image * preparing to merge master * disabling export tests * removing change file for text * change file * disabling test in Dialog * disabling more tests * remove react-text class names Co-authored-by: Sean Monahan <[email protected]> * BREAKING: Update string unions to `spinal-case` (#23001) * react-avatar: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-input: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-radio: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-select: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-spinbutton: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-tabster: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-textarea: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-badge: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-combobox: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * add change files for spinal-case update * react-card: update tabster tab behaviors Updates the `useCard()` implementation after the `react-tabster` spinal-case breaking change. * vr-tests: update types for v9 tests * react-avatar: update badge types in stories Avatar stories were using the old, camelCase PresenceBadge types. * react-badge: fix style that broke with spinal-case change One of the "doNotDisturb" cases was missed due to the non-typesafe way we tested the PresenceBadge status in styles. This test has been changed to a typesafe function call instead. * react-combobox: update props in appearance story Change filledDarker and filledLighter to filled-darker and filled-lighter respectively. * react-badge: update camelCase to spinal-case for strings * fix: missing inputClassName in merge conflict resolution * react-select: update api snapshot * react-alert: update conformance tests * fix: update react-components api snapshot * BREAKING CHANGE: Encapsulate Popper API (#22586) * BREAKING CHANGE: Encapsulate Popper API Updates `@fluentui/react-positioning` so that none of the popper API and types leak anymore * changefiles * update test * update stories * positioned to positionedElement * update api * remove getBasePlacement * add tests * virtual element and boundary * fix types * PR fixes * update md * fix type * changefile * Update packages/react-components/react-combobox/src/components/Combobox/useCombobox.ts Co-authored-by: Ben Howell <[email protected]> * Update packages/react-components/react-tooltip/src/components/Tooltip/useTooltip.tsx Co-authored-by: Ben Howell <[email protected]> * rename popperOptions to positioningOptions Co-authored-by: Ben Howell <[email protected]> * BREAKING: Remove context exports from react-components (#23085) * BREAKING: Remove context exports from react-components In order to provider proper encapsulation and allow us to change internals more easily, this PR removes exports of context objects from react-components. In cases where contexts were removed, the export was replaced with a specific provider component and hook to use the context value. Follows from #22300 * changefile * stop exporting menu contexts * changefile * update md Co-authored-by: Sean Monahan <[email protected]> * react-popover: update api snapshot * BREAKING(react-theme): remove webHighContrastTheme (#23083) * fix(react-theme): Remove webHighContrast theme Co-authored-by: Makoto Morimoto <[email protected]> Co-authored-by: André Dias (he/him) <[email protected]> Co-authored-by: Esteban Munoz Facusse <[email protected]> Co-authored-by: ling1726 <[email protected]> Co-authored-by: Ben Howell <[email protected]> Co-authored-by: Miroslav Stastny <[email protected]>

* react-button: Removing deprecated block prop from all button components (microsoft#23071) * react-button: Removing deprecated block prop from all button components. * Removing block stories. * Adding changefile. * Removing block prop from migration guide and vr tests. * fix(react-text): Update typography wrappers to latest naming (microsoft#22934) * fix: change Headline to Subtitle1 * fix: change Subheadline to Subtitle2 * fix: change Body to Body1 * fix: change Caption to Caption1 * feat: add Caption2 * chore: use better vr story name * chore: add change files * chore: update api * fix: rename components in missing story * feat: removed unused `caption2ClassName` * chore: disable classname conformance test * chore: remove deprecated className * chore: add conformance test exception * BREAKING: Removing deprecated `<componentName>ClassName` exports from v9 packages (microsoft#23026) * removing single static classnames * api files * change files * disabling component-has-static-classname-exported test in react-image * preparing to merge master * disabling export tests * removing change file for text * change file * disabling test in Dialog * disabling more tests * remove react-text class names Co-authored-by: Sean Monahan <[email protected]> * BREAKING: Update string unions to `spinal-case` (microsoft#23001) * react-avatar: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-input: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-radio: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-select: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-spinbutton: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-tabster: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-textarea: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-badge: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-combobox: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * add change files for spinal-case update * react-card: update tabster tab behaviors Updates the `useCard()` implementation after the `react-tabster` spinal-case breaking change. * vr-tests: update types for v9 tests * react-avatar: update badge types in stories Avatar stories were using the old, camelCase PresenceBadge types. * react-badge: fix style that broke with spinal-case change One of the "doNotDisturb" cases was missed due to the non-typesafe way we tested the PresenceBadge status in styles. This test has been changed to a typesafe function call instead. * react-combobox: update props in appearance story Change filledDarker and filledLighter to filled-darker and filled-lighter respectively. * react-badge: update camelCase to spinal-case for strings * fix: missing inputClassName in merge conflict resolution * react-select: update api snapshot * react-alert: update conformance tests * fix: update react-components api snapshot * BREAKING CHANGE: Encapsulate Popper API (microsoft#22586) * BREAKING CHANGE: Encapsulate Popper API Updates `@fluentui/react-positioning` so that none of the popper API and types leak anymore * changefiles * update test * update stories * positioned to positionedElement * update api * remove getBasePlacement * add tests * virtual element and boundary * fix types * PR fixes * update md * fix type * changefile * Update packages/react-components/react-combobox/src/components/Combobox/useCombobox.ts Co-authored-by: Ben Howell <[email protected]> * Update packages/react-components/react-tooltip/src/components/Tooltip/useTooltip.tsx Co-authored-by: Ben Howell <[email protected]> * rename popperOptions to positioningOptions Co-authored-by: Ben Howell <[email protected]> * BREAKING: Remove context exports from react-components (microsoft#23085) * BREAKING: Remove context exports from react-components In order to provider proper encapsulation and allow us to change internals more easily, this PR removes exports of context objects from react-components. In cases where contexts were removed, the export was replaced with a specific provider component and hook to use the context value. Follows from microsoft#22300 * changefile * stop exporting menu contexts * changefile * update md Co-authored-by: Sean Monahan <[email protected]> * react-popover: update api snapshot * BREAKING(react-theme): remove webHighContrastTheme (microsoft#23083) * fix(react-theme): Remove webHighContrast theme Co-authored-by: Makoto Morimoto <[email protected]> Co-authored-by: André Dias (he/him) <[email protected]> Co-authored-by: Esteban Munoz Facusse <[email protected]> Co-authored-by: ling1726 <[email protected]> Co-authored-by: Ben Howell <[email protected]> Co-authored-by: Miroslav Stastny <[email protected]>

* RFC: Global context registration for v9 This RFC proposes a solution for microsoft#21338 that involves registering one single context version per major in the global namespace to be consumed within Fluent. * Update rfcs/react-components/convergence/global-contex-registration.md Co-authored-by: Martin Hochel <[email protected]> * suggestions * Add forwards/backwards compat * feedback * bundle size * discarded solution * Update with code transform info * fix linke Co-authored-by: Martin Hochel <[email protected]>

RFC: Global context registration for v9

b9aefca

This RFC proposes a solution for microsoft#21338 that involves registering one single context version per major in the global namespace to be consumed within Fluent.

msft-fluent-ui-bot assigned Hotell Apr 4, 2022

ling1726 marked this pull request as draft April 4, 2022 07:13

ling1726 added the Type: RFC Request for Feedback label Apr 4, 2022

ling1726 marked this pull request as ready for review April 6, 2022 08:58

ling1726 requested review from a team April 6, 2022 08:58

Hotell reviewed Apr 6, 2022

View reviewed changes

Update rfcs/react-components/convergence/global-contex-registration.md

60927e2

Co-authored-by: Martin Hochel <[email protected]>

Hotell approved these changes Apr 6, 2022

View reviewed changes

ling1726 added 2 commits April 6, 2022 17:11

suggestions

b825447

Merge branch 'rfc/global-context' of https://github.com/ling1726/flue…

c428acf

…ntui into rfc/global-context

raghavthind2005 approved these changes Apr 7, 2022

View reviewed changes

Hotell removed their assignment Apr 8, 2022

Hotell mentioned this pull request Apr 8, 2022

core: introduce core package which will encapsulate packages that contain singletons #22028

Closed

miroslavstastny reviewed Apr 13, 2022

View reviewed changes

Comment thread rfcs/react-components/convergence/global-contex-registration.md

miroslavstastny assigned ling1726 Apr 21, 2022

miroslavstastny added this to the Fluent UI v9 milestone Apr 21, 2022

GeoffCoxMSFT requested review from GeoffCoxMSFT and removed request for a team May 2, 2022 19:28

ling1726 added 2 commits May 4, 2022 15:49

Merge branch 'master' into rfc/global-context

8ea59bd

Add forwards/backwards compat

0ddb757

layershifter reviewed May 4, 2022

View reviewed changes

Comment thread rfcs/react-components/convergence/global-contex-registration.md Outdated

layershifter reviewed May 4, 2022

View reviewed changes

Comment thread rfcs/react-components/convergence/global-contex-registration.md Outdated

ling1726 closed this May 4, 2022

ling1726 reopened this May 4, 2022

layershifter reviewed May 4, 2022

View reviewed changes

ling1726 added 3 commits May 4, 2022 16:14

feedback

062e0b5

bundle size

438977a

discarded solution

5c2e7e5

ling1726 requested a review from a team as a code owner May 5, 2022 10:28

ling1726 force-pushed the rfc/global-context branch from c54dc47 to 5c2e7e5 Compare May 5, 2022 10:28

ling1726 mentioned this pull request May 19, 2022

BREAKING: Remove context exports from react-components #23085

Merged

ling1726 added 3 commits May 25, 2022 16:42

Merge branch 'master' into rfc/global-context

61a63c9

Update with code transform info

5dbe332

fix linke

c04e7db

layershifter approved these changes Jun 1, 2022

View reviewed changes

spmonahan approved these changes Jun 1, 2022

View reviewed changes

ling1726 merged commit e2617fc into microsoft:master Jun 2, 2022


		## Detailed Design or Proposal

		Prototypes can be found in the following repos for create-react-app and next.js SSR respectively:


		## Problem statement

		Many of our partners have complex dependency trees. The complexity in dependency management means that duplicate


	- Bundle size increase for any context (+ 18.95 kB minified)

Uh oh!

Conversation

ling1726 commented Apr 4, 2022

Uh oh!

codesandbox-ci Bot commented Apr 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fabricteam commented Apr 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Bundle size report

Uh oh!

size-auditor Bot commented Apr 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Asset size changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ling1726 Apr 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hotell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

layershifter May 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codesandbox-ci Bot commented Apr 4, 2022 •

edited

Loading

fabricteam commented Apr 4, 2022 •

edited

Loading

size-auditor Bot commented Apr 4, 2022 •

edited

Loading

ling1726 Apr 6, 2022 •

edited

Loading

layershifter May 4, 2022 •

edited

Loading