feat(react-conformance): Deprecate conformance test consistent-callback-args
#30301
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| FluentProviderWithTheme | virtual-rerender | 65 | 59 | 10 | Possible regression |
All results
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 653 | 607 | 5000 | |
| Button | mount | 294 | 308 | 5000 | |
| Field | mount | 1152 | 1099 | 5000 | |
| FluentProvider | mount | 704 | 716 | 5000 | |
| FluentProviderWithTheme | mount | 77 | 77 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 65 | 59 | 10 | Possible regression |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 76 | 80 | 10 | |
| MakeStyles | mount | 829 | 848 | 50000 | |
| Persona | mount | 1726 | 1698 | 5000 | |
| SpinButton | mount | 1397 | 1331 | 5000 |
Perf Analysis (
|
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| ButtonMinimalPerf.default | 97 | 85 | 1.14:1 |
| ChatDuplicateMessagesPerf.default | 159 | 140 | 1.14:1 |
| AttachmentMinimalPerf.default | 92 | 83 | 1.11:1 |
| BoxMinimalPerf.default | 204 | 184 | 1.11:1 |
| ListNestedPerf.default | 334 | 310 | 1.08:1 |
| AvatarMinimalPerf.default | 110 | 103 | 1.07:1 |
| DividerMinimalPerf.default | 208 | 194 | 1.07:1 |
| RadioGroupMinimalPerf.default | 271 | 253 | 1.07:1 |
| RefMinimalPerf.default | 116 | 108 | 1.07:1 |
| SkeletonMinimalPerf.default | 201 | 188 | 1.07:1 |
| DropdownManyItemsPerf.default | 402 | 378 | 1.06:1 |
| MenuMinimalPerf.default | 520 | 492 | 1.06:1 |
| AnimationMinimalPerf.default | 302 | 287 | 1.05:1 |
| AttachmentSlotsPerf.default | 667 | 635 | 1.05:1 |
| ImageMinimalPerf.default | 242 | 231 | 1.05:1 |
| InputMinimalPerf.default | 563 | 538 | 1.05:1 |
| MenuButtonMinimalPerf.default | 984 | 935 | 1.05:1 |
| SliderMinimalPerf.default | 752 | 717 | 1.05:1 |
| IconMinimalPerf.default | 391 | 371 | 1.05:1 |
| AccordionMinimalPerf.default | 87 | 84 | 1.04:1 |
| ButtonOverridesMissPerf.default | 666 | 640 | 1.04:1 |
| LoaderMinimalPerf.default | 195 | 188 | 1.04:1 |
| SplitButtonMinimalPerf.default | 2304 | 2226 | 1.04:1 |
| TextMinimalPerf.default | 201 | 194 | 1.04:1 |
| VideoMinimalPerf.default | 441 | 425 | 1.04:1 |
| AlertMinimalPerf.default | 156 | 152 | 1.03:1 |
| FlexMinimalPerf.default | 160 | 156 | 1.03:1 |
| TableManyItemsPerf.default | 1130 | 1100 | 1.03:1 |
| CustomToolbarPrototype.default | 1473 | 1436 | 1.03:1 |
| ChatMinimalPerf.default | 437 | 428 | 1.02:1 |
| DialogMinimalPerf.default | 437 | 428 | 1.02:1 |
| EmbedMinimalPerf.default | 1904 | 1865 | 1.02:1 |
| HeaderSlotsPerf.default | 471 | 463 | 1.02:1 |
| ProviderMinimalPerf.default | 202 | 198 | 1.02:1 |
| TextAreaMinimalPerf.default | 293 | 288 | 1.02:1 |
| CheckboxMinimalPerf.default | 1160 | 1153 | 1.01:1 |
| DatepickerMinimalPerf.default | 3515 | 3497 | 1.01:1 |
| GridMinimalPerf.default | 185 | 184 | 1.01:1 |
| SegmentMinimalPerf.default | 194 | 192 | 1.01:1 |
| StatusMinimalPerf.default | 395 | 393 | 1.01:1 |
| DropdownMinimalPerf.default | 1409 | 1415 | 1:1 |
| HeaderMinimalPerf.default | 206 | 206 | 1:1 |
| ItemLayoutMinimalPerf.default | 698 | 699 | 1:1 |
| LabelMinimalPerf.default | 224 | 223 | 1:1 |
| ListMinimalPerf.default | 306 | 306 | 1:1 |
| PortalMinimalPerf.default | 83 | 83 | 1:1 |
| ProviderMergeThemesPerf.default | 637 | 640 | 1:1 |
| TableMinimalPerf.default | 229 | 228 | 1:1 |
| ToolbarMinimalPerf.default | 542 | 543 | 1:1 |
| TooltipMinimalPerf.default | 1274 | 1272 | 1:1 |
| CardMinimalPerf.default | 316 | 320 | 0.99:1 |
| CarouselMinimalPerf.default | 260 | 262 | 0.99:1 |
| ListWith60ListItems.default | 361 | 366 | 0.99:1 |
| PopupMinimalPerf.default | 345 | 348 | 0.99:1 |
| TreeMinimalPerf.default | 476 | 479 | 0.99:1 |
| FormMinimalPerf.default | 216 | 220 | 0.98:1 |
| ReactionMinimalPerf.default | 211 | 216 | 0.98:1 |
| TreeWith60ListItems.default | 85 | 87 | 0.98:1 |
| LayoutMinimalPerf.default | 201 | 207 | 0.97:1 |
| ListCommonPerf.default | 375 | 388 | 0.97:1 |
| RosterPerf.default | 1506 | 1555 | 0.97:1 |
| ButtonSlotsPerf.default | 303 | 315 | 0.96:1 |
| ChatWithPopoverPerf.default | 187 | 207 | 0.9:1 |
|
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 85f4638:
|
🕵 fluentuiv8 No visual regressions between this PR and main |
🕵 FluentUIV0 No visual regressions between this PR and main |
📊 Bundle size reportUnchanged fixtures
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: be3a3d59d5c30f75d55c739548d576594ddf5c53 (build) |
🕵 fluentuiv9 No visual regressions between this PR and main |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| BaseButton | mount | 628 | 639 | 5000 | |
| Breadcrumb | mount | 1695 | 1666 | 1000 | |
| Checkbox | mount | 1693 | 1702 | 5000 | |
| CheckboxBase | mount | 1512 | 1476 | 5000 | |
| ChoiceGroup | mount | 3067 | 2949 | 5000 | |
| ComboBox | mount | 660 | 661 | 1000 | |
| CommandBar | mount | 6242 | 6264 | 1000 | |
| ContextualMenu | mount | 13266 | 13105 | 1000 | |
| DefaultButton | mount | 764 | 734 | 5000 | |
| DetailsRow | mount | 2197 | 2184 | 5000 | |
| DetailsRowFast | mount | 2208 | 2197 | 5000 | |
| DetailsRowNoStyles | mount | 2062 | 2039 | 5000 | |
| Dialog | mount | 2797 | 2640 | 1000 | |
| DocumentCardTitle | mount | 216 | 229 | 1000 | |
| Dropdown | mount | 2009 | 2025 | 5000 | |
| FocusTrapZone | mount | 1169 | 1182 | 5000 | |
| FocusZone | mount | 1093 | 1093 | 5000 | |
| GroupedList | mount | 42149 | 41849 | 2 | |
| GroupedList | virtual-rerender | 17987 | 20102 | 2 | |
| GroupedList | virtual-rerender-with-unmount | 50579 | 51555 | 2 | |
| GroupedListV2 | mount | 228 | 229 | 2 | |
| GroupedListV2 | virtual-rerender | 203 | 217 | 2 | |
| GroupedListV2 | virtual-rerender-with-unmount | 229 | 235 | 2 | |
| IconButton | mount | 1086 | 1093 | 5000 | |
| Label | mount | 331 | 338 | 5000 | |
| Layer | mount | 2748 | 2775 | 5000 | |
| Link | mount | 386 | 396 | 5000 | |
| MenuButton | mount | 963 | 971 | 5000 | |
| MessageBar | mount | 21314 | 21301 | 5000 | |
| Nav | mount | 2031 | 1963 | 1000 | |
| OverflowSet | mount | 772 | 781 | 5000 | |
| Panel | mount | 2061 | 1780 | 1000 | |
| Persona | mount | 733 | 730 | 1000 | |
| Pivot | mount | 864 | 880 | 1000 | |
| PrimaryButton | mount | 872 | 846 | 5000 | |
| Rating | mount | 4609 | 4781 | 5000 | |
| SearchBox | mount | 929 | 923 | 5000 | |
| Shimmer | mount | 1881 | 1869 | 5000 | |
| Slider | mount | 1334 | 1314 | 5000 | |
| SpinButton | mount | 2947 | 2918 | 5000 | |
| Spinner | mount | 399 | 392 | 5000 | |
| SplitButton | mount | 1851 | 1844 | 5000 | |
| Stack | mount | 441 | 417 | 5000 | |
| StackWithIntrinsicChildren | mount | 863 | 878 | 5000 | |
| StackWithTextChildren | mount | 2634 | 2598 | 5000 | |
| SwatchColorPicker | mount | 6172 | 6241 | 5000 | |
| TagPicker | mount | 1475 | 1464 | 5000 | |
| Text | mount | 375 | 368 | 5000 | |
| TextField | mount | 935 | 954 | 5000 | |
| ThemeProvider | mount | 834 | 830 | 5000 | |
| ThemeProvider | virtual-rerender | 589 | 582 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 1267 | 1289 | 5000 | |
| Toggle | mount | 621 | 619 | 5000 | |
| buttonNative | mount | 196 | 193 | 5000 |
YuanboXue-Amber
changed the title
consistent-callback-argsconsistent-callback-args
Hotell
approved these changes
Jan 17, 2024
Hotell
changed the title
consistent-callback-argsconsistent-callback-args
behowell
reviewed
Jan 17, 2024
There was a problem hiding this comment.
As far as I can tell from this PR, each component will still have to disable this test, right? And if any new event handlers are added to existing components, they will have to explicitly disable the conformance test for those events?
IMO it would be much better if components didn't have to do anything for this test to pass for new events.
For example, one option would be to make this test be opt-in by prop name: e.g. replace the opt-out ignoreProps option with an opt-in legacyCallbacks or something. Existing components could opt into the test for old events, but new events wouldn't be covered by default. And if the legacyCallbacks option wasn't present (the default case), then this test would pass without doing anything.
This was referenced Jan 18, 2024
Yeah that's a good point. I created #30372 to gather everything that needs to be done to disable old test/enable new rule. I will work on updating the test this/next week |
behowell
reviewed
Jan 24, 2024
tools/workspace-plugin/src/generators/react-library/files/src/testing/isConformant.ts__tmpl__
Outdated
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Deprecate
consistent-callback-argsconformance test. Removing it from new component template.The newly added callback will be tested by consistent-callback-arg eslint rule in #30293
Related issue
#30372