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.

Sign up for GitHub

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

Jump to bottom

fix(babel-make-styles): update regex for rules #18816

Merged
merged 2 commits into from Jul 2, 2021
Merged

fix(babel-make-styles): update regex for rules #18816

merged 2 commits into from Jul 2, 2021

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Jul 2, 2021 •
edited

Pull request checklist

Description of changes

In #18801 we noticed inconsistent behavior of @linaria/shaker: after debugging the issue I realized that it is executed for different files based on OS.

matchedRules in module.ts were different:
https://github.com/callstack/linaria/blob/e92bcd93ed51b847cab0c9e3fa7c578328c1ddaf/packages/babel/src/module.ts#L296-L317

Ubuntu
office-ui-fabric-react/node_modules/inline-style-expand-shorthand/lib/expandProperty.js
matchedRules [
  { test: /node_modules/, action: 'ignore' },
  { action: [Function: shaker] }
]
office-ui-fabric-react/node_modules/inline-style-expand-shorthand/lib/expandWithMerge.js
matchedRules [
  { test: /node_modules/, action: 'ignore' },
  { action: [Function: shaker] }
]
Windows
office-ui-fabric-react\node_modules\inline-style-expand-shorthand\lib\expandProperty.js
matchedRules [
  { action: [Function: shaker] }
]
office-ui-fabric-react\node_modules\inline-style-expand-shorthand\lib\expandWithMerge.js
matchedRules [
  { action: [Function: shaker] }
]

This happened because regex that we were using did not take into account slashes difference:

fluentui/packages/babel-make-styles/src/utils/evaluatePathsInVM.ts

Lines 20 to 23 in 6c37a1c

{
test: /\/node_modules\//,
action: 'ignore',
},

In Linaria's code a proper regex is used: https://github.com/callstack/linaria/blob/e92bcd93ed51b847cab0c9e3fa7c578328c1ddaf/packages/babel/src/utils/loadOptions.ts#L31

@layershifter layershifter mentioned this pull request Jul 2, 2021
Closed
@layershifter layershifter marked this pull request as ready for review July 2, 2021 13:07
@layershifter layershifter requested a review from a team July 2, 2021 13:08
This was referenced Jul 2, 2021
Closed
Closed
@size-auditor
Copy link

size-auditor bot commented Jul 2, 2021

Asset size changes

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

Baseline commit: 6c37a1cb5c312d4be8b239bfd9f6c9f28e9f0d24 (build)

@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 921 841 5000
BaseButton mount 988 975 5000
Breadcrumb mount 2716 2708 1000
ButtonNext mount 553 560 5000
Checkbox mount 1665 1670 5000
CheckboxBase mount 1484 1481 5000
ChoiceGroup mount 5164 5113 5000
ComboBox mount 1014 1022 1000
CommandBar mount 10477 10253 1000
ContextualMenu mount 6509 6579 1000
DefaultButton mount 1216 1189 5000
DetailsRow mount 3987 3799 5000
DetailsRowFast mount 3981 4002 5000
DetailsRowNoStyles mount 3744 3746 5000
Dialog mount 2237 2203 1000
DocumentCardTitle mount 145 152 1000
Dropdown mount 3508 3448 5000
FluentProviderNext mount 7061 6970 5000
FocusTrapZone mount 1854 1917 5000
FocusZone mount 1809 1818 5000
IconButton mount 1889 1983 5000
Label mount 357 347 5000
Layer mount 1873 1896 5000
Link mount 464 493 5000
MakeStyles mount 1834 1807 50000
MenuButton mount 1574 1589 5000
MessageBar mount 2066 2054 5000
Nav mount 3499 3517 1000
OverflowSet mount 1058 1064 5000
Panel mount 2115 2133 1000
Persona mount 848 856 1000
Pivot mount 1478 1507 1000
PrimaryButton mount 1420 1402 5000
Rating mount 8296 8435 5000
SearchBox mount 1480 1445 5000
Shimmer mount 2840 2746 5000
Slider mount 2071 2087 5000
SpinButton mount 5135 5221 5000
Spinner mount 440 434 5000
SplitButton mount 3393 3275 5000
Stack mount 531 537 5000
StackWithIntrinsicChildren mount 1711 1666 5000
StackWithTextChildren mount 5054 5122 5000
SwatchColorPicker mount 11120 10756 5000
Tabs mount 1566 1538 1000
TagPicker mount 2707 2721 5000
TeachingBubble mount 12146 12450 5000
Text mount 458 466 5000
TextField mount 1576 1602 5000
ThemeProvider mount 1355 1329 5000
ThemeProvider virtual-rerender 636 657 5000
Toggle mount 918 897 5000
buttonNative mount 123 125 5000

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ImageMinimalPerf.default 447 397 1.13:1
AccordionMinimalPerf.default 180 162 1.11:1
AvatarMinimalPerf.default 231 212 1.09:1
CardMinimalPerf.default 648 597 1.09:1
ButtonMinimalPerf.default 196 184 1.07:1
ListMinimalPerf.default 557 520 1.07:1
ChatMinimalPerf.default 749 707 1.06:1
DividerMinimalPerf.default 401 379 1.06:1
FormMinimalPerf.default 480 453 1.06:1
TextMinimalPerf.default 400 377 1.06:1
TreeWith60ListItems.default 183 173 1.06:1
ButtonSlotsPerf.default 619 592 1.05:1
CarouselMinimalPerf.default 523 499 1.05:1
ToolbarMinimalPerf.default 1030 980 1.05:1
AlertMinimalPerf.default 318 305 1.04:1
AnimationMinimalPerf.default 445 429 1.04:1
DialogMinimalPerf.default 836 803 1.04:1
LayoutMinimalPerf.default 402 385 1.04:1
RadioGroupMinimalPerf.default 513 491 1.04:1
RefMinimalPerf.default 233 223 1.04:1
StatusMinimalPerf.default 737 706 1.04:1
TreeMinimalPerf.default 873 840 1.04:1
ItemLayoutMinimalPerf.default 1347 1302 1.03:1
ListCommonPerf.default 693 676 1.03:1
ProviderMinimalPerf.default 1076 1044 1.03:1
SkeletonMinimalPerf.default 386 376 1.03:1
ReactionMinimalPerf.default 423 415 1.02:1
TableMinimalPerf.default 446 438 1.02:1
BoxMinimalPerf.default 394 390 1.01:1
CheckboxMinimalPerf.default 2916 2877 1.01:1
EmbedMinimalPerf.default 4439 4407 1.01:1
ListWith60ListItems.default 711 705 1.01:1
PopupMinimalPerf.default 590 587 1.01:1
SplitButtonMinimalPerf.default 4034 4010 1.01:1
IconMinimalPerf.default 677 669 1.01:1
TableManyItemsPerf.default 2109 2093 1.01:1
VideoMinimalPerf.default 700 691 1.01:1
DropdownMinimalPerf.default 3254 3258 1:1
ListNestedPerf.default 577 576 1:1
MenuButtonMinimalPerf.default 1752 1746 1:1
CustomToolbarPrototype.default 4053 4044 1:1
ChatDuplicateMessagesPerf.default 325 327 0.99:1
DatepickerMinimalPerf.default 5565 5629 0.99:1
DropdownManyItemsPerf.default 733 738 0.99:1
GridMinimalPerf.default 357 359 0.99:1
HeaderMinimalPerf.default 410 414 0.99:1
InputMinimalPerf.default 1340 1350 0.99:1
LoaderMinimalPerf.default 730 739 0.99:1
MenuMinimalPerf.default 952 963 0.99:1
PortalMinimalPerf.default 179 180 0.99:1
SliderMinimalPerf.default 1659 1668 0.99:1
TextAreaMinimalPerf.default 575 579 0.99:1
AttachmentMinimalPerf.default 164 168 0.98:1
AttachmentSlotsPerf.default 1164 1187 0.98:1
ButtonOverridesMissPerf.default 1855 1897 0.98:1
ProviderMergeThemesPerf.default 1643 1679 0.98:1
TooltipMinimalPerf.default 1049 1065 0.98:1
HeaderSlotsPerf.default 841 866 0.97:1
LabelMinimalPerf.default 425 436 0.97:1
RosterPerf.default 1281 1324 0.97:1
SegmentMinimalPerf.default 369 380 0.97:1
FlexMinimalPerf.default 332 348 0.95:1
ChatWithPopoverPerf.default 390 413 0.94:1

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 2, 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 b412c6c:

Sandbox Source
Fluent UI Button Configuration
codesandbox-react-template Configuration
codesandbox-react-northstar-template Configuration

ecraig12345
ecraig12345 approved these changes Jul 2, 2021
View reviewed changes
Copy link
Member

@ecraig12345 ecraig12345 left a comment

Choose a reason for hiding this comment

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

Windows strikes again 🤦‍♀️

@ecraig12345 ecraig12345 merged commit 936b451 into master Jul 2, 2021
@ecraig12345 ecraig12345 deleted the fix/bmk-regex branch July 2, 2021 16:44
@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-tooltip@v9.0.0-alpha.52 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-menu@v9.0.0-alpha.45 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-link@v9.0.0-alpha.53 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-image@v9.0.0-alpha.51 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-divider@v9.0.0-alpha.41 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-components@v9.0.0-alpha.74 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-button@v9.0.0-alpha.55 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-badge@v9.0.0-alpha.53 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-avatar@v9.0.0-alpha.52 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-accordion@v9.0.0-alpha.47 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/babel-make-styles@v9.0.0-alpha.22 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-examples@v8.33.4 has been released which incorporates this pull request.:tada:

Handy links:

PeterDraex pushed a commit to PeterDraex/fluentui that referenced this pull request Aug 6, 2021
@layershifter @PeterDraex
fix(babel-make-styles): update regex for rules (microsoft#18816)
95c8599
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ecraig12345 ecraig12345 ecraig12345 approved these changes

@ling1726 ling1726 ling1726 approved these changes

Assignees

@miroslavstastny miroslavstastny

Labels
Fluent UI react-components (v9)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Build Fails when I execute yarn build --to @fluentui/react-examples
6 participants
@layershifter @fabricteam @msft-fluent-ui-bot @ecraig12345 @ling1726 @miroslavstastny