Skip to content

test(components): guard render cost with deterministic browser tests - #3569

Draft
mfal wants to merge 4 commits into
mainfrom
claude/pensive-bohr-9e9dfe
Draft

mfal wants to merge 4 commits into
mainfrom
claude/pensive-bohr-9e9dfe

Conversation

@mfal

@mfal mfal commented Oct 23, 2026

Copy link
Copy Markdown
Member

What & why

Turns findings of the perf harness (#3524) into ordinary browser tests. They count, never time, so they hold on shared CI runners and run in the existing browser job.

List (List.renderCost.browser.test.tsx)

  • Weight per item: at most 340 fibers / 35 DOM nodes for a rich item (Avatar, Heading + AlertBadge, Text, ContextMenu), and 92 / 13 for a simple one. Today's counts are 336 / 34 and 89 / 12. The file says how to move a ceiling, and a failure prints the new count.
  • Render functions: with stable render functions, textValue and onAction, neither a parent re-render nor a sort calls an item render function.
  • Item subtrees: a Profiler around each item shows whether its subtree commits, with or without a render-function call.
  • State updates come from outside, not from clicks. A click changes react-aria's global interaction modality, and every item follows it, so a clicked re-render reported 20 items even for state the list never sees.

DOM identity on a re-render with equal props, following the Markdown (3808e79) and CodeEditor (#3467) tests:

  • Table: rows and cells
  • Select, closed: the trigger and its field
  • Markdown: the CodeMirror editor of a code block

countFibers(container) in src/lib/dev/fiberTree.ts is reduced from the harness's fiber walker, because #3524 is not merged.

How the tests were verified

Each guarded property was broken temporarily, and its test failed:

break fails
Item memo comparator always false both render-function tests (20 and 40 calls)
+5 pass-through components in Heading both fiber ceilings (rich 341, simple 94)
+2 spans in Heading both DOM ceilings
key={Math.random()} on the Table scroll area, Select's Aria.Button, CodeMirror the matching identity test
Math.random() in the deps of Markdown's renderer memo the code-block test, plus the existing one
test.fails → test both, on expected 20 to be +0

WebKit: 3 runs in a row are green. Firefox does not start locally (macOS 27), so the non-blocking Firefox job is the first check of whether react-aria renders a different count there.

Issue

Part of #3528

Checklist

  • PR title is a Conventional Commit and matches the base branch above
  • The issue this PR finishes (if any) is linked with Closes #… (finishes none)
  • pnpm lint is clean and the touched browser tests pass (WebKit)
  • Generated code is committed (none affected)
  • User-facing strings: none
  • Docs: no public API change

🤖 Generated with Claude Code

mfal and others added 3 commits October 23, 2026 11:55
Counts instead of timings, so they hold on shared CI runners:

- Fibers and DOM nodes per item, with a ceiling a little above today's
  counts (rich item 336 / 34, simple 89 / 12). The test file says how to
  move a ceiling.
- A parent re-render and sorting call no item render function while the
  render functions, `textValue` and `onAction` are stable.

`countFibers` (src/lib/dev) walks the fiber tree of a `render()` container,
reduced from the perf harness in #3524.

Verified by breaking each property: +5 fibers or +2 DOM nodes per
Heading fail both ceilings; an Item memo that never holds fails both
render-count tests (20 and 40 calls).

Part of #3528

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ocks across re-renders

A parent re-render with equal props must keep the DOM nodes, as the
Markdown (3808e79) and CodeEditor (#3467) tests already require:

- Table: rows and cells
- Select, closed: the trigger button and its field
- Markdown: the CodeMirror editor of a code block

Verified by forcing a remount (a changing `key` in Table, Select and
CodeEditor) and by un-memoizing Markdown's renderers: each test fails.

Part of #3528

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Item render functions stay uncalled on a parent re-render and on a sort,
but a Profiler around each item shows all 20 subtrees commit anyway: the
rebuilt react-aria collection hands every GridListItem a new ListState
context (#3528). Both cases are `test.fails` until that cascade is gone.

An update next to the list updates no item, which shows the probe can
read 0. Updates are now triggered from outside instead of by clicking:
a click changes react-aria's global interaction modality, and every item
follows that, so a clicked re-render reported 20 items even for state
the list never sees.

Part of #3528

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mfal mfal self-assigned this Oct 23, 2026
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Coverage Report for ./packages/components/

Status Category Percentage Covered / Total
🔵 Lines 64.82% 1410 / 2175
🔵 Statements 64.28% 1449 / 2254
🔵 Functions 50.63% 318 / 628
🔵 Branches 58.94% 758 / 1286
File CoverageNo changed files found.
Generated in workflow #7462 for commit 37270a2 by the Vitest Coverage Report Action

@github-actions

Copy link
Copy Markdown
Contributor

🚀 Preview Deployment

Preview environments are ready:

Type URL
docs pr-3569.docs.review.flow-components.de
storybook pr-3569.storybook.review.flow-components.de

Images:

  • docs: ghcr.io/mittwald/flow/docs:pr-3569
  • storybook: ghcr.io/mittwald/flow/storybook:pr-3569

…ohr-9e9dfe

# Conflicts:
#	packages/components/src/components/Select/Select.browser.test.tsx

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant