Skip to content

test(ui): contain deferred jsdom timers in harnesses - #3751

Merged
kojiwakayama merged 2 commits into
mainfrom
fix/popover-jsdom-timer-drain
Aug 16, 2026
Merged

test(ui): contain deferred jsdom timers in harnesses#3751
kojiwakayama merged 2 commits into
mainfrom
fix/popover-jsdom-timer-drain

Conversation

@kojiwakayama

Copy link
Copy Markdown
Contributor

Summary

  • Drain jsdom deferred selection work after each Popover conformance case.
  • Drain React scheduler cleanup after each Field behavior case.
  • Keep production component behavior and Deno leak sanitization unchanged.

Why

Deno leak sanitization could observe timers that jsdom 28 or React 19 scheduled during synchronous test cleanup after the owning BDD group had already finished. This made the full pre-push suite fail nondeterministically while validating #3723.

The drains stay local to the two independently reproduced jsdom harnesses. This PR does not weaken leak detection or alter runtime code.

Red evidence

  • Exact main Popover trace-leak run: 0 groups passed, 27 steps, 3 timer-leak failures from jsdom selection updates after focus.
  • Exact main Field trace-leak run: 0 groups passed, 8 steps, 1 timer-leak failure from the React scheduler after unmount.

Green evidence

  • Deno 2.7.7 Popover trace-leak run: 27 of 27 steps.
  • Four repeated Popover trace-leak runs: 108 of 108 steps.
  • Ten repeated Field trace-leak runs: 80 of 80 steps.
  • Popover, Field, and Floating trace-leak matrix: 42 of 42 steps.
  • Targeted Bun tests: 2 files passed.
  • Formatting, lint, typecheck, and diff checks passed.
  • Two correctly pinned full pre-push hooks passed at exact head: 3887 tests and 29290 steps, plus cwd and cwd-exclusion suites.

Scope

Test harness only. Unblocks renewed validation of #3723 after this baseline fix is reviewed and merged.

jsdom 28 schedules a selection update on a node timer after focus. Synchronous Popover conformance cleanup could leave that timer alive across the BDD group boundary, making Deno leak sanitization fail nondeterministically. Drain one macrotask after each shared conformance case so the harness owns the deferred work.

Constraint: Production Popover behavior and leak sanitization must remain unchanged
Rejected: Disable timer leak sanitization | hides real test leaks
Rejected: Make every conformance case async | broadens a one-hook harness fix
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep the drain inside the jsdom Popover harness unless jsdom removes the deferred selection timer
Tested: Deno 2.7.7 trace-leaks single and four repeated runs; Popover plus Floating pair 34 steps; Bun targeted; fmt, lint, check, diff-check
Not-tested: Hosted CI before push
React 19 can leave one scheduler setImmediate timer pending after the jsdom Field root is unmounted. Drain one macrotask after each behaviour case so Deno leak sanitization observes the completed scheduler cleanup inside the owning BDD group.

Constraint: Production Field behavior and leak sanitization must remain unchanged
Rejected: Disable timer leak sanitization | hides real resource leaks
Rejected: Change shared BDD teardown globally | exceeds the two proven jsdom harness failures
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep this drain local to the Field jsdom harness unless a shared React harness boundary is introduced with broad evidence
Tested: Exact-main trace-leaks red; ten repeated Deno 2.7.7 trace-leaks green; Popover plus Field plus Floating 42 steps; Bun two files; fmt, lint, check, diff-check
Not-tested: Hosted CI before push
@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@kojiwakayama, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 11 minutes

Limit details: You’ve used all 3 included reviews currently available under your plan.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 11a9c694-8704-4b23-98b2-467dc33117fc

📥 Commits

Reviewing files that changed from the base of the PR and between 65a16a3 and 2d91b26.

📒 Files selected for processing (2)
  • src/react/components/ui/adapter/popover.conformance.test.tsx
  • src/react/components/ui/field.test.tsx

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

📦 Client bundle boundary

Entrypoint Modules Source size Server leaks
src/index.client.ts 454 3065 KiB ⚠️ 39 known

A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in scripts/lint/client-bundle-baseline.json to burn down.

@kojiwakayama
kojiwakayama marked this pull request as ready for review August 16, 2026 01:01
@kojiwakayama
kojiwakayama added this pull request to the merge queue Aug 16, 2026
Merged via the queue into main with commit f30d380 Aug 16, 2026
34 checks passed
@kojiwakayama
kojiwakayama deleted the fix/popover-jsdom-timer-drain branch August 16, 2026 01:18
kojiwakayama added a commit that referenced this pull request Aug 16, 2026
Bring the merged jsdom and React scheduler harness drains into the tenant-error branch so its full leak-sanitized gate can complete without bypasses. Preserve the existing tenant classification hardening, scanner reconciliation, and linear member-name scan.

Constraint: PR #3723 must integrate current main non-destructively after #3751 merged
Rejected: Rebase or force-push the branch | would rewrite reviewed history
Confidence: high
Scope-risk: moderate
Reversibility: clean
Directive: Preserve both the mainline Popover and Field drains and the branch source scanner regressions in future conflict resolution
Tested: Pre-merge synthetic merge-tree completed without conflicts
Not-tested: Post-merge focused and full pinned gates pending
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