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

chore: Remove react-infobutton in favor of keeping only react-infolabel #31007

Merged

Conversation

sopranopillow
Copy link
Contributor

Since react-infobutton has been deprecated and is no longer in use, it is confusing to know which infobutton files are the correct ones for users that don't have insight on its history. Now that we have react-infolabel we should only keep the package that's currently in use.

While we are removing this, react-components will still re-export the package since removing it would break semver even though it's under /unstable.

@sopranopillow sopranopillow self-assigned this Apr 9, 2024
Copy link

codesandbox-ci bot commented Apr 9, 2024

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.

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 9, 2024

🕵 fluentuiv8 No visual regressions between this PR and main

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 9, 2024

Perf Analysis (@fluentui/react-components)

Scenario Render type Master Ticks PR Ticks Iterations Status
FluentProviderWithTheme virtual-rerender-with-unmount 76 68 10 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 594 628 5000
Button mount 294 320 5000
Field mount 1102 1169 5000
FluentProvider mount 696 722 5000
FluentProviderWithTheme mount 81 87 10
FluentProviderWithTheme virtual-rerender 33 37 10
FluentProviderWithTheme virtual-rerender-with-unmount 76 68 10 Possible regression
MakeStyles mount 851 857 50000
Persona mount 1793 1749 5000
SpinButton mount 1399 1407 5000
SwatchPicker mount 1494 1560 5000

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 9, 2024

Perf Analysis (@fluentui/react-northstar)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
ButtonMinimalPerf.default 90 89 1.01:1 analysis
Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AttachmentMinimalPerf.default 97 76 1.28:1
CarouselMinimalPerf.default 272 247 1.1:1
TextMinimalPerf.default 196 178 1.1:1
SegmentMinimalPerf.default 204 187 1.09:1
AvatarMinimalPerf.default 114 106 1.08:1
DividerMinimalPerf.default 207 191 1.08:1
TextAreaMinimalPerf.default 310 286 1.08:1
ButtonSlotsPerf.default 328 306 1.07:1
FormMinimalPerf.default 236 220 1.07:1
ReactionMinimalPerf.default 224 210 1.07:1
DropdownManyItemsPerf.default 403 385 1.05:1
ItemLayoutMinimalPerf.default 726 694 1.05:1
LoaderMinimalPerf.default 192 183 1.05:1
ListMinimalPerf.default 306 293 1.04:1
ProviderMinimalPerf.default 203 195 1.04:1
ChatMinimalPerf.default 437 423 1.03:1
ChatWithPopoverPerf.default 208 202 1.03:1
DropdownMinimalPerf.default 1436 1400 1.03:1
GridMinimalPerf.default 189 183 1.03:1
LabelMinimalPerf.default 217 210 1.03:1
SplitButtonMinimalPerf.default 2290 2226 1.03:1
TableMinimalPerf.default 245 237 1.03:1
CheckboxMinimalPerf.default 1145 1124 1.02:1
MenuButtonMinimalPerf.default 961 942 1.02:1
PopupMinimalPerf.default 344 338 1.02:1
SkeletonMinimalPerf.default 197 193 1.02:1
StatusMinimalPerf.default 401 394 1.02:1
ChatDuplicateMessagesPerf.default 153 151 1.01:1
EmbedMinimalPerf.default 1862 1844 1.01:1
LayoutMinimalPerf.default 204 201 1.01:1
ListCommonPerf.default 378 374 1.01:1
PortalMinimalPerf.default 84 83 1.01:1
ProviderMergeThemesPerf.default 651 646 1.01:1
RefMinimalPerf.default 110 109 1.01:1
SliderMinimalPerf.default 760 754 1.01:1
IconMinimalPerf.default 382 380 1.01:1
TooltipMinimalPerf.default 1281 1267 1.01:1
TreeMinimalPerf.default 493 490 1.01:1
AttachmentSlotsPerf.default 651 653 1:1
ButtonOverridesMissPerf.default 649 650 1:1
DialogMinimalPerf.default 441 442 1:1
HeaderSlotsPerf.default 464 465 1:1
ImageMinimalPerf.default 222 223 1:1
ListNestedPerf.default 326 326 1:1
ListWith60ListItems.default 370 371 1:1
RadioGroupMinimalPerf.default 262 262 1:1
VideoMinimalPerf.default 425 425 1:1
AccordionMinimalPerf.default 85 86 0.99:1
AlertMinimalPerf.default 149 150 0.99:1
DatepickerMinimalPerf.default 3518 3543 0.99:1
FlexMinimalPerf.default 153 155 0.99:1
HeaderMinimalPerf.default 205 207 0.99:1
InputMinimalPerf.default 534 541 0.99:1
MenuMinimalPerf.default 494 502 0.98:1
CustomToolbarPrototype.default 1458 1486 0.98:1
ToolbarMinimalPerf.default 545 554 0.98:1
AnimationMinimalPerf.default 298 307 0.97:1
BoxMinimalPerf.default 188 193 0.97:1
TreeWith60ListItems.default 91 94 0.97:1
TableManyItemsPerf.default 1109 1151 0.96:1
RosterPerf.default 1545 1626 0.95:1
CardMinimalPerf.default 293 318 0.92:1

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 9, 2024

📊 Bundle size report

✅ No changes found

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 9, 2024

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 617 622 5000
Breadcrumb mount 1658 1671 1000
Checkbox mount 1689 1716 5000
CheckboxBase mount 1483 1477 5000
ChoiceGroup mount 2950 2973 5000
ComboBox mount 694 716 1000
CommandBar mount 6457 6532 1000
ContextualMenu mount 11943 12013 1000
DefaultButton mount 788 782 5000
DetailsRow mount 2179 2224 5000
DetailsRowFast mount 2266 2207 5000
DetailsRowNoStyles mount 2031 2061 5000
Dialog mount 2681 2678 1000
DocumentCardTitle mount 233 232 1000
Dropdown mount 2020 2003 5000
FocusTrapZone mount 1140 1129 5000
FocusZone mount 1083 1094 5000
GroupedList mount 42043 42133 2
GroupedList virtual-rerender 17920 20106 2
GroupedList virtual-rerender-with-unmount 50801 51078 2
GroupedListV2 mount 211 232 2
GroupedListV2 virtual-rerender 205 210 2
GroupedListV2 virtual-rerender-with-unmount 228 223 2
IconButton mount 1114 1118 5000
Label mount 340 343 5000
Layer mount 2756 2741 5000
Link mount 410 398 5000
MenuButton mount 971 977 5000
MessageBar mount 21315 21222 5000
Nav mount 1984 2091 1000
OverflowSet mount 795 792 5000
Panel mount 1848 1769 1000
Persona mount 745 751 1000
Pivot mount 925 907 1000
PrimaryButton mount 925 926 5000
Rating mount 4707 4697 5000
SearchBox mount 924 926 5000
Shimmer mount 1858 1880 5000
Slider mount 1335 1365 5000
SpinButton mount 2919 3010 5000
Spinner mount 392 401 5000
SplitButton mount 1875 1875 5000
Stack mount 408 404 5000
StackWithIntrinsicChildren mount 875 835 5000
StackWithTextChildren mount 2664 2590 5000
SwatchColorPicker mount 6351 6367 5000
TagPicker mount 1435 1467 5000
Text mount 378 383 5000
TextField mount 937 945 5000
ThemeProvider mount 836 841 5000
ThemeProvider virtual-rerender 594 585 5000
ThemeProvider virtual-rerender-with-unmount 1298 1275 5000
Toggle mount 612 595 5000
buttonNative mount 192 190 5000

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 9, 2024

🕵 fluentuiv9 No visual regressions between this PR and main

@Hotell
Copy link
Contributor

Hotell commented Apr 12, 2024

wondering what happened to all these snapshots with tabster ? this should not be part of this PR

@sopranopillow sopranopillow marked this pull request as ready for review April 23, 2024 23:54
@fabricteam
Copy link
Collaborator

fabricteam commented Apr 24, 2024

🕵 FluentUIV0 No visual regressions between this PR and main

@sopranopillow
Copy link
Contributor Author

Looks like this breaks test-ssr. The dependency is found correctly in root node_modules, but then its dependencies like @fluentui/react-jsx-runtime/jsx-runtime point towards a lib folder that is not being generated in node_modules.
image

any clues @Hotell or @layershifter?

Copy link
Collaborator

@JustSlone JustSlone left a comment

Choose a reason for hiding this comment

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

Signing off now on the codeowner change

@Hotell
Copy link
Contributor

Hotell commented Apr 29, 2024

Looks like this breaks test-ssr. The dependency is found correctly in root node_modules, but then its dependencies like @fluentui/react-jsx-runtime/jsx-runtime point towards a lib folder that is not being generated in node_modules.

any clues @Hotell or @layershifter?

2 issues:

Storybook:

SSR:

  • i'm unable run the suite locally , getting lot of timeout errors which will fail eventually image
  • UPDATE: i can pass whole suite locally only if I turn off pupeteer timeout via timeout: 0 ( not sure if this is something we wanna do on CI , wdyt @layershifter ? )

esbuild stopped supporting TS path aliases since version 0.19
https://github.com/evanw/esbuild/blob/main/CHANGELOG.md#0190 /
image

So our current configuration and setup was a "ticking bomb" like following setup with cypress.

Solution:

Even if we added build task as pre-requirement for test-ssr these issues would appear sooner than later.

So we can either:

  • provide proper dependency declarations to all packages ( which would introduce circular dependencies - this will be mitigated after implementing [Feature]: improve type-checking performance #30516) + moving test-ssr to *-stories packages
  • implement tsConfigPathAliases esbuild plugin

Ideal solution is to have probably both so until that type-checking epic is done I created esbuild plugin to add ts path resolution back -> everything works as before
b9dcc79

@layershifter
Copy link
Member

Ideal solution is to have probably both so until that type-checking epic is done I created esbuild plugin to add ts path resolution back -> everything works as before

@Hotell agree with your proposal 👍 Thx for looking into that ❤️

@Hotell
Copy link
Contributor

Hotell commented Apr 30, 2024

@Hotell
Copy link
Contributor

Hotell commented May 2, 2024

@sopranopillow Once these 2 PRs land to master , update your branch to get unblocked

both landed to master, now your PR is unblocked 🥳 !

@sopranopillow sopranopillow merged commit cca849d into microsoft:master May 3, 2024
21 checks passed
@sopranopillow sopranopillow deleted the remove-infobutton-package branch May 3, 2024 00:11
miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request Jun 14, 2024
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.

6 participants