@uifabric/foundation withSlots traverses React.Children even if already provided via parent component e.g. StackView

[KevinTCoughlin]

Follow-up of #15462. withSlots should re-use children without having to recursively traverse the component tree (expensive). There are TODO comments mentioning this inefficiency. It appears that the method signature can be reworked to avoid having the re-flatten the React.Children to an array. See the "blue" and "red" outlined call stacks below.

fluentui/packages/foundation/src/slots.tsx

Line 57 in e915a19

children = React.Children.toArray(children);

Can we re-use ...children: React.ReactNode[] already traversed via map in StackView and passed into withSlots? This would eliminate the red call stack entirely.

React.Children traversal can be a performance bottleneck for wrapper components if the component sub-tree is large.

Environment Information

• Package version(s): @uifabric/foundation and dependents
• Browser and OS versions: ALL

Please provide a reproduction of the bug in a codepen:

Profile this page using the browser's performance profiler to observe the performance bug: https://cdpn.io/kevintcoughlin/debug/gOMaYYV.

Priorities and help requested:

Are you willing to submit a PR to fix? No

Requested priority: Normal

Products/sites affected: OneDrive & SharePoint

[paulgildea]

@KevinTCoughlin Thanks!

@dzearing - FYI

[dzearing]

Yup! Talked with Kevin about it a couple days ago. Great find.

We should have a lint rule on referencing Children.count. I'll see if we can add this.

I did a search of Children.count usage in the repo:

@khmakoto I see some usage in Card that seems unnecessary. There's also one in StackItem. Can you help take a look at these?

The Text component is being redone in the @fluentui/react-text package, and does not have the reference. But to fast track removing the reference

The LayoutGroup component in the experimental package should be deleted; in fact we should be deleting all of the dusty components we know aren't used in ODSP, and any we do know should be moved out of experiments into their own package (Tile, TileView)

[msft-fluent-ui-bot]

Because this issue has not had activity for over 150 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.