Conversation
Ports the scenarios from test/performance into the main test suite as inline snapshot tests, so a render count change shows up as a changed line in the PR diff instead of needing CI machinery to report it. createCommitPhases() wraps a tree in React.Profiler and records its commit phase sequence. React fires onRender once per commit, so this counts commits, not individual component renders. waitForQuiescence() lets follow-up work settle first, so a snapshot means the commits a scenario settles into rather than however many happened to land before the assertion ran. Chromium only: jsdom misses the layout driven commits that most of these components do. 10 instances rather than the benchmark's 200 to 500, since commit counts do not scale with instance count. Two scenarios are not ported. The mixed app density benchmark only measures total mount cost, which is a timing question. The scroll area benchmark commits a load dependent number of times because its scrollbars measure from a ResizeObserver, failing 2 of 10 runs under load no matter how long the tree is left to settle. Tabs is ported without Tabs.Indicator for the same reason. The timing suite under test/performance is untouched.
commit: |
Bundle size
PerformanceTotal duration: 1,161.29 ms -67.92 ms(-5.5%) | Renders: 76 (+0) | Paint: 1,845.97 ms -122.32 ms(-6.2%)
14 tests within noise — details Check out the code infra dashboard for more information about this PR. |
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
The recorded commit sequences are specific to React 19's scheduling, and the react-18 CircleCI workflow re-runs this whole suite with react and react-dom swapped through pnpm overrides, so the snapshots would fail there. Gates the nine scenario blocks with reactMajor < 19, the idiom already used for the React.Activity blocks in AccordionPanel and CollapsiblePanel. commitPhases.test.tsx stays ungated. It asserts helper behaviour rather than Base UI's internal commit counts, so it should hold on any supported React.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports the scenarios from
test/performanceinto the main test suite as inline snapshot tests. A render count change then shows up as a changed line in the PR diff, reviewed like any other code change, with no CI machinery behind it.Context: #5403 builds a per-PR render count check out of three workflows, a merge base collector run, a cross workflow artifact trust model and a comment upsert. This is the same signal without any of that. It covers only the render count half; timing is a separate question.
The helper
createCommitPhases()inpackages/react/test/commitPhases.tsx, exported from#test-utils:It replaces three hand rolled Profiler counters already in the suite (
DrawerProvider,CheckboxIndicator,useAnimationsFinishedeach roll their own counter, array push orvi.fn()).waitForQuiescence()matters: mounting schedules follow up work that lands a frame or more later, so without it a snapshot records however much happened to arrive before the assertion and varies run to run.What this measures
Profiler.onRenderfires once per commit, so these are commit counts, not per component render counts. React exposes no public API for the latter. That catches effect driven re-render cascades, double commits and extra commits per keystroke. It does not catch "every item re-rendered inside one commit", the regression class behind #4964. Neither does #5403, worth saying plainly so the check is not oversold.Sample of what gets recorded:
["mount"]["mount", "nested-update"]["mount", "update", "nested-update"]Choices
describe.skipIf(isJSDOM || reactMajor < 19). jsdom misses the layout driven commits that most of these components do. The recorded sequences are specific to React 19 scheduling, and thereact-18workflow re-runs this whole suite with react and react-dom swapped through pnpm overrides, so it has to skip them.commitPhases.test.tsxstays ungated: it asserts helper behaviour, not Base UI commit counts.Not ported
Mixed surface mount (app-like density)measures total mount cost across 330 components, which is a timing question.ResizeObserver, so the commit count depends on machine load: stable 15/15 in isolation but failing 2 of 10 combined runs, at any settle length tried.Tabs.Indicatoris left out of the tabs scenario for the same reason (8/10).Both are called out in comments where they would otherwise look like oversights.
Verification
test/performanceis untouched.