feat(react-drawer): release to 9.0.0 stable #29711

marcosmoura · 2023-10-31T18:47:02Z

Release Drawer to stable. Drawer do not use the new -preview pattern.

fabricteam · 2023-10-31T18:58:45Z

Perf Analysis (`@fluentui/react-components`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	634	627	5000
Button	mount	315	319	5000
Field	mount	1120	1158	5000
FluentProvider	mount	707	717	5000
FluentProviderWithTheme	mount	73	86	10
FluentProviderWithTheme	virtual-rerender	65	67	10
FluentProviderWithTheme	virtual-rerender-with-unmount	70	79	10
MakeStyles	mount	862	881	50000
Persona	mount	1715	1696	5000
SpinButton	mount	1407	1426	5000

codesandbox-ci · 2023-10-31T19:00:11Z

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 2601b36:

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

fabricteam · 2023-10-31T19:00:56Z

📊 Bundle size report

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-components` react-components: Button, FluentProvider & webLightTheme	`70.007 kB` `20.164 kB`
`react-components` react-components: Accordion, Button, FluentProvider, Image, Menu, Popover	`207.417 kB` `59.289 kB`
`react-components` react-components: FluentProvider & webLightTheme	`42.291 kB` `14.005 kB`
`react-portal-compat` PortalCompatProvider	`6.651 kB` `2.252 kB`

_{🤖 This report was generated against aa664abe172b5ceedb63bafdc690ac2e82ab48db}

size-auditor · 2023-10-31T19:08:40Z

Asset size changes

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

Baseline commit: aa664abe172b5ceedb63bafdc690ac2e82ab48db (build)

packages/react-components/react-drawer/package.json

change/@fluentui-react-drawer-3cfc5c74-8b22-419c-a72a-43ecad9c876f.json

change/@fluentui-react-components-03e7f032-b0d9-4221-8522-b7bfb9ffb223.json

packages/react-components/react-components/src/index.ts

layershifter

We should not expose APIs from preview components in APIs for stable components:

fluentui/packages/react-components/react-drawer/src/shared/DrawerBase.types.ts

Line 28 in d35fc26

open?: MotionShorthand<HTMLDivElement>;

fluentui/packages/react-components/react-drawer/src/shared/DrawerBase.types.ts

Line 32 in d35fc26

motion: MotionState<HTMLDivElement>;

Please update types to not reference @fluentui/react-motion-preview.

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

Hotell · 2023-11-01T12:16:52Z

We should not expose APIs from preview components in APIs for stable components:

fluentui/packages/react-components/react-drawer/src/shared/DrawerBase.types.ts

Line 28 in d35fc26

open?: MotionShorthand<HTMLDivElement>;

fluentui/packages/react-components/react-drawer/src/shared/DrawerBase.types.ts

Line 32 in d35fc26

motion: MotionState<HTMLDivElement>;

Please update types to not reference @fluentui/react-motion-preview.

@layershifter Can you elaborate please?

based on what rule is this statement. I'm not aware of any official agreement?
do we have any documentation for this?
drawer has "@fluentui/react-motion-preview": "^0.4.0" as direct dependency also it uses runtime apis

preview packages don't suffer semver breaking issues like old unstable releases

layershifter · 2023-11-01T13:16:06Z

@layershifter Can you elaborate please?

based on what rule is this statement. I'm not aware of any official agreement?

do we have any documentation for this?

drawer has "@fluentui/react-motion-preview": "^0.4.0" as direct dependency also it uses runtime apis

* preview packages don't suffer semver breaking issues like old unstable releases

@Hotell the same discussion happened in #29391.

Currently, drawer components expose an API from preview package via open prop which is public API. You can see an example of its usage in https://react.fluentui.dev/?path=/docs/preview-components-drawer--default#motion-custom.

If a breaking change in @fluentui/react-motion-preview happens and modifies MotionShorthand or MotionState:

it's fine to do breaks in -preview, we will a major bump of @fluentui/react-motion-preview
we will get a break in public interface of @fluentui/react-drawer 🚨

However, now stable and preview packages linked by public interfaces. For example, I use @fluentui/react-components and @fluentui/react-motion-preview in a project:

import { OverlayDrawer } from "@fluentui/react-components" // 9.0.0
import { useMotion } from "@fluentui/react-motion-preview" // 0.4.0

function App() {
  const motion = useMotion();

  return <OverlayDrawer open={motion} />
}

A break in @fluentui/react-motion-preview happens, we bump @fluentui/react-motion-preview to 1.0.0 & @fluentui/react-components to 9.1.0 (as we depend on unstable API in @fluentui/react-drawer).

import { OverlayDrawer } from "@fluentui/react-components" // 9.1.0
import { useMotion } from "@fluentui/react-motion-preview" // 0.4.0

function App() {
  const motion = useMotion();

  // 💥💥💥💥💥💥💥
  // Builds breaks as `motion` type have been changed
  return <OverlayDrawer open={motion} />
}

It's fine to use -preview packages inside stable ones until we are starting to leak APIs from them. It have not been documented as it's a second time when it happens.

Hotell · 2023-11-01T15:20:43Z

thanks for providing needed context @layershifter ,

so basically what should be done here is to inline/copy that type from react-motion-preview within react-drawer component correct ? Then it's the owner responsibility to not break that type contract ( besides runtime contract that's happening behind the scenes ), because TS is structural something like this is easy doable do decouple boundaries with only 1 downside ( no DRY code )

layershifter · 2023-11-01T15:55:58Z

so basically what should be done here is to inline/copy that type from react-motion-preview within react-drawer component correct ?

Yeah, that works.

@marcosmoura is there is a strict requirement to expose API for overrides? I mean this:

fluentui/packages/react-components/react-drawer/src/shared/DrawerBase.types.ts

Line 28 in d35fc26

open?: MotionShorthand<HTMLDivElement>;

Which is basically boolean | Record<X, Y>.

If there is no such requirement now, we will have less constraints in future if we will just expose open: boolean in props.

Hotell · 2023-11-01T18:55:19Z

@marcosmoura is there is a strict requirement to expose API for overrides? I mean this:

fluentui/packages/react-components/react-drawer/src/shared/DrawerBase.types.ts

Line 28 in d35fc26

open?: MotionShorthand<HTMLDivElement>;

Which is basically boolean | Record<X, Y>.

If there is no such requirement now, we will have less constraints in future if we will just expose open: boolean in props.

yup if possible, this would be ideal (drastically reducing that non DRY I mentioned in previous comments ) 🙏

marcosmoura · 2023-11-02T11:24:49Z

so basically what should be done here is to inline/copy that type from react-motion-preview within react-drawer component correct ?

Yeah, that works.

@marcosmoura is there is a strict requirement to expose API for overrides? I mean this:

fluentui/packages/react-components/react-drawer/src/shared/DrawerBase.types.ts

Line 28 in d35fc26

open?: MotionShorthand<HTMLDivElement>;

Which is basically boolean | Record<X, Y>.

If there is no such requirement now, we will have less constraints in future if we will just expose open: boolean in props.

We can remove, but some partners need to do the overrides to integrate with their apps. We can remove the possibility for overrides right now, but those partners will need to hold a little while we release the motion package to stable as well.

* master: breaking(react-drawer): open now only accepts a boolean instead of MotionShorthand (microsoft#29736) fix (breadcrumb): Overflow examples refactoring (microsoft#29723) refactor(tools): replace deprecated apis (microsoft#29624) [Part 3] Update documentation content for all charts (microsoft#29727) chore: throws if FlatTree is used as a subtree (microsoft#29729) feat(react-storybook-addon-export-to-sandbox): make addon generic so it can be published and use by others (microsoft#29674) chore(TimePicker-compat): add bundle size fixture (microsoft#29717) applying package updates Move CredScanSuppression file to root so 1ESPT output task picks it up (microsoft#29722) fix(CI): increase pipeline agent timeout threshold to 90 minutes (microsoft#29710) fix(breadcrumb): examples and styles (microsoft#29679) chore(react-tree): stop unnecessary re-rendering when no actions are available (microsoft#29694) applying package updates

marcosmoura · 2023-11-02T16:52:54Z

We should not expose APIs from preview components in APIs for stable components:

fluentui/packages/react-components/react-drawer/src/shared/DrawerBase.types.ts

Line 28 in d35fc26

open?: MotionShorthand<HTMLDivElement>;

fluentui/packages/react-components/react-drawer/src/shared/DrawerBase.types.ts

Line 32 in d35fc26

motion: MotionState<HTMLDivElement>;

Please update types to not reference @fluentui/react-motion-preview.

@layershifter @Hotell
I updated the types, and the Drawer code does not expose the Motion API anymore.

Hotell

🚢

layershifter · 2023-11-03T13:52:55Z

@marcosmoura after #29736 we still have:

fluentui/packages/react-components/react-drawer/src/shared/DrawerBase.types.ts

Lines 31 to 33 in 667d05d

    
           export type DrawerBaseState = Required<Pick<DrawerBaseProps, 'position' | 'size'>> & { 
        
             motion: MotionState<HTMLDivElement>; 
        
           };

I mentioned it in the original message, too. Is there motivation to keep it as it? It's not so big issue as with props, but *State types are also a part of public API. Especially, if folks would use composition.

Can you please replace MotionState with { canRender: boolean; type: string } (or something similar) to ensure that we will not have problems with breaking changes?

Hotell · 2023-11-03T16:13:20Z

@marcosmoura after #29736 we still have:

fluentui/packages/react-components/react-drawer/src/shared/DrawerBase.types.ts

Lines 31 to 33 in 667d05d

export type DrawerBaseState = Required<Pick<DrawerBaseProps, 'position' | 'size'>> & {

motion: MotionState<HTMLDivElement>;

};

I mentioned it in the original message, too. Is there motivation to keep it as it? It's not so big issue as with props, but *State types are also a part of public API. Especially, if folks would use composition.

Can you please replace MotionState with { canRender: boolean; type: string } (or something similar) to ensure that we will not have problems with breaking changes?

good catch, I checked only api.md report which while import from that package it doesn't show its leaking to anything public.

welp looks like we cannot rely on that thing :).

@marcosmoura let's refactor it to

-import { MotionState } from '@fluentui/react-motion-preview';
type MotionState = { canRender: boolean; type: string }
export type DrawerBaseState = Required<Pick<DrawerBaseProps, 'position' | 'size'>> & {
-  motion: MotionState<HTMLDivElement>;
+  motion: MotionState;
};

though naked type: string is not the best type decision, because if we introduce strict union in future that would break users on TS level.

While not breaking change on runtime, type checking would explode. -> IMO its not breaking change rather than fix, but we dont provide any kinds of codemods etc which might provide not the best v9 bump for our users.

api changes needed on type level

feat(react-drawer): release to stable

0fc7b30

marcosmoura added the Component: Drawer The Fluent v9 Drawer component label Oct 31, 2023

marcosmoura self-assigned this Oct 31, 2023

marcosmoura requested review from a team as code owners October 31, 2023 18:47

Hotell reviewed Nov 1, 2023

View reviewed changes

packages/react-components/react-drawer/package.json Outdated Show resolved Hide resolved

Hotell reviewed Nov 1, 2023

View reviewed changes

change/@fluentui-react-drawer-3cfc5c74-8b22-419c-a72a-43ecad9c876f.json Outdated Show resolved Hide resolved

Hotell reviewed Nov 1, 2023

View reviewed changes

change/@fluentui-react-components-03e7f032-b0d9-4221-8522-b7bfb9ffb223.json Outdated Show resolved Hide resolved

Hotell reviewed Nov 1, 2023

View reviewed changes

packages/react-components/react-components/src/index.ts Show resolved Hide resolved

layershifter requested changes Nov 1, 2023

View reviewed changes

microsoft-github-policy-service bot added the Needs: Author Feedback label Nov 1, 2023

fix: update changelog message

743405b

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

microsoft-github-policy-service bot removed the Needs: Author Feedback label Nov 1, 2023

marcosmoura and others added 2 commits November 1, 2023 12:59

fix: update changelog message

b2de314

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

fix: disallow prerelease

e1a3ca2

marcosmoura added 3 commits November 1, 2023 13:25

fix: deprecate unstable Drawer exports

54ca55c

fix: improve deprecation message from edge cases

bd20c30

Merge branch 'master' into feat/react-drawer/release-to-stable

b92d855

marcosmoura mentioned this pull request Nov 2, 2023

breaking(react-drawer): open now only accepts a boolean instead of MotionShorthand #29736

Merged

marcosmoura requested review from Hotell and layershifter November 2, 2023 16:50

Merge branch 'master' into feat/react-drawer/release-to-stable

9925776

marcosmoura changed the title ~~feat(react-drawer): release to stable~~ feat(react-drawer): release to 9.0.0 stable Nov 2, 2023

marcosmoura enabled auto-merge (squash) November 2, 2023 18:08

Hotell previously approved these changes Nov 3, 2023

View reviewed changes

fix: avoid leaking types from preview packages

5b068d6

marcosmoura requested a review from Hotell November 6, 2023 11:13

Merge branch 'master' into feat/react-drawer/release-to-stable

2601b36

Hotell approved these changes Nov 6, 2023

View reviewed changes

layershifter approved these changes Nov 6, 2023

View reviewed changes

marcosmoura merged commit d93b104 into microsoft:master Nov 6, 2023

marcosmoura deleted the feat/react-drawer/release-to-stable branch November 15, 2023 15:57

feat(react-drawer): release to 9.0.0 stable #29711

feat(react-drawer): release to 9.0.0 stable #29711

Uh oh!

Conversation

marcosmoura commented Oct 31, 2023

Uh oh!

fabricteam commented Oct 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf Analysis (@fluentui/react-components)

Uh oh!

codesandbox-ci bot commented Oct 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fabricteam commented Oct 31, 2023 • 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 Oct 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Asset size changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

layershifter left a comment • 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!

Hotell commented Nov 1, 2023 • edited by marcosmoura Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

layershifter commented Nov 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Hotell commented Nov 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

layershifter commented Nov 1, 2023

Uh oh!

Hotell commented Nov 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marcosmoura commented Nov 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marcosmoura commented Nov 2, 2023

Uh oh!

Hotell left a comment

Choose a reason for hiding this comment

Uh oh!

layershifter commented Nov 3, 2023

Uh oh!

Hotell commented Nov 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fabricteam commented Oct 31, 2023 •

edited

Loading

Perf Analysis (`@fluentui/react-components`)

codesandbox-ci bot commented Oct 31, 2023 •

edited

Loading

fabricteam commented Oct 31, 2023 •

edited

Loading

size-auditor bot commented Oct 31, 2023 •

edited

Loading

layershifter left a comment •

edited

Loading

Hotell commented Nov 1, 2023 •

edited by marcosmoura

Loading

layershifter commented Nov 1, 2023 •

edited

Loading

Hotell commented Nov 1, 2023 •

edited

Loading

Hotell commented Nov 1, 2023 •

edited

Loading

marcosmoura commented Nov 2, 2023 •

edited

Loading