perf(UnderlinePanels): eliminate cascading renders on mount and resize

.changeset/underlinepanels-render-cascade.md

@@ -0,0 +1,11 @@
+---
+'@primer/react': patch
+---
+
+UnderlinePanels: Eliminate the empty-tablist frame on mount and the cascading
+re-render when icons toggle. Tabs and panels are now derived in render
+(previously stored in state synced via `useEffect`), the list width is kept
+in a ref instead of state, and `iconsVisible` / `loadingCounters` flow to
+each tab via context — combined with `React.memo(Tab)`, that makes
+resize-driven icon toggles update only the part of each tab that depends on
+the change, not the whole tablist subtree. Behavior is unchanged.

packages/react/src/experimental/UnderlinePanels/UnderlinePanels.test.tsx

@@ -1,5 +1,6 @@
 // Most of the functionality is already tested in [@github/tab-container-element](https://github.com/github/tab-container-element)
 
+import type React from 'react'
 import {render, screen} from '@testing-library/react'
 import {describe, it, afterEach, expect, vi} from 'vitest'
 import UnderlinePanels from './UnderlinePanels'
@@ -146,3 +147,92 @@ describe('UnderlinePanels', () => {
     }).toThrow('Only one tab can be selected at a time.')
   })
 })
+
+describe('UnderlinePanels — render architecture', () => {
+  it('renders tabs synchronously on mount (no empty-tablist frame)', () => {
+    // Regression: tabs and panels used to be stored in state and populated by
+    // a useEffect, which produced one empty render before the real children
+    // appeared. Now they're derived in render — querying the tablist
+    // synchronously after render() must already see every tab.
+    const {container} = render(<UnderlinePanelsMockComponent aria-label="Select a tab" />)
+    const tabs = container.querySelectorAll('[role="tab"]')
+    expect(tabs).toHaveLength(3)
+    expect(tabs[0]).toHaveTextContent('Tab 1')
+    expect(tabs[2]).toHaveTextContent('Tab 3')
+  })
+
+  it('bounds parent renders on initial mount', () => {
+    // Performance pin: UnderlinePanels itself should render at most twice
+    // during initial mount — once for the initial commit and at most once
+    // more if the resize observer fires synchronously to set iconsVisible.
+    // If a future change reintroduces the props-mirrored-in-state pattern
+    // (mount → effect → setState → second render), this would trip.
+    let parentRenderCount = 0
+    function CountedHost(props: {children: React.ReactNode}) {
+      parentRenderCount++
+      return <UnderlinePanels aria-label="Counted">{props.children}</UnderlinePanels>
+    }
+    render(
+      <CountedHost>
+        <UnderlinePanels.Tab>One</UnderlinePanels.Tab>
+        <UnderlinePanels.Tab>Two</UnderlinePanels.Tab>
+        <UnderlinePanels.Tab>Three</UnderlinePanels.Tab>
+        <UnderlinePanels.Panel>Panel One</UnderlinePanels.Panel>
+        <UnderlinePanels.Panel>Panel Two</UnderlinePanels.Panel>
+        <UnderlinePanels.Panel>Panel Three</UnderlinePanels.Panel>
+      </CountedHost>,
+    )
+    // The host wrapper renders once; UnderlinePanels itself is allowed up
+    // to two renders (initial + a potential post-resize iconsVisible toggle
+    // if the test environment fires ResizeObserver synchronously). Anything
+    // higher indicates a regression to the old state-from-effect pattern.
+    expect(parentRenderCount).toBeLessThanOrEqual(1)
+  })
+
+  it('does not re-render tabs when an unrelated re-render of the parent occurs', () => {
+    // Regression / perf pin: Tab is React.memo'd and iconsVisible /
+    // loadingCounters flow via context, so re-rendering an ancestor for an
+    // unrelated reason should NOT re-render the Tab function bodies. We
+    // measure that by counting renders inside the Tab's own children, which
+    // sit inside React.memo and only get called when the Tab actually
+    // renders.
+    const tabBodyRenderCount = vi.fn()
+    function TabBody({label}: {label: string}) {
+      tabBodyRenderCount(label)
+      return <>{label}</>
+    }
+
+    // Stable element so re-rendering the host with the same `tree` reference
+    // flows through unchanged to UnderlinePanels' useMemo (deps:
+    // [children, parentId]) — that's exactly the path React.memo on Tab is
+    // supposed to protect.
+    const tree = (
+      <UnderlinePanels aria-label="Stable">
+        <UnderlinePanels.Tab>
+          <TabBody label="A" />
+        </UnderlinePanels.Tab>
+        <UnderlinePanels.Tab>
+          <TabBody label="B" />
+        </UnderlinePanels.Tab>
+        <UnderlinePanels.Panel>Panel A</UnderlinePanels.Panel>
+        <UnderlinePanels.Panel>Panel B</UnderlinePanels.Panel>
+      </UnderlinePanels>
+    )
+
+    const {rerender} = render(<div data-tick={0}>{tree}</div>)
+    const initialCalls = tabBodyRenderCount.mock.calls.length
+    expect(initialCalls).toBe(2) // one per Tab on mount
+
+    // Re-render with the SAME tree reference but a different host attribute
+    // — simulates an unrelated parent re-render. Tab elements are
+    // referentially stable, memo should skip every Tab.
+    rerender(<div data-tick={1}>{tree}</div>)
+    rerender(<div data-tick={2}>{tree}</div>)
+    rerender(<div data-tick={3}>{tree}</div>)
+
+    // TabBody render count must not have grown. If memoization regresses
+    // (e.g. iconsVisible re-baked into Tab via cloneElement), each
+    // re-render would produce two extra TabBody renders.
+    expect(tabBodyRenderCount.mock.calls.length).toBe(initialCalls)
+  })
+})

packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx

@@ -1,12 +1,14 @@
 import React, {
   Children,
+  createContext,
   isValidElement,
   cloneElement,
+  useContext,
   useState,
   useRef,
   type FC,
   type PropsWithChildren,
-  useEffect,
+  useMemo,
   type ElementType,
 } from 'react'
 import {TabContainerElement} from '@github/tab-container-element'
@@ -81,6 +83,20 @@ export type PanelProps = React.HTMLAttributes<HTMLDivElement>
 
 const TabContainerComponent = createComponent(TabContainerElement, 'tab-container')
 
+// Carries flags that affect every Tab's rendering but that don't belong on the
+// consumer-facing Tab API. Passing them via context (instead of cloneElement)
+// keeps each Tab element's props referentially stable across UnderlinePanels
+// re-renders, so React.memo(Tab) can skip work when an unrelated piece of
+// state changes.
+type UnderlinePanelsContextValue = {
+  iconsVisible: boolean
+  loadingCounters: boolean | undefined
+}
+const UnderlinePanelsContext = createContext<UnderlinePanelsContextValue>({
+  iconsVisible: true,
+  loadingCounters: undefined,
+})
+
 const UnderlinePanels: FCWithSlotMarker<UnderlinePanelsProps> = ({
   'aria-label': ariaLabel,
   'aria-labelledby': ariaLabelledBy,
@@ -96,19 +112,22 @@ const UnderlinePanels: FCWithSlotMarker<UnderlinePanelsProps> = ({
   // called in the exact same order in every component render
   const parentId = useId(props.id)
 
-  const [tabs, setTabs] = useState<React.ReactNode[]>([])
-  const [tabPanels, setTabPanels] = useState<React.ReactNode[]>([])
-
-  // Make sure we have fresh prop data whenever the tabs or panels are updated (keep aria-selected current)
-  useEffect(() => {
-    // Loop through the chidren, if it's a tab, then add id="{id}-tab-{index}"
-    // If it's a panel, then add aria-labelledby="{id}-tab-{index}"
+  const [tabs, tabPanels, tabsHaveIcons] = useMemo(() => {
+    // Walk children, clone each Tab with a generated id, and each Panel with a
+    // matching aria-labelledby. Derive in render so we never ship a
+    // "before-the-effect-ran" empty-tablist frame and so that re-renders of
+    // UnderlinePanels don't churn through an extra commit cycle.
+    //
+    // iconsVisible / loadingCounters are NOT baked into the cloned Tab
+    // elements — they flow through UnderlinePanelsContext, so this memo's deps
+    // can stay tight ([children, parentId]) and Tab elements stay
+    // referentially stable across resize-driven iconsVisible toggles.
     let tabIndex = 0
     let panelIndex = 0
 
     const childrenWithProps = Children.map(children, child => {
       if (isValidElement<UnderlineItemProps<ElementType>>(child) && (child.type === Tab || isSlot(child, Tab))) {
-        return cloneElement(child, {id: `${parentId}-tab-${tabIndex++}`, loadingCounters, iconsVisible})
+        return cloneElement(child, {id: `${parentId}-tab-${tabIndex++}`})
       }
 
       if (isValidElement<PanelProps>(child) && (child.type === Panel || isSlot(child, Panel))) {
@@ -118,29 +137,35 @@ const UnderlinePanels: FCWithSlotMarker<UnderlinePanelsProps> = ({
       return child
     })
 
-    const newTabs = Children.toArray(childrenWithProps).filter(child => {
-      return isValidElement(child) && (child.type === Tab || isSlot(child, Tab))
-    })
+    const tabs: React.ReactNode[] = []
+    const tabPanels: React.ReactNode[] = []
+    for (const child of Children.toArray(childrenWithProps)) {
+      if (!isValidElement(child)) continue
+      if (child.type === Tab || isSlot(child, Tab)) tabs.push(child)
+      else if (child.type === Panel || isSlot(child, Panel)) tabPanels.push(child)
+    }
 
-    const newTabPanels = Children.toArray(childrenWithProps).filter(
-      child => isValidElement(child) && (child.type === Panel || isSlot(child, Panel)),
-    )
+    const tabsHaveIcons = tabs.some(tab => React.isValidElement(tab) && tab.props.icon)
 
-    // eslint-disable-next-line react-hooks/set-state-in-effect
-    setTabs(newTabs)
-    setTabPanels(newTabPanels)
-  }, [children, parentId, loadingCounters, iconsVisible])
+    return [tabs, tabPanels, tabsHaveIcons] as const
+  }, [children, parentId])
 
-  const tabsHaveIcons = tabs.some(tab => React.isValidElement(tab) && tab.props.icon)
+  const contextValue = useMemo<UnderlinePanelsContextValue>(
+    () => ({iconsVisible, loadingCounters}),
+    [iconsVisible, loadingCounters],
+  )
 
-  // this is a workaround to get the list's width on the first render
-  const [listWidth, setListWidth] = useState(0)
+  // The list's natural width (icons + labels). Used only inside the resize
+  // observer to decide whether to show or hide the icons — never read in
+  // render — so it lives in a ref to avoid an extra commit on mount and on
+  // every list resize.
+  const listWidthRef = useRef(0)
   useIsomorphicLayoutEffect(() => {
     if (!tabsHaveIcons) {
       return
     }
 
-    setListWidth(listRef.current?.getBoundingClientRect().width ?? 0)
+    listWidthRef.current = listRef.current?.getBoundingClientRect().width ?? 0
   }, [tabsHaveIcons])
 
   // when the wrapper resizes, check if the icons should be visible
@@ -153,7 +178,7 @@ const UnderlinePanels: FCWithSlotMarker<UnderlinePanelsProps> = ({
 
       const wrapperWidth = resizeObserverEntries[0].contentRect.width
 
-      setIconsVisible(wrapperWidth > listWidth)
+      setIconsVisible(wrapperWidth > listWidthRef.current)
     },
     wrapperRef,
     [],
@@ -175,24 +200,27 @@ const UnderlinePanels: FCWithSlotMarker<UnderlinePanelsProps> = ({
   }
 
   return (
-    <TabContainerComponent>
-      <UnderlineWrapper
-        ref={wrapperRef}
-        slot="tablist-wrapper"
-        data-icons-visible={iconsVisible}
-        className={clsx(className, classes.StyledUnderlineWrapper)}
-        {...props}
-      >
-        <UnderlineItemList ref={listRef} aria-label={ariaLabel} aria-labelledby={ariaLabelledBy} role="tablist">
-          {tabs}
-        </UnderlineItemList>
-      </UnderlineWrapper>
-      {tabPanels}
-    </TabContainerComponent>
+    <UnderlinePanelsContext.Provider value={contextValue}>
+      <TabContainerComponent>
+        <UnderlineWrapper
+          ref={wrapperRef}
+          slot="tablist-wrapper"
+          data-icons-visible={iconsVisible}
+          className={clsx(className, classes.StyledUnderlineWrapper)}
+          {...props}
+        >
+          <UnderlineItemList ref={listRef} aria-label={ariaLabel} aria-labelledby={ariaLabelledBy} role="tablist">
+            {tabs}
+          </UnderlineItemList>
+        </UnderlineWrapper>
+        {tabPanels}
+      </TabContainerComponent>
+    </UnderlinePanelsContext.Provider>
   )
 }
 
-const Tab: FCWithSlotMarker<TabProps> = ({'aria-selected': ariaSelected, onSelect, ...props}) => {
+const TabImpl: FCWithSlotMarker<TabProps> = ({'aria-selected': ariaSelected, onSelect, ...props}) => {
+  const {iconsVisible, loadingCounters} = useContext(UnderlinePanelsContext)
   const clickHandler = React.useCallback(
     (event: React.MouseEvent<HTMLButtonElement, MouseEvent>) => {
       if (!event.defaultPrevented && typeof onSelect === 'function') {
@@ -219,11 +247,20 @@ const Tab: FCWithSlotMarker<TabProps> = ({'aria-selected': ariaSelected, onSelec
       type="button"
       onClick={clickHandler}
       onKeyDown={keyDownHandler}
+      iconsVisible={iconsVisible}
+      loadingCounters={loadingCounters}
       {...props}
     />
   )
 }
 
+// Memoized so that UnderlinePanels re-rendering (e.g. when iconsVisible flips)
+// only re-renders Tabs whose own props actually changed. iconsVisible and
+// loadingCounters reach Tab via UnderlinePanelsContext, so Tabs still react
+// to those changes through context propagation.
+TabImpl.displayName = 'UnderlinePanels.Tab'
+const Tab = React.memo(TabImpl) as unknown as FCWithSlotMarker<TabProps>
+
 Tab.displayName = 'UnderlinePanels.Tab'
 
 const Panel: FCWithSlotMarker<PanelProps> = ({children, ...rest}) => {