Cascade elimination nav panels

.changeset/underline-nav-cascade-reduction.md

@@ -0,0 +1,21 @@
+---
+'@primer/react': patch
+---
+
+Reduce render cascades in `UnderlineNav` and `UnderlinePanels` (experimental):
+
+- `UnderlineNav.Item` no longer runs a per-item layout effect that dispatches
+  `setChildrenWidth` + `setNoIconChildrenWidth` to the parent. The parent
+  `UnderlineNav` now measures all items in a single layout effect and stores
+  widths in refs (not state). For N items, that's one batched setState
+  instead of 2N. The hook's `updateListAndMenu` now compares items/menuItems
+  by key before committing, so the `ResizeObserver`'s initial fire (which
+  reports the same dimensions the parent layout effect just measured) bails
+  out instead of producing an extra render.
+- `UnderlinePanels` now derives `tabs` and `tabPanels` inline via `useMemo`
+  instead of holding them in state and syncing through a post-paint
+  `useEffect`. The `listWidth` state is gone — the layout effect and resize
+  observer compute icon visibility by measuring both list and wrapper
+  together at decision time.
+
+No public API changes.

packages/react/src/UnderlineNav/UnderlineNav.test.tsx

@@ -1,5 +1,5 @@
 import {describe, expect, it, vi} from 'vitest'
-import type React from 'react'
+import React from 'react'
 import {render, screen} from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import {
@@ -122,7 +122,7 @@
     const {getByRole} = render(<ResponsiveUnderlineNav />)
     const item = getByRole('link', {name: 'Issues (120)'})
     const counter = item.getElementsByTagName('span')[3]
     expect(counter.textContent).toBe('120')
     expect(counter).toHaveAttribute('aria-hidden', 'true')
   })
 
@@ -138,12 +138,12 @@
     const item = getByRole('link', {name: 'Issues (120)'})
     const counter = item.getElementsByTagName('span')[4]
     // non breaking space unified code
     expect(counter.textContent).toBe('\u00A0(120)')
   })
 
   it('respects loadingCounters prop', () => {
     const {getByRole} = render(<ResponsiveUnderlineNav loadingCounters={true} />)
     const item = getByRole('link', {name: 'Actions'})
     const loadingCounter = item.getElementsByTagName('span')[2]
     expect(loadingCounter.className).toContain('LoadingCounter')
     expect(loadingCounter.textContent).toBe('')
@@ -238,6 +238,30 @@
     const textSpan = item.querySelector('[data-component="text"]')
     expect(textSpan).toHaveAttribute('data-content', 'Simple Text')
   })
+
+  it('does not cascade renders on mount (parent-driven measurement)', async () => {
+    // Each `UnderlineNav.Item` previously ran its own layout effect that
+    // dispatched two setState calls to the parent — producing one batched
+    // re-commit on mount in addition to the ResizeObserver's initial fire.
+    // The parent-driven measurement collapses that to a single setState, and
+    // the equality-guarded `updateListAndMenu` causes the ResizeObserver's
+    // first entry to bail out instead of committing identical responsiveProps.
+    //
+    // Asserting an exact upper bound here pins the improvement: with the old
+    // code the counter saw >=3 commits on mount; with the new code it sees
+    // at most 2 (initial + 1 measurement-driven update).
+    let commitCount = 0
+    render(
+      <React.Profiler id="UnderlineNavCascadeTest" onRender={() => commitCount++}>
+        <ResponsiveUnderlineNav />
+      </React.Profiler>,
+    )
+
+    // Allow the ResizeObserver's initial entry to flush.
+    await new Promise(resolve => setTimeout(resolve, 0))
+
+    expect(commitCount).toBeLessThanOrEqual(2)
+  })
 })
 
 describe('Keyboard Navigation', () => {

packages/react/src/UnderlineNav/UnderlineNav.tsx

@@ -1,9 +1,9 @@
 import type {RefObject} from 'react'
-import React, {useRef, forwardRef, useCallback, useState, useEffect} from 'react'
+import React, {useRef, forwardRef, useCallback, useState, useEffect, useMemo} from 'react'
 import {UnderlineNavContext} from './UnderlineNavContext'
 import type {ResizeObserverEntry} from '../hooks/useResizeObserver'
 import {useResizeObserver} from '../hooks/useResizeObserver'
-import type {ChildWidthArray, ResponsiveProps, ChildSize} from './types'
+import type {ChildWidthArray, ResponsiveProps} from './types'
 import VisuallyHidden from '../_VisuallyHidden'
 import {dividerStyles, menuItemStyles, baseMenuMinWidth} from './styles'
 import {UnderlineItemList, UnderlineWrapper, LoadingCounter, GAP} from '../internal/components/UnderlineTabbedInterface'
@@ -17,6 +17,7 @@ import CounterLabel from '../CounterLabel'
 import {invariant} from '../utils/invariant'
 import classes from './UnderlineNav.module.css'
 import {getAnchoredPosition} from '@primer/behaviors'
+import useIsomorphicLayoutEffect from '../utils/useIsomorphicLayoutEffect'
 
 export type UnderlineNavProps = {
   children: React.ReactNode
@@ -165,12 +166,17 @@ export const UnderlineNav = forwardRef(
 
     const [isWidgetOpen, setIsWidgetOpen] = useState(false)
     const [iconsVisible, setIconsVisible] = useState<boolean>(true)
-    const [childWidthArray, setChildWidthArray] = useState<ChildWidthArray>([])
-    const [noIconChildWidthArray, setNoIconChildWidthArray] = useState<ChildWidthArray>([])
+    // Item widths are stored in refs (not state) because they're internal to
+    // the overflow computation — never read during JSX render. Keeping them
+    // out of state means measuring children no longer triggers a re-render.
+    const childWidthArrayRef = useRef<ChildWidthArray>([])
+    const noIconChildWidthArrayRef = useRef<ChildWidthArray>([])
     // Track whether the initial overflow calculation is complete to prevent CLS
     const [isOverflowMeasured, setIsOverflowMeasured] = useState(false)
 
-    const validChildren = getValidChildren(children)
+    // Memoize so a re-render with unchanged children doesn't invalidate the
+    // measurement layout effect's deps.
+    const validChildren = useMemo(() => getValidChildren(children), [children])
 
     // Responsive props object manages which items are in the list and which items are in the menu.
     const [responsiveProps, setResponsiveProps] = useState<ResponsiveProps>({
@@ -204,7 +210,7 @@ export const UnderlineNav = forwardRef(
     }
 
     function getItemsWidth(itemText: string): number {
-      return noIconChildWidthArray.find(item => item.text === itemText)?.width ?? 0
+      return noIconChildWidthArrayRef.current.find(item => item.text === itemText)?.width ?? 0
     }
 
     const swapMenuItemWithListItem = (
@@ -250,7 +256,18 @@ export const UnderlineNav = forwardRef(
 
     const updateListAndMenu = useCallback(
       (props: ResponsiveProps, displayIcons: boolean, overflowMeasured: boolean) => {
-        setResponsiveProps(props)
+        // Equality guards: the ResizeObserver fires an initial entry on
+        // observe() with the same dimensions the parent layout effect
+        // already measured. Without these guards that initial fire
+        // commits an extra render with identical responsiveProps.
+        setResponsiveProps(prev => {
+          const sameItems =
+            prev.items.length === props.items.length && prev.items.every((item, i) => item.key === props.items[i].key)
+          const sameMenu =
+            prev.menuItems.length === props.menuItems.length &&
+            prev.menuItems.every((item, i) => item.key === props.menuItems[i].key)
+          return sameItems && sameMenu ? prev : props
+        })
         setIconsVisible(displayIcons)
 
         if (overflowMeasured) {
@@ -259,19 +276,6 @@ export const UnderlineNav = forwardRef(
       },
       [],
     )
-    const setChildrenWidth = useCallback((size: ChildSize) => {
-      setChildWidthArray(arr => {
-        const newArr = [...arr, size]
-        return newArr
-      })
-    }, [])
-
-    const setNoIconChildrenWidth = useCallback((size: ChildSize) => {
-      setNoIconChildWidthArray(arr => {
-        const newArr = [...arr, size]
-        return newArr
-      })
-    }, [])
 
     const closeOverlay = React.useCallback(() => {
       setIsWidgetOpen(false)
@@ -301,6 +305,47 @@ export const UnderlineNav = forwardRef(
 
     useOnOutsideClick({onClickOutside: closeOverlay, containerRef, ignoreClickRefs: [moreMenuBtnRef]})
 
+    // Measure all items in one parent-driven layout effect instead of having
+    // each UnderlineNavItem dispatch its own setState. Two wins:
+    //   1. N children → 1 setState (was 2N), so far fewer batched updates queued.
+    //   2. The measurements feed `overflowEffect` synchronously here, so the
+    //      ResizeObserver's initial fire becomes a no-op (equality-guarded).
+    useIsomorphicLayoutEffect(() => {
+      if (!listRef.current || !navRef.current) return
+
+      const itemEls = listRef.current.querySelectorAll<HTMLElement>('[data-underline-nav-item]')
+      if (itemEls.length === 0) return
+
+      const childWidths: ChildWidthArray = []
+      const noIconChildWidths: ChildWidthArray = []
+
+      for (const li of itemEls) {
+        const inner = li.firstElementChild as HTMLElement | null
+        if (!inner) continue
+        const rect = inner.getBoundingClientRect()
+        const textEl = inner.querySelector('[data-component="text"]')
+        const iconEl = inner.querySelector<HTMLElement>('[data-component="icon"]')
+        if (!textEl) continue
+        const text = textEl.textContent
+        const iconWidthWithMargin = iconEl
+          ? iconEl.getBoundingClientRect().width +
+            Number(getComputedStyle(iconEl).marginRight.slice(0, -2)) +
+            Number(getComputedStyle(iconEl).marginLeft.slice(0, -2))
+          : 0
+        childWidths.push({text, width: rect.width})
+        noIconChildWidths.push({text, width: rect.width - iconWidthWithMargin})
+      }
+
+      childWidthArrayRef.current = childWidths
+      noIconChildWidthArrayRef.current = noIconChildWidths
+
+      const navWidth = navRef.current.getBoundingClientRect().width
+      const moreMenuWidth = moreMenuRef.current?.getBoundingClientRect().width ?? 0
+      if (navWidth !== 0) {
+        overflowEffect(navWidth, moreMenuWidth, validChildren, childWidths, noIconChildWidths, updateListAndMenu)
+      }
+    }, [validChildren, updateListAndMenu])
+
     useResizeObserver((resizeObserverEntries: ResizeObserverEntry[]) => {
       const navWidth = resizeObserverEntries[0].contentRect.width
       const moreMenuWidth = moreMenuRef.current?.getBoundingClientRect().width ?? 0
@@ -309,8 +354,8 @@ export const UnderlineNav = forwardRef(
           navWidth,
           moreMenuWidth,
           validChildren,
-          childWidthArray,
-          noIconChildWidthArray,
+          childWidthArrayRef.current,
+          noIconChildWidthArrayRef.current,
           updateListAndMenu,
         )
     }, navRef as RefObject<HTMLElement>)
@@ -333,8 +378,6 @@ export const UnderlineNav = forwardRef(
     return (
       <UnderlineNavContext.Provider
         value={{
-          setChildrenWidth,
-          setNoIconChildrenWidth,
           loadingCounters,
           iconsVisible,
         }}

packages/react/src/UnderlineNav/UnderlineNavContext.tsx

@@ -1,14 +1,9 @@
-import type React from 'react'
 import {createContext} from 'react'
 
 export const UnderlineNavContext = createContext<{
-  setChildrenWidth: React.Dispatch<{text: string; width: number}>
-  setNoIconChildrenWidth: React.Dispatch<{text: string; width: number}>
   loadingCounters: boolean
   iconsVisible: boolean
 }>({
-  setChildrenWidth: () => null,
-  setNoIconChildrenWidth: () => null,
   loadingCounters: false,
   iconsVisible: true,
 })

packages/react/src/UnderlineNav/UnderlineNavItem.tsx

@@ -1,9 +1,8 @@
-import type {MutableRefObject, RefObject} from 'react'
-import React, {forwardRef, useRef, useContext} from 'react'
+import type {RefObject} from 'react'
+import React, {forwardRef, useContext} from 'react'
 import type {IconProps} from '@primer/octicons-react'
 import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
 import {UnderlineNavContext} from './UnderlineNavContext'
-import useLayoutEffect from '../utils/useIsomorphicLayoutEffect'
 import {UnderlineItem} from '../internal/components/UnderlineTabbedInterface'
 import classes from './UnderlineNavItem.module.css'
 
@@ -74,33 +73,8 @@ export const UnderlineNavItem = forwardRef(
     },
     forwardedRef,
   ) => {
-    const backupRef = useRef<HTMLElement>(null)
-    const ref = (forwardedRef ?? backupRef) as RefObject<HTMLAnchorElement>
-    const {setChildrenWidth, setNoIconChildrenWidth, loadingCounters, iconsVisible} = useContext(UnderlineNavContext)
-
-    useLayoutEffect(() => {
-      if (ref.current) {
-        const domRect = (ref as MutableRefObject<HTMLElement>).current.getBoundingClientRect()
-
-        const icon = Array.from((ref as MutableRefObject<HTMLElement>).current.children).find(
-          child => child.getAttribute('data-component') === 'icon',
-        )
-
-        const content = Array.from((ref as MutableRefObject<HTMLElement>).current.children).find(
-          child => child.getAttribute('data-component') === 'text',
-        ) as HTMLElement
-        const text = content.textContent as string
-
-        const iconWidthWithMargin = icon
-          ? icon.getBoundingClientRect().width +
-            Number(getComputedStyle(icon).marginRight.slice(0, -2)) +
-            Number(getComputedStyle(icon).marginLeft.slice(0, -2))
-          : 0
-
-        setChildrenWidth({text, width: domRect.width})
-        setNoIconChildrenWidth({text, width: domRect.width - iconWidthWithMargin})
-      }
-    }, [ref, setChildrenWidth, setNoIconChildrenWidth])
+    const ref = forwardedRef as RefObject<HTMLAnchorElement> | undefined
+    const {loadingCounters, iconsVisible} = useContext(UnderlineNavContext)
 
     const keyDownHandler = React.useCallback(
       (event: React.KeyboardEvent<HTMLAnchorElement>) => {
@@ -120,7 +94,10 @@ export const UnderlineNavItem = forwardRef(
     )
 
     return (
-      <li className={classes.UnderlineNavItem}>
+      // `data-underline-nav-item` lets the parent UnderlineNav measure all
+      // items in a single layout effect instead of each item dispatching its
+      // own setState. See UnderlineNav.tsx.
+      <li className={classes.UnderlineNavItem} data-underline-nav-item="">
         <UnderlineItem
           ref={ref}
           as={Component}

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 React from 'react'
 import {render, screen} from '@testing-library/react'
 import {describe, it, afterEach, expect, vi} from 'vitest'
 import UnderlinePanels from './UnderlinePanels'
@@ -145,4 +146,25 @@ describe('UnderlinePanels', () => {
       )
     }).toThrow('Only one tab can be selected at a time.')
   })
+
+  it('does not cascade renders on mount (tabs derived inline, no list-width state)', async () => {
+    // Previously the component held `tabs`/`tabPanels` in state and synced
+    // them via a post-paint `useEffect`, producing one extra render every
+    // time children or `iconsVisible` changed. It also held the list width
+    // in state, with the initial measurement triggering a separate
+    // layout-effect commit. Both are now derived inline (useMemo + folded
+    // into the iconsVisible decision), so mount produces at most one
+    // measurement-driven update on top of the initial commit.
+    let commitCount = 0
+    render(
+      <React.Profiler id="UnderlinePanelsCascadeTest" onRender={() => commitCount++}>
+        <UnderlinePanelsMockComponent aria-label="Select a tab" />
+      </React.Profiler>,
+    )
+
+    // Allow ResizeObserver's initial entry to flush.
+    await new Promise(resolve => setTimeout(resolve, 0))
+
+    expect(commitCount).toBeLessThanOrEqual(2)
+  })
 })