chore(make-styles): migrate to new DX by layershifter · Pull Request #18673 · microsoft/fluentui

layershifter · 2021-06-23T09:38:37Z

Pull request checklist

Addresses an existing issue:
- related to migration: migrate converged packages to new DX (1st stage) #18579
- fixes tools: after nx migration build:local does not work #18675
Include a change request file using $ yarn change

Description of changes

Migration of package to new DX.

Changes that were done:

.storybook directory as scripts were removed as this package does not have stories
tsconfig.json#compilerOptions.lib setting was modified to previous as this package uses ES2015 features
jest.config.js#snapshotSerializers & jest.config.js#setupFilesAfterEnv settings were removed

size-auditor · 2021-06-23T09:49:36Z

Asset size changes

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

Baseline commit: 5a6dd90efb20b162aa3883c34faa211b5620b334 (build)

layershifter · 2021-06-23T09:52:13Z

I tested commands after migration and it looks that build:local does not work as expected 😥 I created an issue #18675.

fabricteam · 2021-06-23T10:21:25Z

Perf Analysis (`@fluentui/react`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	805	825	5000
BaseButton	mount	949	896	5000
Breadcrumb	mount	2616	2643	1000
ButtonNext	mount	505	513	5000
Checkbox	mount	1531	1474	5000
CheckboxBase	mount	1252	1252	5000
ChoiceGroup	mount	4603	4642	5000
ComboBox	mount	1063	992	1000
CommandBar	mount	10099	10184	1000
ContextualMenu	mount	6339	6215	1000
DefaultButton	mount	1127	1155	5000
DetailsRow	mount	3700	3652	5000
DetailsRowFast	mount	3794	3711	5000
DetailsRowNoStyles	mount	3425	3507	5000
Dialog	mount	2121	2103	1000
DocumentCardTitle	mount	132	148	1000
Dropdown	mount	3244	3134	5000
FocusTrapZone	mount	1767	1791	5000
FocusZone	mount	1740	1754	5000
IconButton	mount	1661	1754	5000
Label	mount	351	340	5000
Layer	mount	1750	1794	5000
Link	mount	477	449	5000
MakeStyles	mount	1813	1833	50000
MenuButton	mount	1415	1447	5000
MessageBar	mount	2022	1993	5000
Nav	mount	3147	3175	1000
OverflowSet	mount	1041	1010	5000
Panel	mount	2032	2030	1000
Persona	mount	789	788	1000
Pivot	mount	1396	1366	1000
PrimaryButton	mount	1272	1263	5000
Rating	mount	7578	7449	5000
SearchBox	mount	1324	1312	5000
Shimmer	mount	2488	2477	5000
Slider	mount	1923	1930	5000
SpinButton	mount	4841	4878	5000
Spinner	mount	413	404	5000
SplitButton	mount	3152	3077	5000
Stack	mount	511	501	5000
StackWithIntrinsicChildren	mount	1537	1559	5000
StackWithTextChildren	mount	4453	4465	5000
SwatchColorPicker	mount	10167	10090	5000
Tabs	mount	1402	1370	1000
TagPicker	mount	2411	2426	5000
TeachingBubble	mount	11705	11679	5000
Text	mount	426	403	5000
TextField	mount	1349	1340	5000
ThemeProvider	mount	1166	1184	5000
ThemeProvider	virtual-rerender	601	593	5000
ThemeProviderNext	mount	7098	7117	5000
Toggle	mount	792	767	5000
buttonNative	mount	120	109	5000

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

Perf tests with no regressions

Scenario	Current PR Ticks	Baseline Ticks	Ratio
AttachmentMinimalPerf.default	155	141	1.1:1
IconMinimalPerf.default	611	564	1.08:1
ListWith60ListItems.default	661	619	1.07:1
CarouselMinimalPerf.default	466	439	1.06:1
ListCommonPerf.default	639	601	1.06:1
ListMinimalPerf.default	525	500	1.05:1
TableMinimalPerf.default	407	387	1.05:1
AlertMinimalPerf.default	290	278	1.04:1
AvatarMinimalPerf.default	199	191	1.04:1
DialogMinimalPerf.default	767	738	1.04:1
HeaderSlotsPerf.default	763	732	1.04:1
ChatDuplicateMessagesPerf.default	289	280	1.03:1
DropdownManyItemsPerf.default	677	659	1.03:1
MenuMinimalPerf.default	848	823	1.03:1
SkeletonMinimalPerf.default	352	343	1.03:1
StatusMinimalPerf.default	658	641	1.03:1
TooltipMinimalPerf.default	977	944	1.03:1
ChatMinimalPerf.default	646	631	1.02:1
CheckboxMinimalPerf.default	2711	2661	1.02:1
DatepickerMinimalPerf.default	5313	5197	1.02:1
ImageMinimalPerf.default	364	358	1.02:1
PortalMinimalPerf.default	173	170	1.02:1
RadioGroupMinimalPerf.default	453	443	1.02:1
SegmentMinimalPerf.default	345	339	1.02:1
SliderMinimalPerf.default	1566	1528	1.02:1
TextMinimalPerf.default	341	334	1.02:1
AccordionMinimalPerf.default	146	144	1.01:1
AttachmentSlotsPerf.default	1034	1022	1.01:1
DropdownMinimalPerf.default	3029	3012	1.01:1
ItemLayoutMinimalPerf.default	1188	1171	1.01:1
LabelMinimalPerf.default	373	370	1.01:1
PopupMinimalPerf.default	558	555	1.01:1
ReactionMinimalPerf.default	363	361	1.01:1
SplitButtonMinimalPerf.default	3682	3656	1.01:1
ButtonMinimalPerf.default	158	158	1:1
ChatWithPopoverPerf.default	355	354	1:1
DividerMinimalPerf.default	349	350	1:1
FlexMinimalPerf.default	276	277	1:1
GridMinimalPerf.default	332	333	1:1
HeaderMinimalPerf.default	343	343	1:1
LoaderMinimalPerf.default	681	684	1:1
ProviderMinimalPerf.default	992	996	1:1
CustomToolbarPrototype.default	3747	3763	1:1
TreeMinimalPerf.default	765	764	1:1
AnimationMinimalPerf.default	402	406	0.99:1
BoxMinimalPerf.default	343	345	0.99:1
ButtonOverridesMissPerf.default	1638	1662	0.99:1
EmbedMinimalPerf.default	4071	4094	0.99:1
FormMinimalPerf.default	389	391	0.99:1
LayoutMinimalPerf.default	353	355	0.99:1
ListNestedPerf.default	541	545	0.99:1
MenuButtonMinimalPerf.default	1541	1559	0.99:1
ProviderMergeThemesPerf.default	1631	1642	0.99:1
TableManyItemsPerf.default	1872	1888	0.99:1
ToolbarMinimalPerf.default	907	916	0.99:1
CardMinimalPerf.default	536	547	0.98:1
InputMinimalPerf.default	1247	1277	0.98:1
ButtonSlotsPerf.default	536	555	0.97:1
RefMinimalPerf.default	223	230	0.97:1
TextAreaMinimalPerf.default	459	471	0.97:1
TreeWith60ListItems.default	171	179	0.96:1
VideoMinimalPerf.default	595	622	0.96:1
RosterPerf.default	1102	1172	0.94:1

Hotell · 2021-06-23T20:07:23Z

tsconfig.json#compilerOptions.lib setting was modified to previous as this package uses ES2015 features

@ling1726 can you reference this PR as well in the issue -> It touches what we talked about today regarding language feature parity misalignment. thx!

packages/make-styles/tsconfig.json

packages/make-styles/package.json

Hotell · 2021-06-23T20:19:18Z

jest.config.js#snapshotSerializers & jest.config.js#setupFilesAfterEnv settings were removed

would you be willing to implement those behaviours (to not include those everywhere + not include storybook when its a node package) to the generator ?

Hotell · 2021-06-23T20:20:10Z

nx.json

    "@fluentui/jest-serializer-merge-styles": { "implicitDependencies": [] },
    "@fluentui/keyboard-key": { "implicitDependencies": [] },
-    "@fluentui/make-styles": { "implicitDependencies": [] },
+    "@fluentui/make-styles": { "tags": ["vNext", "platform:web"], "implicitDependencies": [] },


to double check the context boundary, is this package solely for web platform or it can be used also within node ?

It's a web package, some functionality is related to SSR, but I am not considering this usage as within Node.

kk, for better context

platform:web - package used on web

platform:node - package used with node

platform:any - package that can be used in both

We'll enable lint rule that will respect those contexts and staticly defined rules in the future to prevent for example importing node package into web package

packages/make-styles/config/api-extractor.local.json

packages/make-styles/package.json

layershifter · 2021-06-24T12:04:11Z

would you be willing to implement those behaviours (to not include those everywhere + not include storybook when its a node package) to the generator ?

For `jest.config.js`

I don't think that it's related to node/web, I created an issue #18706, please take a look when you will have time.

For storybook

Honestly I am not entirely sure about this. For example, this package will not have stories ever as all of them are in @fluentui/react-make-styles that wraps core functionality from this package. There are only two packages Node-only packages (babel-make-styles and jest-serializer-make-styles), IMO during migration of them we can just delete .storybook and related commands for them. WDYT?

Hotell · 2021-06-25T10:53:43Z

For storybook

While I agree in general, to provide best possible DX for folks migrating this would help a lot without additional cognitive load needed.

Hotell

🙌 thx!

…hore/make-styles-dx � Conflicts: � jest.config.js

msft-fluent-ui-bot · 2021-06-28T07:39:14Z

🎉@fluentui/react-examples@v8.33.0 has been released which incorporates this pull request.:tada:

Handy links:

layershifter added 3 commits June 23, 2021 11:31

chore(make-styles): migrate to new DX

167524e

Change files

c7476d2

Change files

3671cad

layershifter requested a review from a team as a code owner June 23, 2021 09:38

msft-fluent-ui-bot assigned miroslavstastny Jun 23, 2021

layershifter added the Area: Build System label Jun 23, 2021

layershifter requested a review from Hotell June 23, 2021 09:39

layershifter mentioned this pull request Jun 23, 2021

tools: after nx migration build:local does not work #18675

Closed

remove setupFilesAfterEnv

3fb24f3

ling1726 approved these changes Jun 23, 2021

View reviewed changes

Hotell reviewed Jun 23, 2021

View reviewed changes

packages/make-styles/tsconfig.json Outdated Show resolved Hide resolved

packages/make-styles/package.json Outdated Show resolved Hide resolved

Hotell reviewed Jun 23, 2021

View reviewed changes

packages/make-styles/config/api-extractor.local.json Outdated Show resolved Hide resolved

packages/make-styles/package.json Outdated Show resolved Hide resolved

ling1726 mentioned this pull request Jun 24, 2021

converged packages need ES2015: API extractor warns ES5 incompatible features in some packages #18682

Closed

layershifter mentioned this pull request Jun 24, 2021

migration: migrate converged packages to new DX (1st stage) #18579

Closed

32 tasks

layershifter added 2 commits June 24, 2021 12:14

apply review suggestions

5171cfa

fix "build:local" command

35a91c8

layershifter mentioned this pull request Jun 24, 2021

tools: options in jest.config.js should be added on demand #18706

Closed

Hotell approved these changes Jun 25, 2021

View reviewed changes

Merge branch 'master' of https://github.com/microsoft/fluentui into c…

549ead9

…hore/make-styles-dx � Conflicts: � jest.config.js

layershifter enabled auto-merge (squash) June 25, 2021 11:22

layershifter merged commit bf6fd12 into master Jun 25, 2021

layershifter deleted the chore/make-styles-dx branch June 25, 2021 12:25

Hotell mentioned this pull request Jun 28, 2021

tools: "test:watch" is not handled by a generator #18742

Closed

Conversation

layershifter commented Jun 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request checklist

Description of changes

Uh oh!

size-auditor bot commented Jun 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Asset size changes

Uh oh!

layershifter commented Jun 23, 2021

Uh oh!

fabricteam commented Jun 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf Analysis (@fluentui/react)

Perf Analysis (@fluentui/react-northstar)

Uh oh!

Hotell commented Jun 23, 2021

Uh oh!

Uh oh!

Uh oh!

Hotell commented Jun 23, 2021

Uh oh!

Hotell Jun 23, 2021

Choose a reason for hiding this comment

Uh oh!

layershifter Jun 24, 2021

Choose a reason for hiding this comment

Uh oh!

Hotell Jun 25, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

layershifter commented Jun 24, 2021

For jest.config.js

For storybook

Uh oh!

Hotell commented Jun 25, 2021

Uh oh!

Hotell left a comment

Choose a reason for hiding this comment

Uh oh!

msft-fluent-ui-bot commented Jun 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

layershifter commented Jun 23, 2021 •

edited

Loading

size-auditor bot commented Jun 23, 2021 •

edited

Loading

fabricteam commented Jun 23, 2021 •

edited

Loading

Perf Analysis (`@fluentui/react`)

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

For `jest.config.js`