Skip to content

Performance audit: prioritized fixes for render cost, listener leaks, and bundle weight #8676

Open

Description

@mattcosta7

Summary

An audit of @primer/react for runtime and bundle performance problems, ranked by impact. Each item below was checked against the code at 57c2ac594, and most were reproduced with a throwaway Vitest browser test (React 18.3.1, Chromium headless, React development build). Fixes must work on React 18 and 19, so they use only useSyncExternalStore, startTransition, useDeferredValue, React.memo, forwardRef, and the existing internal/hooks/useEffectCallback.

Absolute timings come from a development build, so treat them as relative. Re-render counts do not depend on the build.

Items are independent. Each can be its own PR, and the list can be split into separate issues if that is easier to track.

Not duplicated here

Already in flight: #8562 (stable Escape and outside-click callbacks), #8563 to #8567 (React Compiler for ActionMenu, ButtonBase, LabelGroup, Autocomplete, Dialog), #8609 (LinkItem remount), #8610 (Breadcrumbs overflow). Recent merges already removed many :has() selectors (#8115, #7901), moved useMedia to useSyncExternalStore (#8001), and cached the scroll-ancestor walk (#8116).

P0: bugs that cost performance

  • ActionMenu leaks a keydown handler on every open and close. hooks/useMenuKeyboardNavigation.ts:112 returns () => anchor?.addEventListener('keydown', handler) from its cleanup. It should call removeEventListener.
    • Measured on a real ActionMenu: live keydown handlers on the anchor after each of 5 open/close cycles were 5, 7, 9, 11, 13. Two handlers leak per cycle.
    • Each stale handler closes over the menu container from that open, so detached menu DOM stays reachable for the life of the anchor (inferred from the closure, not heap-measured).
    • Not observed: stale handlers moving focus in the real menu. They only did so in a harness where the container stayed attached.
    • Fix is one word plus a regression test that opens and closes the menu several times and asserts the handler count.
  • Open anchored overlays re-render when nothing moved. hooks/useAnchoredPosition.ts:91-106 always returns a new object from the setPosition updater, so React never bails out. The window-size observer at :142 calls useResizeObserver with no target, which observes document.documentElement, so any page height change triggers an update.
    • Measured: an open AnchoredOverlay committed 2 times across 5 document height changes below the anchor, with the position unchanged. The control, with no page change, committed 0 times in 300 ms.
    • Affects every open ActionMenu, SelectPanel, Autocomplete, and AnchoredOverlay.
    • Fix: return prev when top, left, and anchorSide are unchanged. Listen to window resize for viewport changes, as PageLayout/usePaneWidth.ts already does.
    • Related, not measured: updatePosition reads settings but the deps array at :115 leaves it out, so side or align changes while open may not reposition.
  • DataTable bulk actions rebuild a MutationObserver on document.body every render. DataTable/TableBulkActions.tsx:58-63 is a layout effect with no dependency array. observeFocusedAction observes body with {childList, subtree}. onFocusCapture at :75 already calls it. Fix: drop the effect or depend on selectedCount. Code verified, not measured.

P1: costs that grow with list size or instance count

  • Toggling one DataTable row re-renders every cell. DataTable/useTable.ts:292-318 creates new row and cell objects on every render, with selected baked in. TableRow and TableCell in Table.tsx are not memoized.
    • Measured: 200 rows by 3 columns, mounting calls renderCell 600 times. Toggling one row's checkbox calls it 600 more times.
    • Passing a new data array also re-sorts and rebuilds all rows (useTable.ts:175-229).
    • Fix: cache row objects by source row plus selected and selectable, add a memoized row component, and document stable data and columns.
  • SelectPanel and FilteredActionList re-render every item per keystroke. FilteredActionList.tsx:622-650 passes an inline onSelect to ActionList.Item, and SelectPanel.tsx:688-730 builds an inline onAction per item. No ActionList, FilteredActionList, SelectPanel, DataTable, or TreeView component uses React.memo (grep returns nothing).
    • Measured: 300 items with a counting leading visual. Typing one character that matches every item re-rendered all 300.
    • FilteredActionList/useAnnouncements.tsx runs JSON.stringify over all items and document.querySelector('live-region') on every render, even with announcements disabled (code verified, not measured).
    • Fix: React.memo on MappedActionListItem and ActionList.Item, route onSelect through a ref, useMemo the announcement state, and defer filtering with useDeferredValue, as Autocomplete/AutocompleteMenu.tsx:229-236 does. Memoization only pays off when consumers keep item identity stable, so measure with both stable and inline items.
  • Focusing a TreeView parent re-renders its whole expanded subtree. TreeView/TreeView.tsx:483-497 passes a new object to ItemContext.Provider on every Item render, and isFocused state at :294 changes on focus and blur. The file is not React Compiler compiled.
    • Measured: summed update-commit time after tabbing to an expanded parent, median of 7 runs: 0.5 ms with 10 children, 2.7 ms with 100, 9.8 ms with 400. That is linear in subtree size.
    • Fix: useMemo the context value, or compile the file.
  • Every labelled IconButton mounts a TooltipV2. Button/IconButton.tsx:40-60 wraps the button in a Tooltip unless unsafeDisableTooltip is set. Each instance registers a global Escape handler (TooltipV2/Tooltip.tsx:289) and runs its accessibility DOM scan on mount in production (:237-288; warning and invariant are no-ops in production but their arguments are still computed). hooks/useOnEscapePress.ts calls Object.keys(registry) on every register and unregister.
    • Measured, median of 5 runs, N IconButtons:

      N mount, tooltip mount, none unmount, tooltip unmount, none
      300 15 ms 9 ms 1.3 ms 0.3 ms
      1000 48 ms 26 ms 10.1 ms 0.8 ms
      3000 200 ms 73 ms 75.0 ms 2.8 ms
    • Unmount is clearly super-linear (about 58x for 10x the items). Mount is mildly super-linear. At a few hundred buttons the cost is small.

    • Fix, after perf(hooks): stabilize Escape and outside-click callbacks #8562: subscribe to Escape only while the tooltip is open, wrap the accessibility scan in __DEV__, and replace Object.keys(registry).length with a counter. Do not lazy-mount the tooltip element, because aria-labelledby needs it in the DOM.

  • No shared media-query store. hooks/useResponsiveValue.ts:54-56 always calls useMedia three times, even when the value is not responsive. hooks/useMedia.ts:54 calls window.matchMedia(...) inside getSnapshot, so every render allocates a new MediaQueryList.
  • Extend React Compiler coverage. After perf(react-compiler): enable ActionMenu #8563 to perf(Dialog): enable React Compiler #8567, the remaining hot files in script/react-compiler.mjs unsupportedPatterns are FilteredActionList.tsx, SelectPanel.tsx, TreeView.tsx, AnchoredOverlay.tsx, and the hooks useAnchoredPosition, useFocusTrap, useFocusZone, useResizeObserver. The build targets React 18 with a bundled useMemo fallback (rolldown.config.ts, utils/react-compiler-runtime.ts), so this is safe on both versions. Use the DataTable, SelectPanel, and TreeView measurements above as the before and after check.

P2: bundle weight

  • The legacy JS theme ships with the root ThemeProvider. ThemeProvider.tsx:5 imports theme.ts, which imports legacy-theme/ts/color-schemes.ts (327 KB of source). index.ts:3 also exports theme.
    • Measured with esbuild on theme.ts alone: 275 KB minified, 20 KB gzipped. The gzip size is small because the data repeats, so parse and evaluation cost follows the 275 KB.
    • The per-provider deepmerge(theme, scheme) is cheap: 0.4 to 0.6 ms warm and 1.5 to 3 ms on the first call (Node, 50 runs after warm-up, two runs).
    • @primer/react/next already avoids the theme import, and the root ThemeProvider is already deprecated in favor of it. Removing the default theme from the root entry is a breaking change, so it needs a major version and a decision on timing.
  • Dependencies with no production use. hsluv has no imports anywhere in the repo except package.json. react-intersection-observer is imported only by two test files, yet is listed under dependencies. Both affect install size only.
    • history looks removable (type imports only) but its types appear in public props such as to?: To in Breadcrumbs.tsx:378, so it should stay.

P3: small and local

  • Button/ButtonBase.module.css:28 and :217 use :has([data-kbd-chord]) on every Button. Set a data-* attribute from React instead, as ActionList already does.
  • ActionList/ActionList.module.css:271, :280, and :346 select every descendant (*) of inactive or disabled items. Set the color on the content wrapper and let children inherit.
  • PageLayout/usePaneWidth.ts:404-410 calls setMaxPaneWidth in startTransition whenever the computed max differs, which is likely every throttled tick while the window resizes. Keep the imperative writes per tick and sync React state once at the debounced end. The per-tick change is inferred, not measured.

Suggested order and verification

  1. P0 items, one small PR each, with a regression test where the behavior is observable.
  2. A repeatable benchmark before the P1 work, so each PR reports before and after numbers. The measurements above came from throwaway tests that render with counting components and the React Profiler. They can become Vitest browser benchmarks or Playwright traces over the existing stress stories (TreeView.features.stories.tsx, PageLayout.performance.stories.tsx), plus new stories for 1000-item SelectPanel typing, a 1000-row DataTable toggle, and a page of 1000 IconButtons. Run each on React 18 and 19.
  3. P1 items in the order listed, then P2 and P3.

Not verified

  • No measurement on React 19. The leak and re-render counts do not depend on the React version, but timings might.
  • Timings are from a development build in headless Chromium on one machine, not a production build on throttled hardware.
  • The retained-DOM effect of the keydown leak is inferred from the closure, not measured with a heap snapshot.
  • The useAnchoredPosition stale-settings correctness concern and the usePaneWidth per-tick state update are read from code only.
  • Consumer-side usage (real list sizes, whether items and data are stable) is unknown, and it determines the payoff of the memoization work.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions