Skip to content

ENG-3443: Fix selectUser selector memoization#7927

Merged
gilluminate merged 3 commits intomainfrom
gill/ENG-3443/fix-select-user-memoization
Apr 15, 2026
Merged

ENG-3443: Fix selectUser selector memoization#7927
gilluminate merged 3 commits intomainfrom
gill/ENG-3443/fix-select-user-memoization

Conversation

@gilluminate
Copy link
Copy Markdown
Contributor

@gilluminate gilluminate commented Apr 14, 2026

Ticket ENG-3443

Description Of Changes

The selectUser Redux selector in auth.slice.ts was a plain function that created a new object ({ ...user, isRootUser }) on every call. Since the returned object was referentially different each time, Redux flagged it as an unstable selector, logging a console warning and causing unnecessary rerenders in every component that consumes it (ProtectedRoute, AccountDropdownMenu, ManualTasks, MonitorList, etc.).

This PR wraps selectUser with createSelector from @reduxjs/toolkit so the result is memoized and only recomputes when state.auth changes.

Code Changes

  • Memoized selectUser selector using createSelector with selectAuth as its input selector
  • Added createSelector to the @reduxjs/toolkit import

Steps to Confirm

  1. Run the admin UI dev server (npm run dev in clients/admin-ui/)
  2. Open the browser console
  3. Confirm the "Selector selectUser returned a different result when called with the same parameters" warning no longer appears on page load

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • No UX review needed
  • Followup issues:
    • No followup issues
  • Database migrations:
    • No migrations
  • Documentation:
    • No documentation updates required

gilluminate and others added 2 commits April 14, 2026 16:35
Wrap selectUser with createSelector so it returns a stable reference
when state.auth hasn't changed, eliminating the Redux console warning
about unstable selector results.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Apr 14, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Apr 14, 2026 11:14pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored Apr 14, 2026 11:14pm

Request Review

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 14, 2026

Title Lines Statements Branches Functions
admin-ui Coverage: 8%
6.1% (2663/43633) 5.22% (1293/24734) 4.19% (542/12916)
fides-js Coverage: 78%
78.98% (1962/2484) 65.55% (1214/1852) 72.57% (336/463)
privacy-center Coverage: 88%
85.97% (331/385) 81.36% (179/220) 78.87% (56/71)

Replace Form.useWatch([], form) + isFieldsTouched(true) with a
direct watch on the usernameConfirmation field value. The previous
approach had a vacuous-truth edge case where isFieldsTouched(true)
could return true when no fields were registered yet, which was
masked by unnecessary rerenders from the unmemoized selectUser.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gilluminate gilluminate marked this pull request as ready for review April 14, 2026 23:26
@gilluminate gilluminate requested a review from a team as a code owner April 14, 2026 23:26
@gilluminate gilluminate requested review from speaker-ender and removed request for a team April 14, 2026 23:26
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Code Review

Clean, well-scoped PR. Two targeted fixes with no production risk.

auth.slice.tsselectUser memoization

The root cause diagnosis is correct: spreading { ...user, isRootUser } in a plain selector always produces a new object reference, which RTK's selector stability check rightfully flags. Wrapping with createSelector is the right fix — the memoized result is returned unchanged as long as state.auth hasn't changed.

One minor note on the input selector granularity (see inline comment): using selectAuth means the selector recomputes on any auth change (including token), not just user changes. Not a functional issue given login/logout change both atomically, but worth knowing.

DeleteUserModal.tsxForm.useWatch targeting

Both changes are improvements:

  1. Watching only "usernameConfirmation" instead of all fields ([]) is more targeted.
  2. The disabled condition change from !form.isFieldsTouched(true)!usernameConfirmation is more semantically accurate for this UX (enable the button when text is present, not merely when the field has been touched).

One observation about form.getFieldsError() in the disabled expression (see inline comment) — benign for the current single-field form, but worth keeping in mind if the form grows.

Changelog

Changelog entry is present and accurate. ✅


🔬 Codegraph: unavailable

💡 Write /code-review in a comment to re-run this review.

Comment thread clients/admin-ui/src/features/auth/auth.slice.ts
Comment thread clients/admin-ui/src/features/user-management/DeleteUserModal.tsx
Comment thread clients/admin-ui/src/features/user-management/DeleteUserModal.tsx
Copy link
Copy Markdown
Contributor

@speaker-ender speaker-ender left a comment

Choose a reason for hiding this comment

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

LGTM!

@gilluminate gilluminate added this pull request to the merge queue Apr 15, 2026
Merged via the queue into main with commit e0f8b65 Apr 15, 2026
52 checks passed
@gilluminate gilluminate deleted the gill/ENG-3443/fix-select-user-memoization branch April 15, 2026 15:22
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.

2 participants