chore(react-components): migrate to new dx stage-1#19040
chore(react-components): migrate to new dx stage-1#19040Hotell merged 10 commits intomicrosoft:masterfrom
Conversation
| "pretty-bytes": "5.6.0", | ||
| "raw-loader": "4.0.2", | ||
| "react-app-polyfill": "2.0.0", | ||
| "react-test-renderer": "16.8.6", |
There was a problem hiding this comment.
propagating dev dep to single version policy
| "bundle:storybook": "just-scripts storybook:build", | ||
| "bundle-size": "bundle-size measure", | ||
| "chromatic": "npx chromatic@5.6.3 --project-token $CHROMATIC_PROJECT_TOKEN --exit-zero-on-changes --build-script-name bundle:storybook", | ||
| "chromatic": "npx chromatic@5.6.3 --project-token $CHROMATIC_PROJECT_TOKEN --exit-zero-on-changes --build-script-name build-storybook", |
There was a problem hiding this comment.
with this we will ship a proper production builds (build-storybook sets ENV to production
| import { Source } from '@storybook/addon-docs/blocks'; | ||
| import { createRenderer } from 'react-test-renderer/shallow'; | ||
| import { CodeExample } from './utils'; | ||
| import { CodeExample } from './utils.stories'; |
There was a problem hiding this comment.
renamed migrated storybook related files to include stories suffix
📊 Bundle size reportUnchanged fixtures
|
|
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 1eb466e:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 28c579ac8d2f6128d33ed1a3357ca6ba6f2ee7f5 (build) |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Panel | mount | 2062 | 1383 | 1000 | Possible regression |
All results
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 929 | 907 | 5000 | |
| BaseButton | mount | 893 | 880 | 5000 | |
| Breadcrumb | mount | 2671 | 2674 | 1000 | |
| ButtonNext | mount | 438 | 438 | 5000 | |
| Checkbox | mount | 1494 | 1507 | 5000 | |
| CheckboxBase | mount | 1355 | 1272 | 5000 | |
| ChoiceGroup | mount | 4767 | 4685 | 5000 | |
| ComboBox | mount | 963 | 986 | 1000 | |
| CommandBar | mount | 10256 | 10361 | 1000 | |
| ContextualMenu | mount | 6209 | 6339 | 1000 | |
| DefaultButton | mount | 1101 | 1144 | 5000 | |
| DetailsRow | mount | 3662 | 3686 | 5000 | |
| DetailsRowFast | mount | 3689 | 3652 | 5000 | |
| DetailsRowNoStyles | mount | 3517 | 3543 | 5000 | |
| Dialog | mount | 2144 | 2148 | 1000 | |
| DocumentCardTitle | mount | 162 | 135 | 1000 | |
| Dropdown | mount | 3275 | 3213 | 5000 | |
| FluentProviderNext | mount | 7586 | 7512 | 5000 | |
| FocusTrapZone | mount | 1769 | 1795 | 5000 | |
| FocusZone | mount | 1812 | 1804 | 5000 | |
| IconButton | mount | 1741 | 1775 | 5000 | |
| Label | mount | 337 | 345 | 5000 | |
| Layer | mount | 1783 | 1749 | 5000 | |
| Link | mount | 458 | 455 | 5000 | |
| MakeStyles | mount | 1813 | 1834 | 50000 | |
| MenuButton | mount | 1466 | 1465 | 5000 | |
| MessageBar | mount | 2045 | 2060 | 5000 | |
| Nav | mount | 3250 | 3226 | 1000 | |
| OverflowSet | mount | 1117 | 1108 | 5000 | |
| Panel | mount | 2062 | 1383 | 1000 | Possible regression |
| Persona | mount | 836 | 820 | 1000 | |
| Pivot | mount | 1416 | 1425 | 1000 | |
| PrimaryButton | mount | 1282 | 1327 | 5000 | |
| Rating | mount | 7687 | 7542 | 5000 | |
| SearchBox | mount | 1326 | 1309 | 5000 | |
| Shimmer | mount | 2523 | 2510 | 5000 | |
| Slider | mount | 1954 | 1985 | 5000 | |
| SpinButton | mount | 4933 | 5049 | 5000 | |
| Spinner | mount | 420 | 431 | 5000 | |
| SplitButton | mount | 3180 | 3162 | 5000 | |
| Stack | mount | 505 | 508 | 5000 | |
| StackWithIntrinsicChildren | mount | 1556 | 1507 | 5000 | |
| StackWithTextChildren | mount | 4528 | 4515 | 5000 | |
| SwatchColorPicker | mount | 10174 | 10206 | 5000 | |
| Tabs | mount | 1389 | 1399 | 1000 | |
| TagPicker | mount | 2561 | 2618 | 5000 | |
| TeachingBubble | mount | 11895 | 11985 | 5000 | |
| Text | mount | 423 | 409 | 5000 | |
| TextField | mount | 1368 | 1335 | 5000 | |
| ThemeProvider | mount | 1194 | 1216 | 5000 | |
| ThemeProvider | virtual-rerender | 623 | 585 | 5000 | |
| Toggle | mount | 808 | 815 | 5000 | |
| buttonNative | mount | 120 | 119 | 5000 |
Perf Analysis (@fluentui/react-northstar)
Perf tests with no regressions
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| AlertMinimalPerf.default | 287 | 254 | 1.13:1 |
| DividerMinimalPerf.default | 368 | 342 | 1.08:1 |
| ListWith60ListItems.default | 669 | 620 | 1.08:1 |
| RefMinimalPerf.default | 241 | 225 | 1.07:1 |
| BoxMinimalPerf.default | 354 | 336 | 1.05:1 |
| ChatDuplicateMessagesPerf.default | 306 | 291 | 1.05:1 |
| HeaderMinimalPerf.default | 363 | 346 | 1.05:1 |
| SkeletonMinimalPerf.default | 367 | 350 | 1.05:1 |
| AccordionMinimalPerf.default | 149 | 143 | 1.04:1 |
| ImageMinimalPerf.default | 379 | 366 | 1.04:1 |
| ItemLayoutMinimalPerf.default | 1215 | 1166 | 1.04:1 |
| LabelMinimalPerf.default | 390 | 375 | 1.04:1 |
| RosterPerf.default | 1177 | 1128 | 1.04:1 |
| AttachmentMinimalPerf.default | 153 | 149 | 1.03:1 |
| GridMinimalPerf.default | 327 | 319 | 1.03:1 |
| PopupMinimalPerf.default | 602 | 583 | 1.03:1 |
| TableMinimalPerf.default | 405 | 392 | 1.03:1 |
| AnimationMinimalPerf.default | 404 | 397 | 1.02:1 |
| AvatarMinimalPerf.default | 187 | 183 | 1.02:1 |
| ButtonOverridesMissPerf.default | 1709 | 1672 | 1.02:1 |
| ChatWithPopoverPerf.default | 363 | 356 | 1.02:1 |
| DialogMinimalPerf.default | 739 | 727 | 1.02:1 |
| DropdownMinimalPerf.default | 3135 | 3080 | 1.02:1 |
| FlexMinimalPerf.default | 290 | 283 | 1.02:1 |
| ReactionMinimalPerf.default | 368 | 361 | 1.02:1 |
| TreeMinimalPerf.default | 791 | 776 | 1.02:1 |
| VideoMinimalPerf.default | 621 | 610 | 1.02:1 |
| AttachmentSlotsPerf.default | 1084 | 1073 | 1.01:1 |
| CardMinimalPerf.default | 545 | 539 | 1.01:1 |
| ChatMinimalPerf.default | 640 | 634 | 1.01:1 |
| CheckboxMinimalPerf.default | 2744 | 2715 | 1.01:1 |
| DatepickerMinimalPerf.default | 5509 | 5466 | 1.01:1 |
| EmbedMinimalPerf.default | 4112 | 4063 | 1.01:1 |
| FormMinimalPerf.default | 400 | 395 | 1.01:1 |
| InputMinimalPerf.default | 1268 | 1260 | 1.01:1 |
| LayoutMinimalPerf.default | 356 | 353 | 1.01:1 |
| ListCommonPerf.default | 617 | 608 | 1.01:1 |
| RadioGroupMinimalPerf.default | 439 | 436 | 1.01:1 |
| SegmentMinimalPerf.default | 346 | 344 | 1.01:1 |
| SplitButtonMinimalPerf.default | 3777 | 3748 | 1.01:1 |
| StatusMinimalPerf.default | 684 | 676 | 1.01:1 |
| TableManyItemsPerf.default | 1891 | 1865 | 1.01:1 |
| ToolbarMinimalPerf.default | 920 | 915 | 1.01:1 |
| ButtonSlotsPerf.default | 545 | 546 | 1:1 |
| CarouselMinimalPerf.default | 460 | 458 | 1:1 |
| DropdownManyItemsPerf.default | 679 | 678 | 1:1 |
| ListMinimalPerf.default | 500 | 502 | 1:1 |
| LoaderMinimalPerf.default | 682 | 683 | 1:1 |
| PortalMinimalPerf.default | 178 | 178 | 1:1 |
| ProviderMergeThemesPerf.default | 1677 | 1673 | 1:1 |
| ProviderMinimalPerf.default | 988 | 984 | 1:1 |
| SliderMinimalPerf.default | 1549 | 1545 | 1:1 |
| TextMinimalPerf.default | 343 | 344 | 1:1 |
| CustomToolbarPrototype.default | 3834 | 3843 | 1:1 |
| MenuMinimalPerf.default | 831 | 836 | 0.99:1 |
| TextAreaMinimalPerf.default | 480 | 484 | 0.99:1 |
| HeaderSlotsPerf.default | 762 | 778 | 0.98:1 |
| IconMinimalPerf.default | 600 | 615 | 0.98:1 |
| TooltipMinimalPerf.default | 990 | 1011 | 0.98:1 |
| TreeWith60ListItems.default | 169 | 173 | 0.98:1 |
| ListNestedPerf.default | 528 | 545 | 0.97:1 |
| MenuButtonMinimalPerf.default | 1598 | 1655 | 0.97:1 |
| ButtonMinimalPerf.default | 162 | 169 | 0.96:1 |
3267051 to
edfb0fa
Compare
| "dependencies": [ | ||
| "@babel/core", | ||
| "@babel/preset-typescript", | ||
| "@types/react-test-renderer", |
There was a problem hiding this comment.
syncpack opt out behaviour as it doesn't understand single version policy and devDeps - note the version are the same in all packages having this specified as devDep
There was a problem hiding this comment.
If syncpack can be modified to help with that I'm happy to take a look if you'd like to open an issue to explain this use case.
There was a problem hiding this comment.
hey @JamieMason , thanks for chiming in ! Once this is merged and we'll have some extra time , we gonna submit an issue. cheers
Btw, just out of curiosity how did you managed to notice in a draft PR that there was mention on syncpack ? :D
There was a problem hiding this comment.
Great, sounds good 👍🏻
Btw, just out of curiosity how did you managed to notice in a draft PR that there was mention on syncpack ? :D
from github code/issues search it's (admittedly kind of sad but also) a handy way to find out about problems or feature feedback 😄
There was a problem hiding this comment.
haha nice, thanks for sharing!
| "gzip-size", | ||
| "jju", | ||
| "pretty-bytes", | ||
| "react-test-renderer", |
There was a problem hiding this comment.
same as comment above
| @@ -0,0 +1,37 @@ | |||
| import { create } from '@storybook/theming'; | |||
There was a problem hiding this comment.
copied from react-examples/.storybook
There was a problem hiding this comment.
Instead of duplicating the custom styling files, let's just delete them from react-examples since having the customized storybook styling there isn't important.
There was a problem hiding this comment.
It would have been nice to do that together with this change so it would show up as a rename not a new file (at least in git tools that are configured to handle renames)
| }, | ||
| }); | ||
|
|
||
| function getVnextStories() { |
There was a problem hiding this comment.
custom logic to parse for component stories that are part of react-components suite
| @@ -0,0 +1,111 @@ | |||
| <!-- | |||
There was a problem hiding this comment.
- copied from
react-examples/.storybook - updated docs + fixed selectors
| @@ -0,0 +1,8 @@ | |||
| import { addons } from '@storybook/addons'; | |||
There was a problem hiding this comment.
copied from react-examples/.storybook
| /** | ||
| * @see https://storybook.js.org/docs/react/writing-stories/naming-components-and-hierarchy#sorting-stories | ||
| */ | ||
| order: [ |
There was a problem hiding this comment.
partially copied from react-examples/.storybook and used latest SB syntax for "ordering"
| "start": "yarn storybook", | ||
| "docs": "api-extractor run --config=config/api-extractor.local.json --local", | ||
| "build:local": "tsc -p . --module esnext --emitDeclarationOnly && node ../../scripts/typescript/normalize-import --output dist/react-components/src && yarn docs", | ||
| "storybook": "start-storybook --port 3000 -s ./public", |
There was a problem hiding this comment.
we override sb default static assets behaviour -> static assets are consumed from ./public instead of ./static
| @@ -0,0 +1,4 @@ | |||
| <svg width="76" height="28" viewBox="0 0 76 28" fill="none" xmlns="http://www.w3.org/2000/svg"> | |||
There was a problem hiding this comment.
copied from react-examples/.storybook
| const styles: { [className: string]: string }; | ||
| export default styles; | ||
| } | ||
| declare module '*.svg' { |
There was a problem hiding this comment.
importing svg was type error in both react-exampels and react-components
|
/azp run |
|
Pull request contains merge conflicts. |
155b939 to
803e774
Compare
| "baseUrl": ".", | ||
| "target": "ES5", | ||
| "module": "CommonJS", | ||
| "lib": ["ES2016", "dom"], |
ecraig12345
left a comment
There was a problem hiding this comment.
Some comments but generally looks good. I'd strongly prefer that the duplicated theming files be addressed before merging (just delete from react-examples probably).
| @@ -0,0 +1,37 @@ | |||
| import { create } from '@storybook/theming'; | |||
There was a problem hiding this comment.
Instead of duplicating the custom styling files, let's just delete them from react-examples since having the customized storybook styling there isn't important.
| { | ||
| "extends": "../tsconfig.json", | ||
| "compilerOptions": { | ||
| "allowJs": true, |
There was a problem hiding this comment.
Is this basically just to check the JS files in this folder? (Also anything we need to do to avoid checking JS in any node_modules?)
There was a problem hiding this comment.
context/note:
- this is present in every vNext package that uses storybook ( new DX stage 1 ) / https://github.com/microsoft/fluentui/blob/master/tools/generators/migrate-converged-pkg/index.ts#L190
to check the JS files in this folder
- yes this is needed to enable
checkJsto check files within./storybookand to provide best possible DX via intelisense
Also anything we need to do to avoid checking JS in any node_modules
good thinking! fortunately for us node modules are ignored so no need :)

scripts/tasks/api-extractor.ts
Outdated
| }, | ||
| apiExtractorVerifyTask({ configJsonFilePath: configPath, localBuild }), | ||
| () => { | ||
| overrideApi && overrideApi.resetConfig(); |
There was a problem hiding this comment.
I think doing it like this is going to leave the API Extractor config in a modified state if apiExtractorVerifyTask fails.
There was a problem hiding this comment.
true that. I think this shouldn't be a problem on CI ? as if this fails we will bail the pipeline
There was a problem hiding this comment.
Thanks for the update! My concern was more about the local build scenario, since leaving the files in a bad state could confuse people (and maybe let the changes accidentally get checked in).
There was a problem hiding this comment.
that's indeed a good concern and I appreciate pointing that out 🙌.
btw dunno why but after I added that try/catch pipeline started to fail on api-docs generate-json task 🧐...
There was a problem hiding this comment.
btw dunno why but after I added that try/catch pipeline started to fail on api-docs generate-json task 🧐...
ok looks like I figured it out - turns out just scripts are not that intuitive as one would expect... my bad 👀
603686a to
de9f402
Compare
cebf5cc to
cdfcc64
Compare
cdfcc64 to
5552787
Compare
…kages with new DX
3c6ab2e to
1eb466e
Compare
|
🥳 |

Pull request checklist
$ yarn changePre-requirements
Description of changes
react-componentsvia nxmigrate-converged-pkgmigration generatorFluentUIComponents/toConcepts/folder to mirror the stories IDsFollowup - PR will cover
react-make-stylesfrom dependencies #18752 / chore(react-examples): remove all remaining vNext package deps/storybook setup #19456react-examplesstorybook setup / chore(react-examples): remove all remaining vNext package deps/storybook setup #19456TODO:Focus areas to test