Description
Problem
👋🏼 Hello, while investigating a report of performance lag on https://github.com/github/pull-requests/issues/24571, I did a bit of profiling with Copilot.
I've made some changes on our end, but I would love it if y'all would investigate the below findings.
When SplitPageLayout has a resizable pane (resizable={true}), the usePaneWidth hook registers a window.addEventListener("resize", handleResize) listener that calls syncAll() on every resize event (~60/sec during a drag).
syncAll() unconditionally calls:
startTransition(() => {
setMaxPaneWidth(actualMax);
if (wasClamped) {
setCurrentWidthState(actualMax);
}
});
Even when the computed actualMax hasn't changed from the previous value, setMaxPaneWidth(actualMax) still schedules a React concurrent render because React doesn't bail out of startTransition state updates that set the same value (unlike normal setState).
Impact
On a page with multiple components consuming SplitPageLayout context (e.g. GitHub PR conversation tab), this produces 11 React scheduler calls totaling ~108ms in a single animation frame during resize — with 4× CPU throttle, this pushes individual frames well beyond 100ms and creates visible jank.
Trace evidence
- Chrome Performance trace on a GitHub PR conversation page (4× CPU throttle)
- The
actualMax value doesn't change between most resize ticks (only changes when crossing the DEFAULT_PANE_MAX_WIDTH_DIFF_BREAKPOINT)
Suggested fix
Add a guard before the state update:
const syncAll = () => {
// ... existing logic ...
const actualMax = getMaxPaneWidthRef.current();
paneRef.current?.style.setProperty("--pane-max-width", `${actualMax}px`);
// Only trigger React re-render if values actually changed
const maxChanged = actualMax !== maxPaneWidthRef.current;
const wasClamped = currentWidthRef.current > actualMax;
if (maxChanged || wasClamped) {
startTransition(() => {
setMaxPaneWidth(actualMax);
if (wasClamped) {
setCurrentWidthState(actualMax);
}
});
}
};
This would skip the startTransition entirely when the computed max hasn't changed — which is the common case during horizontal resize that doesn't cross the breakpoint.
Additional context
- The inline style updates (
--pane-max-width, --pane-width) and ARIA updates are fine — they don't trigger React renders
- Consumers can't work around this without disabling
resizable entirely
Steps to reproduce
- Go to a Conversation page on a PR
- Throttle CPU
- Resize the window
- Observe lag
Version
v38.21.0
Browser
No response
Description
Problem
👋🏼 Hello, while investigating a report of performance lag on https://github.com/github/pull-requests/issues/24571, I did a bit of profiling with Copilot.
I've made some changes on our end, but I would love it if y'all would investigate the below findings.
When
SplitPageLayouthas a resizable pane (resizable={true}), theusePaneWidthhook registers awindow.addEventListener("resize", handleResize)listener that callssyncAll()on every resize event (~60/sec during a drag).syncAll()unconditionally calls:Even when the computed
actualMaxhasn't changed from the previous value,setMaxPaneWidth(actualMax)still schedules a React concurrent render because React doesn't bail out ofstartTransitionstate updates that set the same value (unlike normalsetState).Impact
On a page with multiple components consuming
SplitPageLayoutcontext (e.g. GitHub PR conversation tab), this produces 11 React scheduler calls totaling ~108ms in a single animation frame during resize — with 4× CPU throttle, this pushes individual frames well beyond 100ms and creates visible jank.Trace evidence
actualMaxvalue doesn't change between most resize ticks (only changes when crossing theDEFAULT_PANE_MAX_WIDTH_DIFF_BREAKPOINT)Suggested fix
Add a guard before the state update:
This would skip the
startTransitionentirely when the computed max hasn't changed — which is the common case during horizontal resize that doesn't cross the breakpoint.Additional context
--pane-max-width,--pane-width) and ARIA updates are fine — they don't trigger React rendersresizableentirelySteps to reproduce
Version
v38.21.0
Browser
No response