-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Upgrade Repo to React 17 #24356
Conversation
📊 Bundle size reportUnchanged fixtures
|
Update: Resolved ✅ How each error was handled:
Satisfied errors:
|
71b0476
to
08d2484
Compare
Update: Resolved ✅
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 |
Perf Analysis (
|
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 |
packages/react-components/react-utilities/src/compose/resolveShorthand.ts
Outdated
Show resolved
Hide resolved
Update: Resolved ✅
Error with
|
Perf Analysis (
|
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; |
There was a problem hiding this comment.
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.
There was a problem hiding this 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 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀🚀
packages/fluentui/CHANGELOG.md
Outdated
@@ -18,6 +18,12 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm | |||
|
|||
## [Unreleased] | |||
|
|||
### Breaking |
There was a problem hiding this comment.
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*.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
Can someone from @microsoft/fluent-date-time please take a look at this PR? cc: @conniec218 |
@Jahnp @bigbadcapers can one of you sign off at the changes to |
be89cf0
to
05beafb
Compare
@@ -1,4 +1,4 @@ | |||
const os = require('os'); | |||
// const os = require('os'); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
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. |
Changes:
react
,react-is
andreact-dom
dependencies to17.0.2
.@wojtekmaj/enzyme-adapter-react-17
ES2015.Iterable
to v8 packages that target only es5.ReactNodeArray
inreact-utilities
withReactNode[]
.Related Issue(s)
Fixes #20145