Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: migrate @fluentui/react-components #20749

Merged
merged 15 commits into from
Nov 26, 2021
Merged

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Nov 24, 2021

Pull request checklist

Description of changes

@layershifter layershifter requested review from a team as code owners November 24, 2021 11:47
@layershifter layershifter requested a review from a team November 24, 2021 11:47
@fabricteam
Copy link
Collaborator

fabricteam commented Nov 24, 2021

📊 Bundle size report

🤖 This report was generated against ecdea9fb8c041b82997e1b6f6b8be49882dbd9f2

@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 24, 2021

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

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

@layershifter
Copy link
Member Author

layershifter commented Nov 24, 2021

image

A bunch of nice issues 😀

^ kinda solved

@size-auditor
Copy link

size-auditor bot commented Nov 24, 2021

Asset size changes

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

Baseline commit: ecdea9fb8c041b82997e1b6f6b8be49882dbd9f2 (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Nov 24, 2021

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1176 1170 5000
BaseButton mount 1228 1236 5000
Breadcrumb mount 2959 2946 1000
ButtonNext mount 726 746 5000
Checkbox mount 1871 1903 5000
CheckboxBase mount 1584 1657 5000
ChoiceGroup mount 5564 5439 5000
ComboBox mount 1269 1240 1000
CommandBar mount 10825 10882 1000
ContextualMenu mount 9149 9182 1000
DefaultButton mount 1417 1453 5000
DetailsRow mount 4341 4308 5000
DetailsRowFast mount 4281 4438 5000
DetailsRowNoStyles mount 4081 4081 5000
Dialog mount 2788 2861 1000
DocumentCardTitle mount 338 379 1000
Dropdown mount 3704 3740 5000
FluentProviderNext mount 4294 4229 5000
FluentProviderWithTheme mount 413 393 10
FluentProviderWithTheme virtual-rerender 270 269 10
FluentProviderWithTheme virtual-rerender-with-unmount 457 432 10
FocusTrapZone mount 2083 2155 5000
FocusZone mount 2126 2126 5000
IconButton mount 2176 2155 5000
Label mount 516 532 5000
Layer mount 3366 3330 5000
Link mount 692 699 5000
MakeStyles mount 2103 2109 50000
MenuButton mount 1831 1847 5000
MessageBar mount 2236 2289 5000
Nav mount 3757 3836 1000
OverflowSet mount 1349 1337 5000
Panel mount 2746 2756 1000
Persona mount 1065 1094 1000
Pivot mount 1715 1703 1000
PrimaryButton mount 1571 1571 5000
Rating mount 8932 8934 5000
SearchBox mount 1665 1683 5000
Shimmer mount 3023 3024 5000
Slider mount 2297 2315 5000
SpinButton mount 5684 5538 5000
Spinner mount 613 621 5000
SplitButton mount 3622 3711 5000
Stack mount 749 759 5000
StackWithIntrinsicChildren mount 2161 2146 5000
StackWithTextChildren mount 5475 5505 5000
SwatchColorPicker mount 11627 11841 5000
TagPicker mount 3089 3104 5000
TeachingBubble mount 13907 13918 5000
Text mount 640 656 5000
TextField mount 1737 1738 5000
ThemeProvider mount 1442 1440 5000
ThemeProvider virtual-rerender 824 826 5000
ThemeProvider virtual-rerender-with-unmount 2323 2344 5000
Toggle mount 1064 1068 5000
buttonNative mount 298 300 5000

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ChatWithPopoverPerf.default 471 429 1.1:1
RadioGroupMinimalPerf.default 531 484 1.1:1
TextMinimalPerf.default 410 373 1.1:1
CardMinimalPerf.default 662 619 1.07:1
RefMinimalPerf.default 262 246 1.07:1
DividerMinimalPerf.default 431 408 1.06:1
ReactionMinimalPerf.default 444 417 1.06:1
ListCommonPerf.default 760 726 1.05:1
SegmentMinimalPerf.default 423 403 1.05:1
TreeWith60ListItems.default 219 209 1.05:1
AttachmentSlotsPerf.default 1211 1169 1.04:1
ButtonMinimalPerf.default 210 202 1.04:1
GridMinimalPerf.default 391 376 1.04:1
LabelMinimalPerf.default 437 419 1.04:1
AnimationMinimalPerf.default 474 460 1.03:1
AttachmentMinimalPerf.default 182 177 1.03:1
HeaderMinimalPerf.default 415 402 1.03:1
HeaderSlotsPerf.default 871 847 1.03:1
ListWith60ListItems.default 731 707 1.03:1
PopupMinimalPerf.default 643 624 1.03:1
IconMinimalPerf.default 710 687 1.03:1
AlertMinimalPerf.default 317 311 1.02:1
ButtonOverridesMissPerf.default 1896 1861 1.02:1
ChatDuplicateMessagesPerf.default 357 349 1.02:1
ChatMinimalPerf.default 783 771 1.02:1
FlexMinimalPerf.default 329 321 1.02:1
InputMinimalPerf.default 1436 1407 1.02:1
LoaderMinimalPerf.default 751 734 1.02:1
TableManyItemsPerf.default 2192 2141 1.02:1
TableMinimalPerf.default 458 450 1.02:1
TooltipMinimalPerf.default 1161 1139 1.02:1
TreeMinimalPerf.default 906 884 1.02:1
ButtonSlotsPerf.default 626 617 1.01:1
DatepickerMinimalPerf.default 5932 5854 1.01:1
DialogMinimalPerf.default 845 833 1.01:1
FormMinimalPerf.default 476 470 1.01:1
ItemLayoutMinimalPerf.default 1342 1332 1.01:1
LayoutMinimalPerf.default 406 400 1.01:1
ListNestedPerf.default 635 627 1.01:1
MenuMinimalPerf.default 936 923 1.01:1
MenuButtonMinimalPerf.default 1828 1803 1.01:1
RosterPerf.default 1347 1338 1.01:1
SliderMinimalPerf.default 1855 1835 1.01:1
SplitButtonMinimalPerf.default 4774 4709 1.01:1
StatusMinimalPerf.default 778 768 1.01:1
TextAreaMinimalPerf.default 578 575 1.01:1
AvatarMinimalPerf.default 228 229 1:1
BoxMinimalPerf.default 401 400 1:1
CarouselMinimalPerf.default 543 542 1:1
CheckboxMinimalPerf.default 2885 2872 1:1
DropdownManyItemsPerf.default 783 785 1:1
DropdownMinimalPerf.default 3180 3187 1:1
EmbedMinimalPerf.default 4577 4600 1:1
ImageMinimalPerf.default 432 434 1:1
ListMinimalPerf.default 575 575 1:1
ProviderMergeThemesPerf.default 1808 1814 1:1
CustomToolbarPrototype.default 4357 4365 1:1
VideoMinimalPerf.default 728 731 1:1
ProviderMinimalPerf.default 1231 1246 0.99:1
SkeletonMinimalPerf.default 398 404 0.99:1
ToolbarMinimalPerf.default 1053 1073 0.98:1
PortalMinimalPerf.default 187 192 0.97:1
AccordionMinimalPerf.default 177 185 0.96:1

@@ -6,13 +6,16 @@ import { PositioningProps } from '@fluentui/react-positioning';
export const PopperImperativeHandle = () => {
const [loading, setLoading] = React.useState(true);
const popperRef: PositioningProps['popperRef'] = React.useRef({ updatePosition: () => null });
const timeoutRef = React.useRef(0);
const timeoutRef = React.useRef<ReturnType<typeof setTimeout>>();
Copy link
Member Author

@layershifter layershifter Nov 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hotell Now Node typings are leaking to stories, not sure that it's okay 😯 Is it happens because non all packages were migrated yet?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so because the custom logic that is being provided in root /.storybook and in react-components - (main.utils.js), which uses node APIs, node typings are being included to the checking program ( which is how TS works ). At this point there is no functionality that allows us to prevent that.

Only solution is to be explicit and stop doing antipatterns like accesing globals directly.

Instead:

timeoutRef.current = setTimeout(() => setLoading(false), 1000);

Do:

timeoutRef.current = window.setTimeout(() => setLoading(false), 1000);

This will resolve the type issues you're having.


Another thingy:

Now because I introduced tests in react-components/.storybook , we would need to add jest glboals to react-components/.storybook/tsconfig.json. That will pollute all stories, thus allowing anyone to use jest globals in stories. I don't think that's the proper solution ATM, so instead we can do following for that test file:

@filename react-components/.storybook/main.utils.test.js

+/// <reference types="jest" />
const utils = require('./main.utils');

describe(`main utils`, () => {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is using <reference types="jest" /> is a practice that we should follow in such cases?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideal scenario is to have this in separate package (logic,tests) with proper globals set. but as a temporary workaround/corner case I think so, yes.

Why?

  • tests are not shipped not exported so not used in some other context, thus it is ok to specify local globals inline, if not specified on package level

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also going forward ( once we migrate to jest 27 ) we should stop using globals as jest provides imports for their API ( one less global in our lives yay )

@layershifter
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Contributor

@Hotell Hotell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to the moon ! 🙌 thx !

@layershifter layershifter merged commit f9cf2e6 into master Nov 26, 2021
@layershifter layershifter deleted the chore/migrate-rc branch November 26, 2021 17:13
mlp73 pushed a commit to mlp73/fluentui that referenced this pull request Jan 17, 2022
* chore: migrate @fluentui/react-components

* Change files

* restore script

* restore changes

* fix config

* update configs to include both JS & TS

* fix typings errors

* fix SB config

* bump pkg version

* Apply suggestions from code review

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

* fix fmt

* run generator once more

* apply review suggestions

Co-authored-by: Martin Hochel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants