perf(TreeView): Cache tree items in typeahead for better INP #7334
Conversation
- Add useTreeItemCache hook to cache DOM queries for tree items - Update useRovingTabIndex and useTypeahead to use cached items - Add documentation for acceptable :has() selector usage Part of #7312
🦋 Changeset detectedLatest commit: c7887ff The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
👋 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 |
github-actions
bot
added
the
integration-tests: recommended
There was a problem hiding this comment.
Pull request overview
This PR optimizes TreeView typeahead performance by caching tree item DOM queries to reduce expensive querySelectorAll calls during user input. The changes eliminate redundant DOM traversal on every keystroke, targeting 85-95% reduction in input latency for large trees.
Key Changes:
- Introduced
useTreeItemCachehook with MutationObserver-based cache invalidation - Replaced direct DOM queries with cached lookups in
useTypeahead - Added CSS documentation explaining
:has()selector usage
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
useTreeItemCache.ts |
New hook providing cached tree item queries with automatic invalidation on structural changes |
useTypeahead.ts |
Replaced querySelectorAll with cached lookup from useTreeItemCache |
useRovingTabIndex.ts |
Added null check for tree root element |
TreeView.module.css |
Added comment documenting acceptable descendant :has() selector usage |
perf-treeview-typeahead-cache.md |
Changeset documenting the performance improvements |
| const items = Array.from(root?.querySelectorAll('[role=treeitem]') || []) | ||
| if (!root) return | ||
|
|
||
| const items = Array.from(root.querySelectorAll('[role=treeitem]')) |
There was a problem hiding this comment.
This function still uses direct DOM queries instead of the new useTreeItemCache hook. For consistency with the performance improvements in useTypeahead, consider using the cached tree items here as well to avoid redundant queries.
There was a problem hiding this comment.
we aren't doing this because the cost isn't worth it for these interactions, which need to do this anyway - but we might revisit that if we need to.
Tests cover: - Basic tree item querying - Null container handling - Empty tree caching (null vs empty array) - Cache invalidation on structural changes - Cache invalidation on role attribute changes - aria-expanded changes do NOT invalidate cache - Nested tree item handling
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
@mattcosta7 Looks like new tests were adding since you branched out of main. Backmerging main so that they show up in CI on the PR (not just in merge queue) |
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/9163 |
Summary
Performance optimizations for TreeView typeahead functionality to improve INP.
Changes
Expected INP Impact
Why this matters
TreeView typeahead was calling
querySelectorAllon every keystroke to find matching items. For large trees, this is extremely expensive. By caching the tree items and only invalidating when structure changes, we:Part of the INP performance optimization effort. See #7312 for full context.