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

Enable eslint-plugin-react-hooks for react-next #14478

Merged
merged 6 commits into from
Aug 13, 2020

Conversation

ecraig12345
Copy link
Member

@ecraig12345 ecraig12345 commented Aug 12, 2020

Pull request checklist

  • Include a change request file using $ yarn change

Description of changes

Enable eslint-plugin-react-hooks for react-next to help catch hook-related mistakes, and fix existing issues. Also add a rule to our eslint config to help catch accidental usage of DOM globals (such as focus() which was accidentally used in SearchBox).

In the interest of getting this merged more quickly, I temporarily disabled the rule in a bunch of places for CalloutContent and SpinButton and will make follow-up issues to fix this.

Also started making some updates to the release notes breaking changes to alphabetize components and add a couple missing things (or notes about components to look over later).

* The left position of the beak
*/
const [beakLeft, setBeakLeft] = React.useState<string>();
return React.useMemo(() => {
Copy link
Member Author

Choose a reason for hiding this comment

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

The parts of the beak position will never change independently, so this is a much more efficient way to calculate and update them.

});
timeoutIds.splice(0, timeoutIds.length); // clear array
Copy link
Member Author

Choose a reason for hiding this comment

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

Without this, the array of timeouts keeps unnecessarily growing (and each one is unnecessarily cleared again) with every resize event

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 12, 2020

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 98de017:

Sandbox Source
Fluent UI Button Configuration
microsoft/fluentui: codesandbox-react-template Configuration
microsoft/fluentui: codesandbox-react-next-template Configuration
microsoft/fluentui: codesandbox-react-northstar-template Configuration

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Aug 12, 2020

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 910 906 5000
ButtonNext mount 585 607 5000
Checkbox mount 1564 1595 5000
CheckboxBase mount 1327 1343 5000
CheckboxNext mount 1730 1730 5000
ChoiceGroup mount 5034 5051 5000
ComboBox mount 929 963 1000
CommandBar mount 7766 7773 1000
ContextualMenu mount 14350 14992 1000
DefaultButton mount 1124 1131 5000
DetailsRow mount 3592 3585 5000
DetailsRowFast mount 3605 3578 5000
DetailsRowNoStyles mount 3345 3476 5000
Dialog mount 1489 1510 1000
DocumentCardTitle mount 1833 1854 1000
Dropdown mount 2661 2679 5000
FocusZone mount 1803 1837 5000
IconButton mount 1735 1799 5000
Label mount 355 338 5000
Link mount 449 460 5000
LinkNext mount 470 474 5000
MenuButton mount 1476 1531 5000
Nav mount 3316 3280 1000
Panel mount 1470 1477 1000
Persona mount 852 874 1000
Pivot mount 1459 1482 1000
PivotNext mount 1431 1382 1000
PrimaryButton mount 1336 1289 5000
SearchBox mount 1302 1313 5000
SearchBoxNext mount 1343 1327 5000
Slider mount 1518 1524 5000
SliderNext mount 1948 1978 5000
SpinButton mount 5012 5255 5000
SpinButtonNext mount 5129 5118 5000
Spinner mount 439 436 5000
SplitButton mount 3117 3449 5000
Stack mount 526 553 5000
StackWithIntrinsicChildren mount 2082 2071 5000
StackWithTextChildren mount 5180 5135 5000
TagPicker mount 2821 2881 5000
Text mount 423 435 5000
TextField mount 1405 1365 5000
ThemeProvider mount 3023 2988 5000
ThemeProvider virtual-rerender 459 458 5000
Toggle mount 853 851 5000
ToggleNext mount 850 838 5000
button mount 132 119 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.44 0.48 0.92:1 2000 874
🦄 Button.Fluent 0.11 0.19 0.58:1 5000 543
🔧 Checkbox.Fluent 0.66 0.35 1.89:1 1000 658
🦄 Dialog.Fluent 0.15 0.22 0.68:1 5000 765
🔧 Dropdown.Fluent 3.09 0.49 6.31:1 1000 3086
🔧 Icon.Fluent 0.14 0.05 2.8:1 5000 723
🦄 Image.Fluent 0.07 0.11 0.64:1 5000 346
🔧 Slider.Fluent 1.64 0.37 4.43:1 1000 1638
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 347
🦄 Tooltip.Fluent 0.11 19.43 0.01:1 5000 548

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
TableMinimalPerf.default 403 380 1.06:1
Button.Fluent 543 513 1.06:1
Text.Fluent 347 326 1.06:1
AttachmentMinimalPerf.default 162 155 1.05:1
FormMinimalPerf.default 394 376 1.05:1
VideoMinimalPerf.default 621 589 1.05:1
Icon.Fluent 723 686 1.05:1
Tooltip.Fluent 548 522 1.05:1
AvatarMinimalPerf.default 470 450 1.04:1
BoxMinimalPerf.default 347 333 1.04:1
PortalMinimalPerf.default 128 123 1.04:1
AnimationMinimalPerf.default 399 386 1.03:1
CarouselMinimalPerf.default 452 438 1.03:1
ChatDuplicateMessagesPerf.default 434 420 1.03:1
ChatWithPopoverPerf.default 483 469 1.03:1
DropdownManyItemsPerf.default 765 742 1.03:1
FlexMinimalPerf.default 278 270 1.03:1
HeaderSlotsPerf.default 777 756 1.03:1
ToolbarMinimalPerf.default 952 924 1.03:1
TooltipMinimalPerf.default 810 788 1.03:1
TreeWith60ListItems.default 233 226 1.03:1
Checkbox.Fluent 658 639 1.03:1
ChatMinimalPerf.default 601 590 1.02:1
EmbedMinimalPerf.default 1956 1909 1.02:1
ImageMinimalPerf.default 367 359 1.02:1
ItemLayoutMinimalPerf.default 1296 1270 1.02:1
ListNestedPerf.default 892 873 1.02:1
ListWith60ListItems.default 1111 1091 1.02:1
ReactionMinimalPerf.default 380 371 1.02:1
SliderMinimalPerf.default 1698 1667 1.02:1
StatusMinimalPerf.default 668 656 1.02:1
IconMinimalPerf.default 652 640 1.02:1
TreeMinimalPerf.default 867 846 1.02:1
AttachmentSlotsPerf.default 1171 1154 1.01:1
DividerMinimalPerf.default 362 360 1.01:1
DropdownMinimalPerf.default 3048 3012 1.01:1
GridMinimalPerf.default 326 324 1.01:1
ListCommonPerf.default 958 951 1.01:1
ListMinimalPerf.default 476 471 1.01:1
SegmentMinimalPerf.default 335 332 1.01:1
TableManyItemsPerf.default 2176 2155 1.01:1
AccordionMinimalPerf.default 152 152 1:1
HierarchicalTreeMinimalPerf.default 406 407 1:1
InputMinimalPerf.default 1341 1340 1:1
PopupMinimalPerf.default 664 667 1:1
ProviderMergeThemesPerf.default 1968 1967 1:1
RefMinimalPerf.default 221 221 1:1
SplitButtonMinimalPerf.default 3846 3845 1:1
CustomToolbarPrototype.default 3871 3880 1:1
Dialog.Fluent 765 768 1:1
Dropdown.Fluent 3086 3084 1:1
CardMinimalPerf.default 546 549 0.99:1
LayoutMinimalPerf.default 382 384 0.99:1
MenuButtonMinimalPerf.default 1544 1557 0.99:1
ProviderMinimalPerf.default 949 963 0.99:1
RadioGroupMinimalPerf.default 404 407 0.99:1
TextMinimalPerf.default 333 336 0.99:1
TextAreaMinimalPerf.default 454 457 0.99:1
CheckboxMinimalPerf.default 2884 2949 0.98:1
DialogMinimalPerf.default 768 781 0.98:1
MenuMinimalPerf.default 862 878 0.98:1
SkeletonMinimalPerf.default 400 409 0.98:1
Image.Fluent 346 354 0.98:1
Slider.Fluent 1638 1665 0.98:1
ButtonMinimalPerf.default 167 172 0.97:1
ButtonSlotsPerf.default 580 598 0.97:1
HeaderMinimalPerf.default 349 358 0.97:1
Avatar.Fluent 874 900 0.97:1
LoaderMinimalPerf.default 726 755 0.96:1
LabelMinimalPerf.default 385 404 0.95:1
AlertMinimalPerf.default 284 307 0.93:1

@size-auditor
Copy link

size-auditor bot commented Aug 12, 2020

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-next-Coachmark 88.093 kB 88.182 kB ExceedsBaseline     89 bytes
office-ui-fabric-react fluentui-react-next-SearchBox 177.741 kB 177.76 kB ExceedsBaseline     19 bytes
office-ui-fabric-react fluentui-react-next-Callout 78.406 kB 78.423 kB ExceedsBaseline     17 bytes
office-ui-fabric-react fluentui-react-next-Toggle 47.589 kB 47.594 kB ExceedsBaseline     5 bytes
office-ui-fabric-react fluentui-react-next-Fabric 38.632 kB 38.636 kB ExceedsBaseline     4 bytes
office-ui-fabric-react fluentui-react-next-Slider 51.764 kB 51.766 kB ExceedsBaseline     2 bytes
office-ui-fabric-react fluentui-react-next-Checkbox 46.157 kB 46.159 kB ExceedsBaseline     2 bytes
office-ui-fabric-react fluentui-react-next-Link 38.215 kB 38.216 kB ExceedsBaseline     1 bytes
office-ui-fabric-react fluentui-react-next-ContextualMenu 145.566 kB 145.559 kB BelowBaseline     -7 bytes
office-ui-fabric-react fluentui-react-next-Popup 11.42 kB 11.404 kB BelowBaseline     -16 bytes
office-ui-fabric-react fluentui-react-next-Modal 90.551 kB 90.535 kB BelowBaseline     -16 bytes
office-ui-fabric-react fluentui-react-next-PersonaCoin 114.2 kB 114.122 kB BelowBaseline     -78 bytes
office-ui-fabric-react fluentui-react-next-Image 42.131 kB 42.053 kB BelowBaseline     -78 bytes
office-ui-fabric-react fluentui-react-next-Persona 114.2 kB 114.122 kB BelowBaseline     -78 bytes
office-ui-fabric-react fluentui-react-next-SelectedItemsList 268.041 kB 267.956 kB BelowBaseline     -85 bytes

ExceedsTolerance Over Tolerance (1024 B) ExceedsBaseline Over Baseline BelowBaseline Below Baseline New New Deleted  Removed 1 kB = 1000 B

Baseline commit: e34570d86cd27872c5b76c06022757afbaf22707 (build)

@ecraig12345 ecraig12345 merged commit 673554b into microsoft:master Aug 13, 2020
@ecraig12345 ecraig12345 deleted the eslint-hooks-next branch August 13, 2020 00:00
@msft-github-bot
Copy link
Contributor

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@JustSlone JustSlone added this to the v8: Pre-snap milestone Aug 21, 2020
levithomason pushed a commit to levithomason/fluentui that referenced this pull request Aug 24, 2020
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