RFC: slot children render function support

[bsunderhus]

Summary

Slot children render function is a complex API that is not properly supported. #27089

To support this, this RFC proposes to create a custom JSX pragma to ensure slot declaration will not lose any property
and will be capable of properly rendering with children render function.

RFC

Current issues

Slot children render function won't override existing children. #27089

Repro: https://codesandbox.io/s/elated-babbage-46ez1w?file=/example.tsx

It's not possible to use children render function to override an entire slot, if children are already used. The above is a minimal repro of an issue that can be seen in Fluent in the AccordionHeader component. The component renders the button slot which contains its own children. JSX children will win over props children - which is how children render functions are rendered by getSlots

fluentui/packages/react-components/react-accordion/src/components/AccordionHeader/renderAccordionHeader.tsx

Lines 13 to 25 in 220b321

	const { slots, slotProps } = getSlots<AccordionHeaderSlots>(state);
	return (
	<AccordionHeaderContext.Provider value={contextValues.accordionHeader}>
	<slots.root {...slotProps.root}>
	<slots.button {...slotProps.button}>
	{state.expandIconPosition === 'start' && <slots.expandIcon {...slotProps.expandIcon} />}
	{slots.icon && <slots.icon {...slotProps.icon} />}
	{slotProps.root.children}
	{state.expandIconPosition === 'end' && <slots.expandIcon {...slotProps.expandIcon} />}
	</slots.button>
	</slots.root>
	</AccordionHeaderContext.Provider>

Requirements for children render function:

• should be able to override the entire slot
• When rendering the original children, should respect nested slots.

[fabricteam]

📊 Bundle size report

_{🤖 This report was generated against 4f76849de438f2ee7842121ac3b8176ba1c0a2a5}

[codesandbox-ci]

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 7fc2f47:

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration
modest-goldberg-ltb7b8	PR

[size-auditor]

Asset size changes

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

Baseline commit: 4f76849de438f2ee7842121ac3b8176ba1c0a2a5 (build)

[layershifter]

Based on a discussion yesterday: the approach with slot() function will be a breaking change for consumers that use composition and rely on custom render templates that are using getSlots().

[layershifter]

@bsunderhus I prototyped a less breaking change that allows us to keep existing code or to do gradual changes: https://codesandbox.io/s/wispy-leftpad-f8yi37?file=/example.tsx.

Key changes compared to current implementation:

• resolveShorthand stores defaultProps.children, it's a single thing that we need to store to make it working
• getSlots() does not handle typeof children === 'function', it will be handled by JSX pragma
• JSX pragma is a thin wrapper that handles a case with function

That solution passing cases in this example. Key benefit is that we don't change other internals and don't need to apply changes to other hooks. The next step could be moving functionality of getSlots() to JSX pragma.

[bsunderhus]

@bsunderhus I prototyped a less breaking change that allows us to keep existing code or to do gradual changes: https://codesandbox.io/s/wispy-leftpad-f8yi37?file=/example.tsx.

Key changes compared to current implementation:

• resolveShorthand stores defaultProps.children, it's a single thing that we need to store to make it working
• getSlots() does not handle typeof children === 'function', it will be handled by JSX pragma
• JSX pragma is a thin wrapper that handles a case with function

That solution passing cases in this example. Key benefit is that we don't change other internals and don't need to apply changes to other hooks. The next step could be moving functionality of getSlots() to JSX pragma.

I'll see to add it as an option on the RFC! Thx for the contribution @layershifter!

[ling1726]

Based on a discussion yesterday: the approach with slot() function will be a breaking change for consumers that use composition and rely on custom render templates that are using getSlots().

Honestly, why do we have _unstable suffix if we're not allowed to change internals signature for cases that would clearly improve internals?

@layershifter @bsunderhus let's ask WISE (a.k.a @jspurlin) for their thoughts on this, they are a partner that have fully invested in composition

[layershifter]

@bsunderhus As RFC is quite big, would you mind to run doctoc to generate table of contents in it? 🙏