Skip to content

perf(test): make role+name queries O(match) in the date-component suites#3816

Merged
cixzhang merged 5 commits into
facebook:mainfrom
Han5991:perf/daterange-test-speed
Jul 12, 2026
Merged

perf(test): make role+name queries O(match) in the date-component suites#3816
cixzhang merged 5 commits into
facebook:mainfrom
Han5991:perf/daterange-test-speed

Conversation

@Han5991

@Han5991 Han5991 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Summary

The four date-component suites were the slowest files in packages/core, and profiling showed why: their month grid keeps ~35–85 role=button nodes mounted (visible in Calendar's case, inside the closed popover for the *Input components), and RTL's getByRole('button', {name}) computes the accessible name of every candidate before filtering — each computation consulting jsdom's slow getComputedStyle (~5ms/node). One trigger lookup: ~450ms. A name-less getAllByRole('button', {hidden: true}): 29ms.

Changes

  • New shared helper packages/core/src/__tests__/fastRoleQueries.ts (same convention as TestIcon.tsx): getButton()/queryButton() keep RTL's exact name algorithm (dom-accessibility-api, the library RTL uses internally) but collect candidates with {hidden: true} and inject a constant "visible" style stub into the name computation — under {hidden: true} semantics visibility must not exclude candidates, so the stub removes only the getComputedStyle cost. Documented trade-off: first-match-wins, no tree-wide uniqueness check.
  • All button role+name queries in the four suites now use the helpers (41 call sites). Queries that were already cheap (combobox/tooltip/grid — few candidates or no name filter) stay stock RTL.
  • dom-accessibility-api declared as a root devDependency: it already ships in node_modules as a transitive dep, but without the declaration the hoisted copy is RTL's 0.5.16, whose exports map has no types conditiontsc fails with TS7016 (implicitly has an 'any' type), which is exactly what broke this PR's typecheck when the declaration was briefly dropped. ^0.6.3 pins the copy whose exports carry types.

Measurements (local, sequential runs on the same machine)

file before after
DateRangeInput.test.tsx (34 tests) 34.3s 1.3s
Calendar.test.tsx (45 tests) 7.2s 1.9s
DateInput.test.tsx (68 tests) 5.1s 2.2s
DateTimeInput.test.tsx (64 tests) 4.1s 1.8s

Full suite: 5,893 tests green, wall 105.9s → 92.8s, worker-summed tests phase 297.7s → 235.1s. Core typecheck green. All tests assert the same things they did before.

@vercel

vercel Bot commented Jul 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview, Comment Jul 12, 2026 1:26am

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 11, 2026
@Han5991
Han5991 marked this pull request as ready for review July 11, 2026 12:02
@Han5991
Han5991 marked this pull request as draft July 11, 2026 12:18
@Han5991 Han5991 changed the title perf(test): make DateRangeInput role+name queries O(match) instead of O(all buttons) perf(test): make role+name queries O(match) in the date-component suites Jul 11, 2026
@Han5991
Han5991 force-pushed the perf/daterange-test-speed branch from 0d6ef23 to 25bbcb6 Compare July 11, 2026 12:27
@Han5991
Han5991 marked this pull request as ready for review July 11, 2026 12:31
@github-actions github-actions Bot added needs:code-review High-risk change (new package/component/API) — needs human code review before merge community Authored by a community contributor (not on the eng/design team) labels Jul 11, 2026
@Han5991
Han5991 force-pushed the perf/daterange-test-speed branch from af294c9 to 3d5e2c2 Compare July 11, 2026 21:10
@github-actions

github-actions Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

PR Analysis Report

No new or modified components detected.

Bundle Size Summary

Package Size (ESM) Size (CJS) Gzipped
@astryxdesign/core N/A 4.6KB 0B

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | View full report

@cixzhang cixzhang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for speeding up this test!

@cixzhang
cixzhang enabled auto-merge (squash) July 11, 2026 22:41
Han5991 added 5 commits July 12, 2026 10:21
… O(all buttons)

DateRangeInput.test.tsx was the slowest file in the suite: 34.3s for 34
tests (~1s each), 10x slower per test than DateInput's. Profiling showed
the tests' render is cheap (~20ms) — the cost was every
getByRole('button', {name}) query: the closed popover keeps a two-month
Calendar (~85 role=button nodes) permanently mounted, RTL computes the
accessible name of every candidate before filtering, and each
computation consults jsdom's slow getComputedStyle (~5ms/node). One
trigger lookup: ~450ms. A name-less getAllByRole with hidden:true runs
in 29ms.

Replace the 23 button role+name queries with file-local getButton()/
queryButton() helpers that use the same name algorithm RTL uses
internally (dom-accessibility-api, promoted from transitive to explicit
devDependency) but inject a constant "visible" style stub — with
{hidden: true} semantics, visibility must not exclude candidates anyway,
so the stub only removes the getComputedStyle cost, not correctness.
Trade-off, documented in the helper: first-match-wins, no uniqueness
check across the tree.

File time: 34.3s -> 5.7s wall (tests 34.3s -> 1.7s). All 34 tests pass
unchanged in what they assert; core typecheck and the neighboring
DateInput/DateTimeInput/Calendar suites (218 tests) stay green.

Candidates for a follow-up using the same pattern via a shared
test-utils helper: Calendar.test.tsx (12s), DateInput (7.5s),
DateTimeInput (5.9s).
…dency"

The undeclared import type-checks only by luck of which copy wins the
shamefully-hoist: without the declaration, RTL's transitive 0.5.16 lands
at the root, and its package.json exports map has no types condition
(import -> dist/index.mjs with no index.d.mts), so tsc fails with
TS7016 'implicitly has an any type' — exactly what broke the PR's
typecheck job. Declaring ^0.6.3 pins the copy whose exports carry
types. Runtime never noticed; the type graph is why the declaration is
needed today, not later.
…DateTimeInput

Promote DateRangeInput's file-local getButton/queryButton helpers to a
shared packages/core/src/__tests__/fastRoleQueries.ts (same convention
as TestIcon.tsx) and apply them to the other three date-component
suites, which pay the same tax: every getByRole('button', {name}) query
computes accessible names for the whole mounted month grid through
jsdom's slow getComputedStyle.

Measured locally (same machine, sequential runs):
  Calendar.test.tsx      7.2s -> 1.9s  (45 tests)
  DateInput.test.tsx     5.1s -> 2.2s  (68 tests)
  DateTimeInput.test.tsx 4.1s -> 1.8s  (64 tests)
  DateRangeInput.test.tsx stays at ~1.3s with the shared helper

Full suite: 5893 tests green, wall 105.9s -> 92.8s, worker-summed
'tests' phase 297.7s -> 235.1s. Core typecheck green. Queries that were
already cheap (combobox/tooltip/grid — few candidates or no name
filter) are left as stock RTL.
Lint flagged the `{...} as CSSStyleDeclaration` object-literal cast
(@typescript-eslint/consistent-type-assertions wants `const x: T = {...}`).
A plain `: CSSStyleDeclaration` annotation can't work — the interface has
hundreds of required members and the stub only implements getPropertyValue.

Type the stub honestly as Pick<CSSStyleDeclaration, 'getPropertyValue'>
(no literal cast, typechecks) and move the widening to the call site as an
identifier assertion, which the rule's objectLiteralTypeAssertions option
does not govern. Lint clean, core typecheck green, all four date suites
(252 tests) pass.
auto-merge was automatically disabled July 12, 2026 01:23

Head branch was pushed to by a user without write access

@Han5991
Han5991 force-pushed the perf/daterange-test-speed branch from 3d5e2c2 to 8f456a5 Compare July 12, 2026 01:23
@github-actions github-actions Bot removed the needs:code-review High-risk change (new package/component/API) — needs human code review before merge label Jul 12, 2026
@Han5991

Han5991 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

@cixzhang I resolved the conflicts and pushed the changes. Please review it again.

@cixzhang
cixzhang merged commit 8a9d3a2 into facebook:main Jul 12, 2026
17 checks passed
@Han5991
Han5991 deleted the perf/daterange-test-speed branch July 12, 2026 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. community Authored by a community contributor (not on the eng/design team)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants