Uh oh!
There was an error while loading. Please reload this page.
perf(PageLayout): eliminate ~614ms forced reflow from getComputedStyle on mount - #7532
Conversation
🦋 Changeset detectedLatest commit: 8fdeef7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the |
There was a problem hiding this comment.
Pull request overview
This PR removes a major mount-time performance bottleneck in PageLayout by eliminating a getComputedStyle() call that can force expensive synchronous layout recalculation. It replaces the DOM/CSS variable read with a viewport-width-based lookup derived from the same breakpoint logic already encoded in PageLayout.module.css.
Changes:
- Add
getMaxWidthDiffFromViewport()and use it on mount and when crossing the 1280px breakpoint, avoidinggetComputedStyle()forced reflow. - Export the wide breakpoint
--pane-max-width-diffvalue (959) fromPageLayout.module.cssvia:exportto keep JS/CSS in sync. - Update and extend unit tests to reflect correct wide-breakpoint behavior and cover the new helper.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/react/src/PageLayout/usePaneWidth.ts | Replaces mount and resize-breakpoint diff calculation with a viewport-only lookup helper. |
| packages/react/src/PageLayout/usePaneWidth.test.ts | Updates expectations for wide breakpoint behavior and adds new helper unit tests. |
| packages/react/src/PageLayout/PageLayout.module.css | Exposes the wide diff value (paneMaxWidthDiffWide) via :export for JS consumption. |
| .changeset/pagelayout-remove-reflow.md | Adds a patch changeset describing the performance fix. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
francinelucca
left a comment
There was a problem hiding this comment.
looking good! some questions/suggestions
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
hectahertz
commented
Feb 13, 2026
@francinelucca everything should be addressed now! |
liuliu-dev
left a comment
There was a problem hiding this comment.
Nice fix! Learned some tricks in this pr 💖
Replace getPaneMaxWidthDiff (which calls getComputedStyle, forcing a synchronous layout recalc) with getMaxWidthDiffFromViewport, a pure JS function that derives the same value from window.innerWidth. The CSS variable --pane-max-width-diff only has two values controlled by a single @media (min-width: 1280px) breakpoint, so a simple threshold check is semantically equivalent — no DOM query needed. This eliminates ~614ms of blocking time on mount for pages with large DOM trees (e.g. SplitPageLayout).
…romViewport tests
Replace vi.spyOn(window, 'innerWidth', 'get').mockReturnValue(...) with
vi.stubGlobal('innerWidth', ...) to prevent spy leaks. The outer describe
block's afterEach calls vi.unstubAllGlobals(), which correctly cleans up
stubGlobal but does not restore spyOn mocks. This makes the tests
consistent with the rest of the file and avoids order-dependent failures.…nments Add canUseDOM check so the function returns DEFAULT_MAX_WIDTH_DIFF instead of throwing when window is unavailable (SSR, node tests, build-time evaluation). canUseDOM was already imported in the file.
The function is no longer called after replacing it with getMaxWidthDiffFromViewport(). Keeping it around is a footgun — it calls getComputedStyle which forces synchronous layout. Remove it and its associated tests.
The test previously stubbed innerWidth without firing a resize event, then asserted against a stale maxWidthDiffRef — a scenario that cannot occur in a real browser. Dispatch the resize event so the breakpoint crossing updates the cached diff value, making the assertion realistic.
c8c9aac to
8fdeef7Compare👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/14285 |
Closes #
SplitPageLayout'susePaneWidthhook callsgetComputedStyle()inside auseLayoutEffecton mount, forcing a synchronous layout. This is the single largest source of client-side latency across profiled Files controller routes on github.com.Impact on github.com (production profiling)
getPaneMaxWidthDiffOn the Code View page,
getPaneMaxWidthDiffalone forces the browser to synchronously lay out 430 pending DOM nodes in a single 529ms layout update.The fix
The CSS variable
--pane-max-width-diffonly has two possible values, controlled by a single@media (min-width: 1280px)breakpoint:511px959pxThis is fully determinable from
window.innerWidth, nogetComputedStyleneeded:The same replacement is applied to the resize handler's breakpoint-crossing path.
Chrome DevTools Performance Trace Results
Profiled on the Heavy Content performance story (~5,400 DOM nodes, 1,366 flex containers).
PageLayout-caused Forced Reflow:
Style Recalculation:
LCP (4x CPU throttle):
Changelog
New
getMaxWidthDiffFromViewport(): derives the--pane-max-width-diffvalue fromwindow.innerWidthwithout touching the DOMChanged
usePaneWidthuses the viewport-based lookup instead ofgetComputedStyle959) fromPageLayout.module.cssvia:exportto keep JS and CSS in syncRemoved
N/A
Rollout strategy
Testing & Reviewing
usePaneWidthtests pass, with expectations corrected to reflect real-world breakpoint behavior at viewport >= 1280pxgetMaxWidthDiffFromViewportcovering below, at, and above the 1280px breakpointgetComputedStylealways returned the default (511) even at 1280px viewport. The new viewport-based approach correctly returns 959 at that width.Merge checklist