perf(test): make role+name queries O(match) in the date-component suites#3816
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Han5991
marked this pull request as ready for review
July 11, 2026 12:02
Han5991
requested review from
cixzhang,
ejhammond,
imdreamrunner,
josephfarina,
marie-lucas,
nynexman4464 and
thedjpetersen
as code owners
July 11, 2026 12:02
Han5991
marked this pull request as draft
July 11, 2026 12:18
Han5991
force-pushed
the
perf/daterange-test-speed
branch
from
July 11, 2026 12:27
0d6ef23 to
25bbcb6
Compare
Han5991
marked this pull request as ready for review
July 11, 2026 12:31
Han5991
force-pushed
the
perf/daterange-test-speed
branch
from
July 11, 2026 21:10
af294c9 to
3d5e2c2
Compare
Contributor
PR Analysis ReportNo new or modified components detected. Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Generated by PR Enrichment workflow | View full report |
cixzhang
approved these changes
Jul 11, 2026
cixzhang
left a comment
Contributor
There was a problem hiding this comment.
Thanks for speeding up this test!
cixzhang
enabled auto-merge (squash)
July 11, 2026 22:41
… 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
force-pushed
the
perf/daterange-test-speed
branch
from
July 12, 2026 01:23
3d5e2c2 to
8f456a5
Compare
Contributor
Author
|
@cixzhang I resolved the conflicts and pushed the changes. Please review it again. |
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
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.
Summary
The four date-component suites were the slowest files in packages/core, and profiling showed why: their month grid keeps ~35–85
role=buttonnodes mounted (visible in Calendar's case, inside the closed popover for the*Inputcomponents), and RTL'sgetByRole('button', {name})computes the accessible name of every candidate before filtering — each computation consulting jsdom's slowgetComputedStyle(~5ms/node). One trigger lookup: ~450ms. A name-lessgetAllByRole('button', {hidden: true}): 29ms.Changes
packages/core/src/__tests__/fastRoleQueries.ts(same convention asTestIcon.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 thegetComputedStylecost. Documented trade-off: first-match-wins, no tree-wide uniqueness check.dom-accessibility-apideclared 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, whoseexportsmap has no types condition —tscfails with TS7016 (implicitly has an 'any' type), which is exactly what broke this PR's typecheck when the declaration was briefly dropped.^0.6.3pins the copy whose exports carry types.Measurements (local, sequential runs on the same machine)
Full suite: 5,893 tests green, wall 105.9s → 92.8s, worker-summed
testsphase 297.7s → 235.1s. Core typecheck green. All tests assert the same things they did before.