-
Notifications
You must be signed in to change notification settings - Fork 649
Add useDeferredValue to FilteredActionList for improved typing responsiveness with large lists #7411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 469db2d 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 |
- Import `useDeferredValue` from React - Create `deferredItems` using `useDeferredValue(items)` - Keep input immediately responsive with immediate `filterValue` - Use `deferredItems` for rendering the list to avoid blocking keystrokes - Maintain immediate `items` for user interactions like Enter key and select all - All tests passing Co-authored-by: mattcosta7 <[email protected]>
Co-authored-by: mattcosta7 <[email protected]>
|
👋 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 |
|
@copilot add a changeset |
Co-authored-by: mattcosta7 <[email protected]>
…erformance (#7414) Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: mattcosta7 <[email protected]>
|
@copilot update the pr description to describe expected benefits of this approach and impact on performance/ web vitals |
Updated the PR description with detailed sections on: Expected Benefits:
Web Vitals Impact:
User Experience: Better handling of 100-1000+ item lists, improved performance on low-end devices, benefits for fast typists and screen reader users. Also added technical details explaining how |
There was a problem hiding this comment.
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 implements a performance optimization for FilteredActionList by using React's useDeferredValue hook to improve typing responsiveness when filtering large lists. The implementation defers expensive list re-rendering operations while keeping the text input immediately responsive.
Key Changes
- Added
useDeferredValueto defer theitemsprop for rendering purposes - Extracted list rendering logic into a memoized
FilteredActionListItemscomponent - Refactored
getItemListForEachGroupto accept items as a parameter, enabling usage with both immediate and deferred items - Maintained immediate items for user interactions (keyboard navigation, select-all) while using deferred items for rendering
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
packages/react/src/FilteredActionList/FilteredActionList.tsx |
Core implementation: added useDeferredValue hook, extracted memoized FilteredActionListItems component, refactored getItemListForEachGroup to support both immediate and deferred items |
.changeset/perf-filtered-action-list-deferred.md |
Changeset documenting the performance improvement as a patch release |
| )} | ||
| {/* @ts-expect-error div needs a non nullable ref */} | ||
| <div ref={scrollContainerRef} className={classes.Container}> | ||
| {/* eslint-disable-next-line react-hooks/refs -- getBodyContent accesses scrollContainerRef.current for conditional loading indicator rendering, which is safe in this context */} |
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This eslint-disable comment appears to be unnecessary and potentially misleading. The comment suggests that getBodyContent accesses scrollContainerRef.current, which it does, but this is not a violation of React hooks rules. The function is called during render (line 546), and the conditional ref access inside the function (line 453) is a standard React pattern. If there's no actual eslint error here, this comment should be removed.
| {/* eslint-disable-next-line react-hooks/refs -- getBodyContent accesses scrollContainerRef.current for conditional loading indicator rendering, which is safe in this context */} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot was this disable necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The eslint-disable comment is actually necessary. The linter correctly detects that scrollContainerRef.current is accessed during render (in getBodyContent() at line 453), which is a ref access violation. However, this is intentional for the loading indicator height calculation. I've improved the comment to better explain why this ref access is safe in this context (commit 9388403).
|
@copilot ensure that any components inheriting performance benefits from this are mentioned in the changeset |
- Improved eslint-disable comment to better explain why the ref access is intentional - Updated changeset to explicitly mention that SelectPanel inherits performance improvements Co-authored-by: mattcosta7 <[email protected]>
Updated the changeset to explicitly mention that SelectPanel inherits these performance improvements (commit 9388403). SelectPanel is the primary component that uses FilteredActionList internally, so it automatically benefits from the deferred rendering optimization. |
|
@mattcosta7 How confident do you feel about these changes? It's hard to review because it's a bit theoretical? I'd love to see some numbers from dotcom or maybe add a stress testing story to this repo's storybook |
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/9923 |
🔬 github-ui Integration Test Results
❌ Troubleshooting Failed ChecksCIThis check runs linting, type checking, and unit tests. Check the workflow logs for specific failures. Need help? If you believe this failure is unrelated to your changes, please reach out to the Primer team for assistance. |
going to close this for now - I missed something related in how we pass filtered items here, and I don't think this is correct. Should have kept a draft longer |
FilteredActionListcan experience input lag when filtering large lists because list re-rendering blocks the input on every keystroke. This PR implements React'suseDeferredValuehook to defer expensive list re-renders while keeping the input immediately responsive, following the same pattern already established in theAutocompletecomponent.Implementation
Core changes:
Wrap items with
useDeferredValue:Extract list rendering to memoized component:
Use deferred items for rendering, immediate items for interactions:
deferredItems(deferrable, expensive operation)filterValuefor instant visual feedbackonFilterChangecallback remains synchronous (maintains API contract)items(accuracy over deferral)Refactor:
getItemListForEachGroupnow accepts items as a parameter to support both immediate and deferred lists.Expected Benefits
Performance Improvements
Reduced Input Lag: By deferring list re-renders, the text input remains responsive even when filtering large datasets (100+ items). Users see their keystrokes appear instantly without delay.
Non-Blocking Rendering: React's scheduler can interrupt deferred list rendering to handle urgent updates (user input), preventing the UI from freezing during expensive operations.
Optimized Re-render Cycles: Using
React.memowithuseDeferredValueallows React to skip re-rendering the list component when the deferred value hasn't caught up yet, reducing unnecessary work.Smooth Typing Experience: During rapid typing, React batches deferred updates, rendering the list only after the user pauses rather than on every keystroke.
Web Vitals Impact
Interaction to Next Paint (INP) - Primary improvement target:
Cumulative Layout Shift (CLS) - Neutral impact:
First Input Delay (FID) / INP - Indirect benefit:
User Experience Benefits
Components Benefiting from This Change
Technical Details
How
useDeferredValueWorksReact's
useDeferredValuecreates a "stale" version of a value that updates with lower priority:itemsupdates immediately (urgent)deferredItemsis marked for update but with low priority (deferrable)filterValuefirst (high priority)deferredItemsand re-renders the list (low priority)deferredItemsupdates, only rendering the final statePattern Consistency
This follows the same approach as
Autocomplete:AutocompleteusesuseDeferredValue(inputValue)for internal filtering inAutocompleteMenuFilteredActionListusesuseDeferredValue(items)for list rendering, since filtering happens externally in the parentScreenshot
The component displays and filters correctly with the deferred rendering optimization in place.
Changelog
Changed
FilteredActionListnow usesuseDeferredValuefor items rendering to improve typing responsiveness with large listsRollout strategy
Internal optimization with no API changes or breaking behavior.
Testing & Reviewing
All existing tests pass (2255 tests) without modification. Deferred rendering doesn't affect the synchronous contract between parent and child. Manually verified in Storybook with various list sizes.
Merge checklist
Original prompt
This pull request was created from Copilot chat.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.