Optimize client settings selector rerenders#3745
Draft
cursor[bot] wants to merge 3 commits into
Draft
Conversation
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
| return typeof value === "object" && value !== null; | ||
| } | ||
|
|
||
| export function areSettingsSelectionsEqual<T>(left: T, right: T): boolean { |
Contributor
There was a problem hiding this comment.
🟡 Medium hooks/useSettings.ts:51
areSettingsSelectionsEqual uses Object.keys to compare non-primitive selections, so Set, Map, Date, and class instances with no enumerable own keys all compare equal to each other. When a selector returns one of these (e.g. useClientSettings((s) => new Set(s.favorites.map(...)))), createSettingsSelectorSnapshotReader treats every new selection as equal to the stale cached one and keeps returning the previous value, so the component stops updating after the first render. Consider falling back to Object.is for non-plain objects so distinct object identities still produce updates.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/hooks/useSettings.ts around line 51:
`areSettingsSelectionsEqual` uses `Object.keys` to compare non-primitive selections, so `Set`, `Map`, `Date`, and class instances with no enumerable own keys all compare equal to each other. When a selector returns one of these (e.g. `useClientSettings((s) => new Set(s.favorites.map(...)))`), `createSettingsSelectorSnapshotReader` treats every new selection as equal to the stale cached one and keeps returning the previous value, so the component stops updating after the first render. Consider falling back to `Object.is` for non-plain objects so distinct object identities still produce updates.
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.
What Changed
useClientSettings/ merged settings hooks so consumers keep the previous selected reference when their selected value is shallow-equal.Why
A manual React performance pass found that settings selector consumers subscribed to the entire client settings object. Updating an unrelated client setting, such as
favorites, caused selected consumers likeProjectGroupingConsumerandThreadSortConsumerto rerender even though their selected values were unchanged.react-doctor was run with
corepack pnpm dlx react-doctor@latest . --verbose --category performance --no-telemetry; it reported broader performance findings, while this PR keeps one focused fix for the settings selector fanout found during the manual pass.UI Changes
Before/after React Scan recordings were captured with a temporary Vite fixture using the real settings hooks, then the fixture was removed before commit:
before_settings_selector_rerenders.mp4— five unrelated favorites updates increased both selected consumer counters from 2 to 12, with React Scan purple render highlights on the consumer panels.after_settings_selector_rerenders.mp4— the same five updates left both counters stable at 2, with no React Scan highlights on the consumer panels.Checklist
Validation:
corepack pnpm --filter @t3tools/web test -- src/hooks/useSettings.test.tspassed (web unit project: 148 files / 1280 tests)NODE_OPTIONS="--experimental-strip-types" corepack pnpm exec vp checkpassed with existing warningscorepack pnpm exec vp run typecheckpassedNote
Reduce rerenders in client settings hooks by stabilizing selector references
areSettingsSelectionsEqual, a shallow equality comparator, andcreateSettingsSelectorSnapshotReader, a factory that returns the previous selection reference when the selected slice is shallowly equal across updates.useSelectedClientSettings, a new hook that composesuseSyncExternalStorewith the snapshot reader to avoid returning new references when unrelated settings change.useClientSettingsanduseMergedSettingsto delegate touseSelectedClientSettings, so components only re-render when their selected slice actually changes.useClientSettingsanduseMergedSettingsnow use shallow equality instead of referential equality to determine whether to propagate a new value, which may suppress re-renders that previously occurred.📊 Macroscope summarized c19a486. 1 file reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.