feat(react-jsx-runtime): implements custom JSX pragma #27472
Merged
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
bsunderhus
force-pushed
the
react-jsx-runtime/feat--implements-custom-jsx-pragma
branch
from
eb275e8 to
132e7bf
Compare
📊 Bundle size report
Unchanged fixtures
|
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 676 | 692 | 5000 | |
| Button | mount | 390 | 371 | 5000 | |
| Field | mount | 1236 | 1234 | 5000 | |
| FluentProvider | mount | 910 | 902 | 5000 | |
| FluentProviderWithTheme | mount | 123 | 119 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 110 | 107 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 109 | 113 | 10 | |
| InfoButton | mount | 18 | 22 | 5000 | |
| MakeStyles | mount | 1133 | 1131 | 50000 | |
| Persona | mount | 1968 | 1929 | 5000 | |
| SpinButton | mount | 1562 | 1573 | 5000 |
Perf Analysis (
|
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| FormMinimalPerf.default | 291 | 264 | 1.1:1 |
| RefMinimalPerf.default | 146 | 134 | 1.09:1 |
| AccordionMinimalPerf.default | 103 | 96 | 1.07:1 |
| FlexMinimalPerf.default | 210 | 196 | 1.07:1 |
| TreeWith60ListItems.default | 115 | 108 | 1.06:1 |
| SegmentMinimalPerf.default | 261 | 248 | 1.05:1 |
| AttachmentSlotsPerf.default | 856 | 827 | 1.04:1 |
| AvatarMinimalPerf.default | 137 | 132 | 1.04:1 |
| DividerMinimalPerf.default | 267 | 256 | 1.04:1 |
| LabelMinimalPerf.default | 288 | 278 | 1.04:1 |
| GridMinimalPerf.default | 243 | 236 | 1.03:1 |
| ImageMinimalPerf.default | 275 | 266 | 1.03:1 |
| PortalMinimalPerf.default | 121 | 117 | 1.03:1 |
| ReactionMinimalPerf.default | 273 | 264 | 1.03:1 |
| StatusMinimalPerf.default | 517 | 502 | 1.03:1 |
| AttachmentMinimalPerf.default | 99 | 97 | 1.02:1 |
| ButtonOverridesMissPerf.default | 838 | 823 | 1.02:1 |
| CardMinimalPerf.default | 384 | 377 | 1.02:1 |
| CarouselMinimalPerf.default | 344 | 338 | 1.02:1 |
| ChatMinimalPerf.default | 536 | 526 | 1.02:1 |
| ChatWithPopoverPerf.default | 250 | 245 | 1.02:1 |
| DialogMinimalPerf.default | 587 | 576 | 1.02:1 |
| HeaderSlotsPerf.default | 567 | 558 | 1.02:1 |
| MenuMinimalPerf.default | 616 | 604 | 1.02:1 |
| TextMinimalPerf.default | 251 | 245 | 1.02:1 |
| AnimationMinimalPerf.default | 373 | 371 | 1.01:1 |
| DropdownMinimalPerf.default | 1820 | 1803 | 1.01:1 |
| EmbedMinimalPerf.default | 2413 | 2393 | 1.01:1 |
| ListMinimalPerf.default | 386 | 381 | 1.01:1 |
| MenuButtonMinimalPerf.default | 1255 | 1240 | 1.01:1 |
| PopupMinimalPerf.default | 455 | 451 | 1.01:1 |
| RadioGroupMinimalPerf.default | 331 | 328 | 1.01:1 |
| TextAreaMinimalPerf.default | 351 | 349 | 1.01:1 |
| VideoMinimalPerf.default | 535 | 532 | 1.01:1 |
| AlertMinimalPerf.default | 191 | 191 | 1:1 |
| BoxMinimalPerf.default | 247 | 247 | 1:1 |
| ButtonMinimalPerf.default | 109 | 109 | 1:1 |
| CheckboxMinimalPerf.default | 1559 | 1553 | 1:1 |
| InputMinimalPerf.default | 720 | 721 | 1:1 |
| ItemLayoutMinimalPerf.default | 897 | 893 | 1:1 |
| LayoutMinimalPerf.default | 262 | 262 | 1:1 |
| ListNestedPerf.default | 403 | 403 | 1:1 |
| RosterPerf.default | 1818 | 1823 | 1:1 |
| ProviderMergeThemesPerf.default | 976 | 975 | 1:1 |
| ProviderMinimalPerf.default | 277 | 276 | 1:1 |
| SkeletonMinimalPerf.default | 256 | 256 | 1:1 |
| TableManyItemsPerf.default | 1420 | 1416 | 1:1 |
| ToolbarMinimalPerf.default | 665 | 662 | 1:1 |
| TreeMinimalPerf.default | 587 | 587 | 1:1 |
| ButtonSlotsPerf.default | 407 | 412 | 0.99:1 |
| DropdownManyItemsPerf.default | 485 | 490 | 0.99:1 |
| ListCommonPerf.default | 483 | 488 | 0.99:1 |
| LoaderMinimalPerf.default | 239 | 241 | 0.99:1 |
| SplitButtonMinimalPerf.default | 2980 | 3001 | 0.99:1 |
| TableMinimalPerf.default | 299 | 301 | 0.99:1 |
| CustomToolbarPrototype.default | 1872 | 1887 | 0.99:1 |
| ChatDuplicateMessagesPerf.default | 192 | 196 | 0.98:1 |
| IconMinimalPerf.default | 453 | 461 | 0.98:1 |
| HeaderMinimalPerf.default | 261 | 268 | 0.97:1 |
| TooltipMinimalPerf.default | 1574 | 1622 | 0.97:1 |
| ListWith60ListItems.default | 459 | 477 | 0.96:1 |
| SliderMinimalPerf.default | 898 | 932 | 0.96:1 |
| DatepickerMinimalPerf.default | 4592 | 4855 | 0.95: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 62cce49:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: faa429a450e970352796776268983c5081d7e72a (build) |
🕵 fluentuiv9 No visual regressions between this PR and main |
🕵 FluentUI-v0 No visual regressions between this PR and main |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| BaseButton | mount | 802 | 799 | 5000 | |
| Breadcrumb | mount | 2219 | 2170 | 1000 | |
| Checkbox | mount | 2138 | 2114 | 5000 | |
| CheckboxBase | mount | 1858 | 1864 | 5000 | |
| ChoiceGroup | mount | 3673 | 3725 | 5000 | |
| ComboBox | mount | 865 | 870 | 1000 | |
| CommandBar | mount | 7892 | 7908 | 1000 | |
| ContextualMenu | mount | 14885 | 15211 | 1000 | |
| DefaultButton | mount | 983 | 973 | 5000 | |
| DetailsRow | mount | 2891 | 2904 | 5000 | |
| DetailsRowFast | mount | 2896 | 2866 | 5000 | |
| DetailsRowNoStyles | mount | 2612 | 2682 | 5000 | |
| Dialog | mount | 3395 | 3389 | 1000 | |
| DocumentCardTitle | mount | 312 | 313 | 1000 | |
| Dropdown | mount | 2520 | 2522 | 5000 | |
| FocusTrapZone | mount | 1512 | 1523 | 5000 | |
| FocusZone | mount | 1428 | 1418 | 5000 | |
| GroupedList | mount | 49329 | 57266 | 2 | |
| GroupedList | virtual-rerender | 23874 | 23847 | 2 | |
| GroupedList | virtual-rerender-with-unmount | 73706 | 72688 | 2 | |
| GroupedListV2 | mount | 309 | 309 | 2 | |
| GroupedListV2 | virtual-rerender | 291 | 304 | 2 | |
| GroupedListV2 | virtual-rerender-with-unmount | 315 | 319 | 2 | |
| IconButton | mount | 1414 | 1409 | 5000 | |
| Label | mount | 455 | 449 | 5000 | |
| Layer | mount | 3585 | 3628 | 5000 | |
| Link | mount | 522 | 520 | 5000 | |
| MenuButton | mount | 1206 | 1225 | 5000 | |
| MessageBar | mount | 27254 | 27308 | 5000 | |
| Nav | mount | 2526 | 2523 | 1000 | |
| OverflowSet | mount | 1001 | 988 | 5000 | |
| Panel | mount | 2208 | 2239 | 1000 | |
| Persona | mount | 969 | 969 | 1000 | |
| Pivot | mount | 1163 | 1164 | 1000 | |
| PrimaryButton | mount | 1106 | 1095 | 5000 | |
| Rating | mount | 5736 | 5745 | 5000 | |
| SearchBox | mount | 1153 | 1148 | 5000 | |
| Shimmer | mount | 2333 | 2310 | 5000 | |
| Slider | mount | 1710 | 1706 | 5000 | |
| SpinButton | mount | 3720 | 3752 | 5000 | |
| Spinner | mount | 528 | 520 | 5000 | |
| SplitButton | mount | 2398 | 2402 | 5000 | |
| Stack | mount | 539 | 535 | 5000 | |
| StackWithIntrinsicChildren | mount | 1113 | 1127 | 5000 | |
| StackWithTextChildren | mount | 3223 | 3212 | 5000 | |
| SwatchColorPicker | mount | 8073 | 8084 | 5000 | |
| TagPicker | mount | 1987 | 1962 | 5000 | |
| Text | mount | 503 | 497 | 5000 | |
| TextField | mount | 1200 | 1194 | 5000 | |
| ThemeProvider | mount | 1108 | 1127 | 5000 | |
| ThemeProvider | virtual-rerender | 768 | 772 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 1715 | 1725 | 5000 | |
| Toggle | mount | 796 | 815 | 5000 | |
| buttonNative | mount | 279 | 280 | 5000 |
behowell
reviewed
Apr 6, 2023
packages/react-components/react-utilities/src/compose/resolveShorthand.ts
Outdated
Show resolved
Hide resolved
packages/react-components/react-utilities/src/compose/resolveShorthand.ts
Outdated
Show resolved
Hide resolved
fabricteam
reviewed
Apr 7, 2023
change/@fluentui-react-jsx-runtime-92a3c6b2-e767-4128-8320-7e237ef69eaa.json
Show resolved
Hide resolved
packages/react-components/react-utilities/src/compose/resolveShorthand.test.tsx
Show resolved
Hide resolved
packages/react-components/react-utilities/src/compose/resolveShorthand.ts
Outdated
Show resolved
Hide resolved
packages/react-components/react-utilities/src/compose/getSlots.ts
Outdated
Show resolved
Hide resolved
…horthand.ts Co-authored-by: Ben Howell <48106640+behowell@users.noreply.github.com>
bsunderhus
force-pushed
the
react-jsx-runtime/feat--implements-custom-jsx-pragma
branch
2 times, most recently
from
722dac4 to
b61aa75
Compare
& restructure jsx to simplify
bsunderhus
force-pushed
the
react-jsx-runtime/feat--implements-custom-jsx-pragma
branch
from
b61aa75 to
1bfca4a
Compare
layershifter
approved these changes
Apr 12, 2023
marcosmoura
added a commit
to marcosmoura/fluentui
that referenced
this pull request
Apr 13, 2023
…t/drawer-light-dismiss-prop * marcosmoura/feat/drawer-component: (51 commits) fix: upgrade react-dialog Doc: Add/Remove TreeItem and useFlatTree stories (microsoft#27515) applying package updates Fix accessibility labels for tree stories (microsoft#27443) fix: BasePickerListBelow aria-describedby with values (microsoft#27525) chore: Remove deprecated InputField, etc. shim components (microsoft#27492) [Feature]: allow aria-description attribute on v9 (microsoft#27478) feat(react-jsx-runtime): implements custom JSX pragma (microsoft#27472) chore: update generator to include "test-ssr" script (microsoft#27463) applying package updates fix: add "style" to exports field (microsoft#27274) applying package updates feat(react-datepicker-compat): Make package public (microsoft#27524) fix: react-combobox defines fallbackPositions for popup on small screens (microsoft#27521) fix: use tabster to focus on trigger, instead of manually invoking .focus (microsoft#27512) add cxe-red to react-shared-context ownership (microsoft#27296) docs: add props tables for all exported Popover controls (microsoft#27503) fix: Update FormFieldShim to work with Field context changes (microsoft#27436) Fix wrong state narration for multiple selected legends (microsoft#27429) add codeowners to theme designer v9 (microsoft#27311) ...
behowell
reviewed
Apr 13, 2023
Comment on lines
+8
to
+45
| export function createElement<P extends {}>( | ||
| type: React.ElementType<P>, | ||
| props?: P | null, | ||
| ...children: React.ReactNode[] | ||
| ): React.ReactElement<P> | null { | ||
| if (!isSlotComponent(props)) { | ||
| return React.createElement(type, props, ...children); | ||
| } | ||
|
|
||
| const result = normalizeRenderFunction(props, children); | ||
| return React.createElement( | ||
| React.Fragment, | ||
| {}, | ||
| result.renderFunction(type, { ...result.props, children: result.children }), | ||
| ) as React.ReactElement<P>; | ||
| } | ||
|
|
||
| function normalizeRenderFunction<Props extends UnknownSlotProps>( | ||
| propsWithMetadata: WithMetadata<Props>, | ||
| overrideChildren?: React.ReactNode[], | ||
| ): { | ||
| props: Props; | ||
| children: React.ReactNode; | ||
| renderFunction: SlotRenderFunction<Props>; | ||
| } { | ||
| const { [SLOT_RENDER_FUNCTION_SYMBOL]: renderFunction, children: externalChildren, ...props } = propsWithMetadata; | ||
|
|
||
| const children: React.ReactNode = | ||
| Array.isArray(overrideChildren) && overrideChildren.length > 0 | ||
| ? React.createElement(React.Fragment, {}, ...overrideChildren) | ||
| : externalChildren; | ||
|
|
||
| return { | ||
| children, | ||
| renderFunction, | ||
| props: props as UnknownSlotProps as Props, | ||
| }; | ||
| } |
There was a problem hiding this comment.
It seems like these functions could be simplified quite a bit, to reduce the number of Fragments and object spreads, and to be easier to understand:
export function createElement<P extends {}>(
type: React.ElementType<P>,
props?: P | null,
...children: React.ReactNode[]
): React.ReactElement<P> | null {
if (!isSlotComponent(props)) {
return React.createElement(type, props, children);
}
const { [SLOT_RENDER_FUNCTION_SYMBOL]: renderFunction, ...renderProps } = props;
if (Array.isArray(children) && children.length > 0) {
renderProps.children = children;
}
return React.createElement(React.Fragment, {}, renderFunction(type, renderProps));
}There was a problem hiding this comment.
renderProps.children cannot be an array! it has to be converted to a fragment. otherwise we'll be requiring for the render function to properly provide keys for each children element.
in this simplification, renderProps.children might end up being an array.
Our tests are even catching this problem:
Nonetheless I can see clear simplifications with this method. I'll see to follow up on this with another PR
This was referenced Apr 13, 2023
bsunderhus
deleted the
react-jsx-runtime/feat--implements-custom-jsx-pragma
branch
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.
New Behavior
@fluentui/react-jsx-runtime@fluentui/react-jsx-runtimegetSlotsNextas a compatible version ofgetSlotswith the newest custom JSX pragmaresolveShorthandandgetSlotsaccordingly to ensure previous behaviourgetSlotsNextresolveShorthandRelated Issue(s)
RFC: slot children render function supportdecision #27375