Skip to content

perf(hasInteractiveNodes): Optimize with combined selector and early attribute checks - #7342

Merged
mattcosta7 merged 13 commits into
mainfrom
perf/hasinteractivenodes-cleanup
Dec 27, 2025
Merged

perf(hasInteractiveNodes): Optimize with combined selector and early attribute checks#7342
mattcosta7 merged 13 commits into
mainfrom
perf/hasinteractivenodes-cleanup

Conversation

@mattcosta7

@mattcosta7mattcosta7 commented Dec 15, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Performance optimizations for hasInteractiveNodes utility function to improve INP.

Changes

  1. Combined selector - Use querySelectorAll with combined selector instead of recursive traversal
  2. Early attribute checks - Check disabled, hidden, inert attributes before expensive getComputedStyle
  3. Deferred getComputedStyle - Only call getComputedStyle when CSS-based visibility check is absolutely needed

Expected INP Impact

ScenarioBeforeAfterImprovement
Worst case (large DOM subtree, 500+ nodes)~50-100ms (recursive traversal + getComputedStyle per node)<10ms80-90% reduction
Average case (medium subtree, 50-200 nodes)~15-40ms<5ms67-88% reduction
Best case (small subtree, <50 nodes)~5-15ms<2ms60-87% reduction

Why this matters

hasInteractiveNodes is called by:

  • AvatarStack - To detect if avatars are clickable
  • ActionList - To detect interactive items
  • Various other components that need to detect focusable children

Previous implementation problems:

  1. Recursive traversal - Visited every node in subtree individually
  2. getComputedStyle per node - Called expensive style calculation for every single node
  3. No early termination optimization

New implementation:

  1. Single querySelectorAll - Browser-optimized selector matching
  2. Attribute-first checks - Fast O(1) attribute lookup before style calc
  3. Early return - Stops at first interactive node found

Part of the INP performance optimization effort. See #7312 for full context.

@mattcosta7
perf(hasInteractiveNodes): Optimize with combined selector and early …
e203ffd
…attribute checks
- Use combined querySelectorAll selector instead of recursive traversal
- Check attribute-based states (disabled, hidden, inert) before getComputedStyle
- Only call getComputedStyle when CSS-based visibility check is needed
Part of #7312
@changeset-bot

changeset-botBot commented Dec 15, 2025•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 531fcbe

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
@primer/reactPatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actionsgithub-actionsBot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Dec 15, 2025
@github-actions

github-actionsBot commented Dec 15, 2025

Copy link
Copy Markdown
Contributor

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the integration-tests: skipped manually label to skip these checks.

@github-actions
github-actionsBottemporarily deployed to storybook-preview-7342 December 15, 2025 15:13 Inactive
@mattcosta7mattcosta7 self-assigned this Dec 15, 2025
CopilotAI reviewed Dec 15, 2025
View reviewed changes

CopilotAI 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.

Pull request overview

This PR optimizes the hasInteractiveNodes utility function to reduce INP (Interaction to Next Paint) by replacing recursive DOM traversal with a more efficient implementation using browser-native selector matching and deferring expensive style calculations.

Key changes:

  • Replace recursive findInteractiveChildNodes with single querySelectorAll call using combined selector
  • Reorder checks in isNonValidInteractiveNode to prioritize fast attribute lookups before expensive getComputedStyle calls
  • Remove unused findInteractiveChildNodes function

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
packages/react/src/internal/utils/hasInteractiveNodes.tsRefactored to use querySelectorAll with combined selector and optimized validation order
.changeset/perf-hasinteractivenodes-cleanup.mdAdded changeset documenting performance improvements

Comment threadpackages/react/src/internal/utils/hasInteractiveNodes.ts Outdated
@mattcosta7
Apply suggestion from @Copilot
0fb75e3
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7342 December 15, 2025 18:11 Inactive
@mattcosta7
mattcosta7 marked this pull request as ready for review December 15, 2025 19:36
@mattcosta7
mattcosta7 requested a review from a team as a code ownerDecember 15, 2025 19:36
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7342 December 15, 2025 19:45 Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7342 December 16, 2025 04:54 Inactive
@lesliecdubs
lesliecdubs requested review from francinelucca and removed request for joshblackDecember 16, 2025 23:28
francinelucca
francinelucca approved these changes Dec 17, 2025
View reviewed changes

@francineluccafrancinelucca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

✨

@francinelucca

francinelucca commented Dec 17, 2025

Copy link
Copy Markdown
Member

@copilot can you add tests for hasInteractiveNodes

CopilotAI mentioned this pull request Dec 17, 2025
Merged
13 tasks

CopilotAI commented Dec 17, 2025

Copy link
Copy Markdown
Contributor

@francinelucca I've opened a new pull request, #7350, to work on those changes. Once the pull request is ready, I'll request review from you.

CopilotAIand others added 2 commits December 17, 2025 12:13
@francinelucca
test: Add comprehensive test coverage for hasInteractiveNodes utility (…
80dcc10
…#7350)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: francinelucca <40550942+francinelucca@users.noreply.github.com>
@francinelucca
Merge branch 'main' into perf/hasinteractivenodes-cleanup
d107065
francinelucca
francinelucca approved these changes Dec 17, 2025
View reviewed changes

@francineluccafrancinelucca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

✨

@github-actions
github-actionsBottemporarily deployed to storybook-preview-7342 December 17, 2025 17:27 Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7342 December 18, 2025 15:05 Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7342 December 19, 2025 22:57 Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7342 December 23, 2025 17:53 Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7342 December 23, 2025 19:16 Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7342 December 23, 2025 20:02 Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7342 December 26, 2025 22:23 Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7342 December 26, 2025 22:34 Inactive
@primer-integration

primer-integrationBot commented Dec 26, 2025

Copy link
Copy Markdown

👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/9492

@primer-integration

primer-integrationBot commented Dec 27, 2025

Copy link
Copy Markdown

🔬 github-ui Integration Test Results

CheckStatusDetails
CI✅ PassedView run
Projects (Memex)✅ PassedView run
VRT✅ PassedView run

✨ All checks passed! Your integration PR is ready for review.

@mattcosta7
mattcosta7 added this pull request to the merge queueDec 27, 2025
Merged via the queue into main with commit a8b42b2Dec 27, 2025
52 checks passed
@mattcosta7
mattcosta7 deleted the perf/hasinteractivenodes-cleanup branch December 27, 2025 00:22
@primerprimerBot mentioned this pull request Dec 27, 2025
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@francineluccafrancineluccafrancinelucca approved these changes

Assignees

@mattcosta7mattcosta7

Labels

integration-tests: recommendedThis change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mattcosta7@francinelucca