-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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: replace @fluentui/make-styles with @griffel/core #21320
chore: replace @fluentui/make-styles with @griffel/core #21320
Conversation
bab5899
to
9e8dd22
Compare
change/@fluentui-babel-make-styles-ee9b3856-f3d4-4861-b40a-5d8804aed9a2.json
Outdated
Show resolved
Hide resolved
change/@fluentui-jest-serializer-make-styles-922bd6c1-bf07-40bf-9ee2-78f5c9e44260.json
Outdated
Show resolved
Hide resolved
change/@fluentui-react-make-styles-cdadc702-0cf1-4498-83d0-a686f679f21f.json
Outdated
Show resolved
Hide resolved
@@ -1,4 +1,4 @@ | |||
import { mergeClasses, makeStyles, createDOMRenderer } from '@fluentui/make-styles'; | |||
import { mergeClasses, makeStyles, createDOMRenderer } from '@griffel/core'; |
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.
This perf test should migrate sooner or later to Griffel repo itself.
@@ -1,4 +1,4 @@ | |||
import { mergeClasses } from '@fluentui/make-styles'; | |||
import { mergeClasses } from '@fluentui/react-make-styles'; |
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.
These tests should use imports from react-*
package as this package have only in in dependencies
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.
don't understand, why can't these tests depend on Griffel ?
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.
oh I see you're wrapping Griffel with react-make-styles
?
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.
oh I see you're wrapping Griffel with react-make-styles?
Yes, as we have many packages I will replace them one by one.
don't understand, why can't these tests depend on Griffel ?
It's mistake that there was @fluentui/make-styles
as it is not in dependencies:
"@fluentui/react-make-styles": "9.0.0-beta.4", |
They should depend on @griffel/react
, but it's not a thing yet.
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 1581cb9:
|
📊 Bundle size reportUnchanged fixtures
|
…yershifter/office-ui-fabric-react into chore/replace-mk-with-griffel
Asset size changes
Over Tolerance (1024 B) Over Baseline Below Baseline New Removed 1 kB = 1000 B Baseline commit: 8dfa712156b70414205b87b5b6d099367b0c297d (build) |
Perf Analysis (
|
Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
---|---|---|---|---|---|
ContextualMenu | mount | 23033 | 7745 | 1000 | Possible regression |
FluentProviderWithTheme | virtual-rerender-with-unmount | 165 | 194 | 10 | Possible regression |
All results
Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
---|---|---|---|---|---|
Avatar | mount | 810 | 777 | 5000 | |
BaseButton | mount | 843 | 824 | 5000 | |
Breadcrumb | mount | 2470 | 2439 | 1000 | |
ButtonNext | mount | 448 | 448 | 5000 | |
Checkbox | mount | 1326 | 1359 | 5000 | |
CheckboxBase | mount | 1129 | 1140 | 5000 | |
ChoiceGroup | mount | 4158 | 4134 | 5000 | |
ComboBox | mount | 873 | 850 | 1000 | |
CommandBar | mount | 9285 | 9284 | 1000 | |
ContextualMenu | mount | 23033 | 7745 | 1000 | Possible regression |
DefaultButton | mount | 1055 | 1019 | 5000 | |
DetailsRow | mount | 3385 | 3309 | 5000 | |
DetailsRowFast | mount | 3339 | 3307 | 5000 | |
DetailsRowNoStyles | mount | 3251 | 3186 | 5000 | |
Dialog | mount | 2223 | 2276 | 1000 | |
DocumentCardTitle | mount | 178 | 169 | 1000 | |
Dropdown | mount | 2847 | 2773 | 5000 | |
FluentProviderNext | mount | 1710 | 1739 | 5000 | |
FluentProviderWithTheme | mount | 167 | 160 | 10 | |
FluentProviderWithTheme | virtual-rerender | 97 | 104 | 10 | |
FluentProviderWithTheme | virtual-rerender-with-unmount | 165 | 194 | 10 | Possible regression |
FocusTrapZone | mount | 1585 | 1653 | 5000 | |
FocusZone | mount | 1633 | 1640 | 5000 | |
IconButton | mount | 1526 | 1565 | 5000 | |
Label | mount | 330 | 337 | 5000 | |
Layer | mount | 2515 | 2596 | 5000 | |
Link | mount | 421 | 447 | 5000 | |
MakeStyles | mount | 1499 | 1499 | 50000 | |
MenuButton | mount | 1338 | 1287 | 5000 | |
MessageBar | mount | 1791 | 1805 | 5000 | |
Nav | mount | 2928 | 2898 | 1000 | |
OverflowSet | mount | 989 | 999 | 5000 | |
Panel | mount | 2261 | 2159 | 1000 | |
Persona | mount | 732 | 782 | 1000 | |
Pivot | mount | 1292 | 1283 | 1000 | |
PrimaryButton | mount | 1169 | 1132 | 5000 | |
Rating | mount | 6633 | 6640 | 5000 | |
SearchBox | mount | 1194 | 1178 | 5000 | |
Shimmer | mount | 2211 | 2246 | 5000 | |
Slider | mount | 1765 | 1714 | 5000 | |
SpinButton | mount | 4372 | 4370 | 5000 | |
Spinner | mount | 389 | 396 | 5000 | |
SplitButton | mount | 2765 | 2770 | 5000 | |
Stack | mount | 470 | 479 | 5000 | |
StackWithIntrinsicChildren | mount | 2031 | 2049 | 5000 | |
StackWithTextChildren | mount | 4498 | 4517 | 5000 | |
SwatchColorPicker | mount | 10822 | 9869 | 5000 | |
TagPicker | mount | 2290 | 2346 | 5000 | |
TeachingBubble | mount | 11292 | 11245 | 5000 | |
Text | mount | 399 | 387 | 5000 | |
TextField | mount | 1212 | 1235 | 5000 | |
ThemeProvider | mount | 1233 | 1055 | 5000 | |
ThemeProvider | virtual-rerender | 540 | 546 | 5000 | |
ThemeProvider | virtual-rerender-with-unmount | 2870 | 1617 | 5000 | |
Toggle | mount | 717 | 706 | 5000 | |
buttonNative | mount | 137 | 141 | 5000 |
Perf Analysis (@fluentui/react-northstar
)
Perf tests with no regressions
Scenario | Current PR Ticks | Baseline Ticks | Ratio |
---|---|---|---|
DropdownMinimalPerf.default | 4299 | 2682 | 1.6:1 |
DatepickerMinimalPerf.default | 6731 | 5403 | 1.25:1 |
HeaderMinimalPerf.default | 347 | 299 | 1.16:1 |
ListWith60ListItems.default | 628 | 543 | 1.16:1 |
AvatarMinimalPerf.default | 191 | 167 | 1.14:1 |
StatusMinimalPerf.default | 602 | 564 | 1.07:1 |
TextMinimalPerf.default | 315 | 298 | 1.06:1 |
VideoMinimalPerf.default | 565 | 535 | 1.06:1 |
AccordionMinimalPerf.default | 160 | 153 | 1.05:1 |
ChatWithPopoverPerf.default | 349 | 332 | 1.05:1 |
ListNestedPerf.default | 494 | 470 | 1.05:1 |
TreeWith60ListItems.default | 167 | 159 | 1.05:1 |
AlertMinimalPerf.default | 243 | 234 | 1.04:1 |
AttachmentSlotsPerf.default | 950 | 911 | 1.04:1 |
DividerMinimalPerf.default | 323 | 312 | 1.04:1 |
CardMinimalPerf.default | 489 | 474 | 1.03:1 |
DropdownManyItemsPerf.default | 595 | 578 | 1.03:1 |
FlexMinimalPerf.default | 253 | 245 | 1.03:1 |
HeaderSlotsPerf.default | 669 | 651 | 1.03:1 |
LoaderMinimalPerf.default | 601 | 581 | 1.03:1 |
SegmentMinimalPerf.default | 313 | 304 | 1.03:1 |
TreeMinimalPerf.default | 703 | 681 | 1.03:1 |
AnimationMinimalPerf.default | 478 | 469 | 1.02:1 |
BoxMinimalPerf.default | 295 | 289 | 1.02:1 |
ButtonSlotsPerf.default | 487 | 476 | 1.02:1 |
DialogMinimalPerf.default | 659 | 644 | 1.02:1 |
GridMinimalPerf.default | 298 | 292 | 1.02:1 |
ImageMinimalPerf.default | 329 | 322 | 1.02:1 |
InputMinimalPerf.default | 1107 | 1086 | 1.02:1 |
LabelMinimalPerf.default | 336 | 331 | 1.02:1 |
RosterPerf.default | 1032 | 1011 | 1.02:1 |
PopupMinimalPerf.default | 541 | 531 | 1.02:1 |
ReactionMinimalPerf.default | 320 | 315 | 1.02:1 |
SkeletonMinimalPerf.default | 306 | 300 | 1.02:1 |
TextAreaMinimalPerf.default | 437 | 427 | 1.02:1 |
CheckboxMinimalPerf.default | 2313 | 2288 | 1.01:1 |
ListCommonPerf.default | 557 | 553 | 1.01:1 |
MenuButtonMinimalPerf.default | 1429 | 1413 | 1.01:1 |
RadioGroupMinimalPerf.default | 398 | 395 | 1.01:1 |
SplitButtonMinimalPerf.default | 3717 | 3667 | 1.01:1 |
TableManyItemsPerf.default | 1630 | 1615 | 1.01:1 |
ToolbarMinimalPerf.default | 820 | 808 | 1.01:1 |
ButtonOverridesMissPerf.default | 1465 | 1466 | 1:1 |
CarouselMinimalPerf.default | 411 | 413 | 1:1 |
ChatMinimalPerf.default | 626 | 628 | 1:1 |
EmbedMinimalPerf.default | 3550 | 3546 | 1:1 |
PortalMinimalPerf.default | 152 | 152 | 1:1 |
ProviderMergeThemesPerf.default | 1481 | 1477 | 1:1 |
RefMinimalPerf.default | 206 | 205 | 1:1 |
SliderMinimalPerf.default | 1414 | 1420 | 1:1 |
TooltipMinimalPerf.default | 889 | 885 | 1:1 |
ProviderMinimalPerf.default | 996 | 1005 | 0.99:1 |
TableMinimalPerf.default | 346 | 351 | 0.99:1 |
CustomToolbarPrototype.default | 3580 | 3613 | 0.99:1 |
ItemLayoutMinimalPerf.default | 1005 | 1025 | 0.98:1 |
LayoutMinimalPerf.default | 311 | 321 | 0.97:1 |
MenuMinimalPerf.default | 733 | 762 | 0.96:1 |
IconMinimalPerf.default | 520 | 547 | 0.95:1 |
FormMinimalPerf.default | 334 | 358 | 0.93:1 |
ListMinimalPerf.default | 456 | 494 | 0.92:1 |
AttachmentMinimalPerf.default | 141 | 157 | 0.9:1 |
ButtonMinimalPerf.default | 154 | 173 | 0.89:1 |
ChatDuplicateMessagesPerf.default | 262 | 325 | 0.81:1 |
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.
LGTM. Any reason we're not using fixed versions anymore?
Good call, I will change this 👍 |
|
||
const boxStyles: React.CSSProperties = { | ||
const boxStyles: ICSSInJSStyle = { |
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.
why were changes to N* necssary ?
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.
PR Changes
This PR is extracted from #21318.
As we agreed to move out and rename
makeStyles
(#20882), it's time to do this ⏳ The migration will be done package by package, this PR replaces@fluentui/make-styles
with@griffel/core
, the next one (#21318) will delete@fluentui/make-styles
at all.The single difference in
@griffel/core
are renamed types:MakeStyles*
=>Griffel*
.Changes in bundle size
This happened because of bump
rtl-css-js
&stylis
: new versions simply contain more code. Affects only runtime version.Changes in N*
csstype
bumpstylis
bump (Fix position:sticky; prefix for Safari browser thysultan/stylis#258)