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

Upgrade Repo to React 17 #24356

Merged
merged 47 commits into from
Aug 22, 2022

Conversation

TristanWatanabe
Copy link
Member

@TristanWatanabe TristanWatanabe commented Aug 12, 2022

Changes:

  • Makes various changes to make v0, v8 and v9 React 17 compatible:
    • Updates react, react-is and react-dom dependencies to 17.0.2.
    • now using React 17 enzyme adapter @wojtekmaj/enzyme-adapter-react-17
    • Adds ES2015.Iterable to v8 packages that target only es5.
    • Replaces usage of deprecated ReactNodeArray in react-utilities with ReactNode[].

Related Issue(s)

Fixes #20145

@fabricteam
Copy link
Collaborator

fabricteam commented Aug 12, 2022

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-accordion
Accordion (including children components)
79.562 kB
24.121 kB
79.349 kB
24.053 kB
-213 B
-68 B
react-avatar
AvatarGroup
15.072 kB
6.016 kB
14.859 kB
5.939 kB
-213 B
-77 B
react-avatar
AvatarGroupItem
68.464 kB
19.067 kB
68.251 kB
18.987 kB
-213 B
-80 B
react-combobox
Combobox (including child components)
72.762 kB
23.757 kB
72.549 kB
23.686 kB
-213 B
-71 B
react-combobox
Dropdown (including child components)
71.026 kB
23.291 kB
70.813 kB
23.222 kB
-213 B
-69 B
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
189.031 kB
51.967 kB
188.818 kB
51.901 kB
-213 B
-66 B
react-dialog
Dialog (including children components)
85.574 kB
25.521 kB
85.361 kB
25.458 kB
-213 B
-63 B
react-menu
Menu (including children components)
115.91 kB
35.382 kB
115.697 kB
35.316 kB
-213 B
-66 B
react-menu
Menu (including selectable components)
119.109 kB
35.876 kB
118.896 kB
35.806 kB
-213 B
-70 B
react-overflow
hooks only
10.898 kB
4.174 kB
10.685 kB
4.104 kB
-213 B
-70 B
react-popover
Popover
103.05 kB
31.561 kB
102.837 kB
31.496 kB
-213 B
-65 B
react-radio
Radio
36.238 kB
12 kB
36.025 kB
11.914 kB
-213 B
-86 B
react-radio
RadioGroup
14.361 kB
5.728 kB
14.148 kB
5.654 kB
-213 B
-74 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
global-context
createContext
533 B
341 B
global-context
createContextSelector
554 B
348 B
priority-overflow
createOverflowManager
2.936 kB
1.212 kB
react-alert
Alert
83.79 kB
20.841 kB
react-avatar
Avatar
48.283 kB
13.644 kB
react-badge
Badge
22.503 kB
7.153 kB
react-badge
CounterBadge
23.406 kB
7.449 kB
react-badge
PresenceBadge
23.947 kB
7.022 kB
react-button
Button
36.396 kB
9.579 kB
react-button
CompoundButton
43.469 kB
10.812 kB
react-button
MenuButton
39.014 kB
10.456 kB
react-button
SplitButton
46.544 kB
11.84 kB
react-button
ToggleButton
51.91 kB
11.003 kB
react-card
Card - All
67.458 kB
19.264 kB
react-card
Card
63.14 kB
18.176 kB
react-card
CardFooter
8.461 kB
3.555 kB
react-card
CardHeader
9.504 kB
3.896 kB
react-card
CardPreview
8.562 kB
3.61 kB
react-components
react-components: FluentProvider & webLightTheme
32.895 kB
10.778 kB
react-divider
Divider
16.359 kB
5.853 kB
react-image
Image
10.68 kB
4.215 kB
react-input
Input
23.554 kB
7.644 kB
react-label
Label
9.238 kB
3.815 kB
react-link
Link
12.231 kB
4.925 kB
react-portal
Portal
10.576 kB
3.875 kB
react-positioning
usePositioning
19.7 kB
7.404 kB
react-provider
FluentProvider
15.655 kB
5.835 kB
react-select
Select
20.746 kB
7.299 kB
react-slider
Slider
32.07 kB
10.033 kB
react-spinbutton
SpinButton
43.899 kB
12.362 kB
react-spinner
Spinner
19.848 kB
6.384 kB
react-switch
Switch
32.562 kB
10.253 kB
react-text
Text - Default
11.682 kB
4.561 kB
react-text
Text - Wrappers
14.992 kB
4.995 kB
react-textarea
Textarea
23.674 kB
7.83 kB
react-theme
Single theme token import
69 B
89 B
react-theme
Teams: all themes
29.224 kB
6.255 kB
react-theme
Teams: Light theme
17.088 kB
4.89 kB
react-tooltip
Tooltip
41.504 kB
14.622 kB
react-utilities
SSRProvider
180 B
159 B
🤖 This report was generated against 16aa65dcae8f75c6a221225fd0eb43800650ac66

@TristanWatanabe
Copy link
Member Author

TristanWatanabe commented Aug 12, 2022

Update: Resolved ✅

How each error was handled:

  1. ⚠ react 17.0.2 missed @cactuslab/usepubsub ^16.9.0 --> Added to satisfied ignore list, @cactuslab/usepubsub does not have react 17 in its peerDeps.
  2. ⚠ react 17.0.2 missed package.json ^16.0.0-0 -> Just needed to remove missed instance of enzyme react 16 adapter
  3. ⚠ react-dom 17.0.2 missed package.json ^16.0.0-0 -> Just needed to remove missed instance of enzyme react 16 adapter
  4. ⚠react 17.0.2 missed storybook-addon-performance ^16.8.0 -> Bumped storybook-addon-performance to version with react 17 in its peerDeps.
  5. ⚠ react-dom 17.0.2 missed package.json ^16.8.0 -> Bumped storybook-addon-performance to version with react 17 in its peerDeps.

Satisfied errors:

devDependencies
  ⚠ react      17.0.2  missed  @cactuslab/usepubsub         ^16.9.0  
  ⚠ react      17.0.2  missed  package.json                 ^16.0.0-0
  ⚠ react-dom  17.0.2  missed  package.json                 ^16.0.0-0
  ⚠ react      17.0.2  missed  storybook-addon-performance  ^16.8.0  
  ⚠ react-dom  17.0.2  missed  package.json                 ^16.8.0  
–––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––
  ✖ 5 ISSUES (5 missed, 0 failed)
–––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––
ℹ Run with `--fix` to resolve issues

azure-pipelines.yml Outdated Show resolved Hide resolved
@TristanWatanabe
Copy link
Member Author

TristanWatanabe commented Aug 12, 2022

Update: Resolved ✅

  • Added @types/react back to resolutions, this time upgraded to 17.0.44

A bunch of typing errors from the installed modules occuring at @fluentui/react-conformance and @fluentui/test-utilities. Typing conflicts for @types/react from react, react-dom and enzyme within installed dependencies. Build error

@fabricteam
Copy link
Collaborator

fabricteam commented Aug 12, 2022

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
TooltipMinimalPerf.default 1892 941 2.01:1
RosterPerf.default 1706 851 2:1
VideoMinimalPerf.default 618 543 1.14:1
IconMinimalPerf.default 568 524 1.08:1
CustomToolbarPrototype.default 2385 2208 1.08:1
ImageMinimalPerf.default 346 324 1.07:1
ChatWithPopoverPerf.default 305 291 1.05:1
ProviderMinimalPerf.default 332 320 1.04:1
ListWith60ListItems.default 509 496 1.03:1
TreeWith60ListItems.default 138 134 1.03:1
AnimationMinimalPerf.default 475 466 1.02:1
AttachmentSlotsPerf.default 884 870 1.02:1
HeaderMinimalPerf.default 325 318 1.02:1
HeaderSlotsPerf.default 690 676 1.02:1
PopupMinimalPerf.default 569 558 1.02:1
SliderMinimalPerf.default 1331 1310 1.02:1
StatusMinimalPerf.default 614 603 1.02:1
ButtonSlotsPerf.default 427 424 1.01:1
DialogMinimalPerf.default 691 687 1.01:1
ItemLayoutMinimalPerf.default 995 988 1.01:1
LabelMinimalPerf.default 343 340 1.01:1
ListMinimalPerf.default 462 459 1.01:1
LoaderMinimalPerf.default 528 521 1.01:1
ReactionMinimalPerf.default 333 331 1.01:1
SegmentMinimalPerf.default 313 310 1.01:1
TableManyItemsPerf.default 1585 1563 1.01:1
AlertMinimalPerf.default 219 220 1:1
ButtonOverridesMissPerf.default 1184 1182 1:1
CardMinimalPerf.default 472 472 1:1
DatepickerMinimalPerf.default 4777 4771 1:1
DropdownMinimalPerf.default 2587 2584 1:1
FlexMinimalPerf.default 255 255 1:1
InputMinimalPerf.default 991 988 1:1
LayoutMinimalPerf.default 316 316 1:1
ListCommonPerf.default 525 526 1:1
MenuButtonMinimalPerf.default 1350 1352 1:1
ProviderMergeThemesPerf.default 987 988 1:1
RefMinimalPerf.default 191 191 1:1
SplitButtonMinimalPerf.default 3300 3292 1:1
TreeMinimalPerf.default 700 703 1:1
CarouselMinimalPerf.default 362 365 0.99:1
ChatDuplicateMessagesPerf.default 224 227 0.99:1
CheckboxMinimalPerf.default 1923 1951 0.99:1
DividerMinimalPerf.default 316 320 0.99:1
DropdownManyItemsPerf.default 557 562 0.99:1
EmbedMinimalPerf.default 2961 3002 0.99:1
MenuMinimalPerf.default 739 750 0.99:1
ToolbarMinimalPerf.default 800 809 0.99:1
AttachmentMinimalPerf.default 130 132 0.98:1
AvatarMinimalPerf.default 162 165 0.98:1
ButtonMinimalPerf.default 144 147 0.98:1
ChatMinimalPerf.default 645 660 0.98:1
ListNestedPerf.default 470 481 0.98:1
SkeletonMinimalPerf.default 289 296 0.98:1
TableMinimalPerf.default 359 365 0.98:1
TextMinimalPerf.default 303 310 0.98:1
TextAreaMinimalPerf.default 412 422 0.98:1
GridMinimalPerf.default 292 300 0.97:1
PortalMinimalPerf.default 139 145 0.96:1
RadioGroupMinimalPerf.default 392 410 0.96:1
BoxMinimalPerf.default 292 311 0.94:1
AccordionMinimalPerf.default 127 136 0.93:1
FormMinimalPerf.default 331 363 0.91:1

@TristanWatanabe
Copy link
Member Author

TristanWatanabe commented Aug 12, 2022

Update: Resolved ✅

@types/react@17 references Iterable which is only available from ES2015 and up. These packages below target es5 only so in order to mitigate, added ES2015.Iterable to the lib portion of their tsconfig files.


Error with @fluentui/theme, @fluentui/react-hooks, @fluentui/style-utilities, @fluentui/react-file-type-icons, @fluentui/foundation-legacy, @fluentui/react-date-time, @fluentui/react-cards:

../../node_modules/@types/react/index.d.ts:232:31 - error TS2304: Cannot find name 'Iterable'.
ERR! 
ERR! 232     type ReactFragment = {} | Iterable<ReactNode>;
ERR!                                   ~~~~~~~~

@fabricteam
Copy link
Collaborator

fabricteam commented Aug 12, 2022

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1596 1639 5000
Button mount 1185 1151 5000
FluentProvider mount 1915 1929 5000
FluentProviderWithTheme mount 710 719 10
FluentProviderWithTheme virtual-rerender 669 649 10
FluentProviderWithTheme virtual-rerender-with-unmount 706 701 10
MakeStyles mount 2245 2241 50000
SpinButton mount 3150 3223 5000

@@ -138,7 +138,7 @@ export type Slots<S extends SlotPropsRecord> = {
};

// @public
export type SlotShorthandValue = React_2.ReactChild | React_2.ReactNodeArray | React_2.ReactPortal;
export type SlotShorthandValue = React_2.ReactChild | React_2.ReactNode[] | React_2.ReactPortal;
Copy link
Member Author

Choose a reason for hiding this comment

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

ReactNodeArray is deprecated and throwing an error, replaced with ReactNode[]

Error was:
@deprecated — Use either ReactNode[] if you need an array or Iterable<ReactNode> if its passed to a host component.

@TristanWatanabe TristanWatanabe requested review from a team as code owners August 17, 2022 21:29
Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

I verified briefly N* parts and it works correctly 👍

Copy link
Member

@ling1726 ling1726 left a comment

Choose a reason for hiding this comment

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

🚀🚀

@@ -18,6 +18,12 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## [Unreleased]

### Breaking
Copy link
Contributor

Choose a reason for hiding this comment

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

@layershifter and @ling1726 , I see you both signed off on this PR so this is probably fine but I just want to confirm that this breaking change is acceptable for N*.

Copy link
Member

Choose a reason for hiding this comment

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

@spmonahan thanks for calling out I overlooked that part, totally missed it 👍

I was not able to find an exact quote from React team, but react-is should be handled as a regular dependency, for example like in MUI: https://github.com/mui/material-ui/blob/dfc0b89c665de8e5e3e4ecf68d8c782e74dbb65c/packages/mui-joy/package.json#L68. I.e. there is no need to have it in peerDependencies.

FYI @TristanWatanabe

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved react-is back to dependencies and just bumped it to version 17.0.2

@TristanWatanabe
Copy link
Member Author

Can someone from @microsoft/fluent-date-time please take a look at this PR? cc: @conniec218

@TristanWatanabe
Copy link
Member Author

@Jahnp @bigbadcapers can one of you sign off at the changes to react-file-type-icons ? :)

@@ -1,4 +1,4 @@
const os = require('os');
// const os = require('os');
Copy link
Member Author

Choose a reason for hiding this comment

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

@spmonahan this was throwing a lint error when trying to merge with master since import is unused so commented it out for now

Copy link
Contributor

Choose a reason for hiding this comment

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

What was the lint error? That package is part of Node so I wouldn't expect any issues.

Copy link
Member Author

Choose a reason for hiding this comment

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

'os' is declared but its value is never read.ts(6133)
'os' is assigned a value but never used.eslint[no-unused-vars](https://eslint.org/docs/rules/no-unused-vars)

@TristanWatanabe TristanWatanabe merged commit c8f9d1e into microsoft:master Aug 22, 2022
@TristanWatanabe TristanWatanabe deleted the react-17-upgrade branch August 22, 2022 17:00
@Zenulous
Copy link

This would be awesome to release asap. We are using Next.js for some projects and at this point React 17+ is really a requirement.

@Hotell Hotell mentioned this pull request Nov 2, 2022
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade our Repo to React 17, tests, VR, examples, etc.