Add as prop to allowed props from getNativeElementProps#19883
Merged
theerebuss merged 7 commits intomicrosoft:masterfrom Sep 23, 2021
theerebuss:add-as-to-getnativeprops
Merged
Add as prop to allowed props from getNativeElementProps#19883theerebuss merged 7 commits intomicrosoft:masterfrom theerebuss:add-as-to-getnativeprops
as prop to allowed props from getNativeElementProps#19883theerebuss merged 7 commits intomicrosoft:masterfrom
theerebuss:add-as-to-getnativeprops
Conversation
Collaborator
📊 Bundle size report
Unchanged fixtures
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: b551b682cff01f796bd7e381be96e86720e07f7a (build) |
Collaborator
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 1029 | 1051 | 5000 | |
| BaseButton | mount | 1001 | 1013 | 5000 | |
| Breadcrumb | mount | 2723 | 2729 | 1000 | |
| ButtonNext | mount | 496 | 491 | 5000 | |
| Checkbox | mount | 1686 | 1693 | 5000 | |
| CheckboxBase | mount | 1442 | 1430 | 5000 | |
| ChoiceGroup | mount | 5181 | 5225 | 5000 | |
| ComboBox | mount | 1068 | 1050 | 1000 | |
| CommandBar | mount | 10827 | 10735 | 1000 | |
| ContextualMenu | mount | 6862 | 6892 | 1000 | |
| DefaultButton | mount | 1251 | 1260 | 5000 | |
| DetailsRow | mount | 4001 | 4062 | 5000 | |
| DetailsRowFast | mount | 4071 | 4059 | 5000 | |
| DetailsRowNoStyles | mount | 3797 | 3788 | 5000 | |
| Dialog | mount | 2561 | 2605 | 1000 | |
| DocumentCardTitle | mount | 160 | 172 | 1000 | |
| Dropdown | mount | 3489 | 3442 | 5000 | |
| FluentProviderNext | mount | 7250 | 7094 | 5000 | |
| FluentProviderWithTheme | mount | 351 | 350 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 100 | 100 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 463 | 485 | 10 | |
| FocusTrapZone | mount | 1911 | 1948 | 5000 | |
| FocusZone | mount | 1881 | 1867 | 5000 | |
| IconButton | mount | 1937 | 1953 | 5000 | |
| Label | mount | 362 | 360 | 5000 | |
| Layer | mount | 3132 | 3229 | 5000 | |
| Link | mount | 512 | 512 | 5000 | |
| MakeStyles | mount | 1861 | 1868 | 50000 | |
| MenuButton | mount | 1685 | 1674 | 5000 | |
| MessageBar | mount | 2112 | 2149 | 5000 | |
| Nav | mount | 3546 | 3595 | 1000 | |
| OverflowSet | mount | 1175 | 1211 | 5000 | |
| Panel | mount | 2474 | 2465 | 1000 | |
| Persona | mount | 892 | 896 | 1000 | |
| Pivot | mount | 1612 | 1555 | 1000 | |
| PrimaryButton | mount | 1415 | 1399 | 5000 | |
| Rating | mount | 8699 | 8581 | 5000 | |
| SearchBox | mount | 1493 | 1478 | 5000 | |
| Shimmer | mount | 2879 | 2792 | 5000 | |
| Slider | mount | 2135 | 2109 | 5000 | |
| SpinButton | mount | 5437 | 5430 | 5000 | |
| Spinner | mount | 437 | 457 | 5000 | |
| SplitButton | mount | 3468 | 3429 | 5000 | |
| Stack | mount | 552 | 543 | 5000 | |
| StackWithIntrinsicChildren | mount | 1924 | 1946 | 5000 | |
| StackWithTextChildren | mount | 5270 | 5269 | 5000 | |
| SwatchColorPicker | mount | 11339 | 11369 | 5000 | |
| Tabs | mount | 1527 | 1517 | 1000 | |
| TagPicker | mount | 2969 | 2913 | 5000 | |
| TeachingBubble | mount | 13651 | 13767 | 5000 | |
| Text | mount | 497 | 459 | 5000 | |
| TextField | mount | 1514 | 1550 | 5000 | |
| ThemeProvider | mount | 1235 | 1275 | 5000 | |
| ThemeProvider | virtual-rerender | 599 | 618 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 2136 | 2086 | 5000 | |
| Toggle | mount | 907 | 894 | 5000 | |
| buttonNative | mount | 120 | 121 | 5000 |
Perf Analysis (@fluentui/react-northstar)
Perf tests with no regressions
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| AccordionMinimalPerf.default | 201 | 172 | 1.17:1 |
| ButtonMinimalPerf.default | 223 | 202 | 1.1:1 |
| PortalMinimalPerf.default | 199 | 181 | 1.1:1 |
| IconMinimalPerf.default | 751 | 684 | 1.1:1 |
| SkeletonMinimalPerf.default | 417 | 389 | 1.07:1 |
| PopupMinimalPerf.default | 656 | 617 | 1.06:1 |
| LoaderMinimalPerf.default | 790 | 751 | 1.05:1 |
| RefMinimalPerf.default | 256 | 243 | 1.05:1 |
| VideoMinimalPerf.default | 741 | 703 | 1.05:1 |
| BoxMinimalPerf.default | 425 | 408 | 1.04:1 |
| FlexMinimalPerf.default | 328 | 315 | 1.04:1 |
| HeaderMinimalPerf.default | 432 | 417 | 1.04:1 |
| RadioGroupMinimalPerf.default | 521 | 501 | 1.04:1 |
| SegmentMinimalPerf.default | 425 | 410 | 1.04:1 |
| DropdownManyItemsPerf.default | 785 | 762 | 1.03:1 |
| ListMinimalPerf.default | 587 | 571 | 1.03:1 |
| TextAreaMinimalPerf.default | 604 | 585 | 1.03:1 |
| AnimationMinimalPerf.default | 458 | 449 | 1.02:1 |
| AttachmentMinimalPerf.default | 180 | 177 | 1.02:1 |
| CardMinimalPerf.default | 671 | 659 | 1.02:1 |
| ChatWithPopoverPerf.default | 450 | 440 | 1.02:1 |
| DatepickerMinimalPerf.default | 6057 | 5952 | 1.02:1 |
| DialogMinimalPerf.default | 842 | 829 | 1.02:1 |
| DividerMinimalPerf.default | 416 | 406 | 1.02:1 |
| EmbedMinimalPerf.default | 4683 | 4601 | 1.02:1 |
| GridMinimalPerf.default | 396 | 389 | 1.02:1 |
| LayoutMinimalPerf.default | 428 | 419 | 1.02:1 |
| MenuMinimalPerf.default | 963 | 946 | 1.02:1 |
| MenuButtonMinimalPerf.default | 1861 | 1823 | 1.02:1 |
| ToolbarMinimalPerf.default | 1078 | 1052 | 1.02:1 |
| TooltipMinimalPerf.default | 1200 | 1178 | 1.02:1 |
| ButtonOverridesMissPerf.default | 1959 | 1938 | 1.01:1 |
| ButtonSlotsPerf.default | 627 | 618 | 1.01:1 |
| HeaderSlotsPerf.default | 894 | 881 | 1.01:1 |
| ItemLayoutMinimalPerf.default | 1383 | 1373 | 1.01:1 |
| ListNestedPerf.default | 655 | 647 | 1.01:1 |
| ReactionMinimalPerf.default | 441 | 435 | 1.01:1 |
| SplitButtonMinimalPerf.default | 4780 | 4715 | 1.01:1 |
| CustomToolbarPrototype.default | 4465 | 4436 | 1.01:1 |
| AvatarMinimalPerf.default | 238 | 237 | 1:1 |
| CarouselMinimalPerf.default | 520 | 518 | 1:1 |
| DropdownMinimalPerf.default | 3357 | 3369 | 1:1 |
| InputMinimalPerf.default | 1413 | 1407 | 1:1 |
| LabelMinimalPerf.default | 438 | 437 | 1:1 |
| TableManyItemsPerf.default | 2184 | 2186 | 1:1 |
| TableMinimalPerf.default | 464 | 465 | 1:1 |
| TreeMinimalPerf.default | 905 | 908 | 1:1 |
| ChatMinimalPerf.default | 731 | 737 | 0.99:1 |
| CheckboxMinimalPerf.default | 2930 | 2947 | 0.99:1 |
| ListCommonPerf.default | 729 | 733 | 0.99:1 |
| TextMinimalPerf.default | 386 | 389 | 0.99:1 |
| AlertMinimalPerf.default | 301 | 308 | 0.98:1 |
| AttachmentSlotsPerf.default | 1219 | 1238 | 0.98:1 |
| ImageMinimalPerf.default | 439 | 447 | 0.98:1 |
| ProviderMinimalPerf.default | 1253 | 1276 | 0.98:1 |
| StatusMinimalPerf.default | 746 | 763 | 0.98:1 |
| ChatDuplicateMessagesPerf.default | 335 | 347 | 0.97:1 |
| FormMinimalPerf.default | 465 | 481 | 0.97:1 |
| ListWith60ListItems.default | 721 | 744 | 0.97:1 |
| ProviderMergeThemesPerf.default | 1787 | 1834 | 0.97:1 |
| SliderMinimalPerf.default | 1859 | 1914 | 0.97:1 |
| RosterPerf.default | 1293 | 1351 | 0.96:1 |
| TreeWith60ListItems.default | 209 | 224 | 0.93:1 |
khmakoto
reviewed
Sep 21, 2021
| excludedPropNames?: string[], | ||
| ): TAttributes { | ||
| const allowedPropNames = (tagName && nativeElementMap[tagName]) || htmlElementProperties; | ||
| allowedPropNames.as = 1; |
Member
There was a problem hiding this comment.
I think this should be:
Suggested change
| allowedPropNames.as = 1; | |
| allowedPropNames.as = excludedPropNames.indexOf('as') === -1 ? 1 : 0; |
So that you respect if they passed as as part of the excluded props.
Contributor
Author
There was a problem hiding this comment.
excludedProps take precedence over the allowed props. I ended up missing that we use this for getSlotsCompat so I went ahead and excluded as from there too.
Also, added much needed tests 😄
added 4 commits
September 22, 2021 12:08
tringakrasniqi
approved these changes
Sep 22, 2021
Contributor
tringakrasniqi
left a comment
There was a problem hiding this comment.
Approving for react-text
khmakoto
approved these changes
Sep 22, 2021
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Pull request checklist
getNativeElementPropsdoes not propagateasprop #19785$ yarn changeDescription of changes
asprop to allowed props ofgetNativeElementProps.asprop to the excluded props forgetSlotsCompatas that would change the API for components that are not using simplified prop merging yet.