From 0b9c528368ce9a56797d341ee212270858ac8c19 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 29 May 2026 02:48:28 -0700 Subject: [PATCH] Move notification settings ref sync out of effect (#3065) * Move notification settings ref sync out of effect * Record notification step effect cleanup audit --- docs/reference/react-performance-audit.md | 4 +++- .../src/components/onboarding/NotificationStep.tsx | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/reference/react-performance-audit.md b/docs/reference/react-performance-audit.md index 258b3593e01..61da27adaf5 100644 --- a/docs/reference/react-performance-audit.md +++ b/docs/reference/react-performance-audit.md @@ -47,7 +47,7 @@ Initial inventory: ## Coverage Ledger -Current count after low-risk PRs #3038, #3041, #3042, #3044, #3051, #3052, #3053, #3054, #3055, #3056, #3058, #3059, #3060, #3062, #3063, and #3064: 946 Effect hook call sites. +Current count after low-risk PRs #3038, #3041, #3042, #3044, #3051, #3052, #3053, #3054, #3055, #3056, #3058, #3059, #3060, #3062, #3063, #3064, and #3065: 945 Effect hook call sites. | Area | Files / signal | Scan status | Notes | | ------------------------------ | -------------------------------------------------------------------------------------------------------- | --------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------- | @@ -96,6 +96,7 @@ These are candidate batches, not final conclusions. Each item needs code inspect | PR V | Workspace board selection pruning | Local kanban selection is repaired in an Effect after the drawer closes or board rows change. | `use-workspace-kanban-selection.ts` covered by #3062 | Low | | PR W | Workspace board column resize | Two mirror Effects sync the latest commit callback and external committed width after render. | `use-workspace-kanban-column-resize.ts` covered by #3063 | Low | | PR X | Terminal quick-command dialog draft | Dialog draft and agent preset search are reset in an Effect after the dialog opens or retargets. | `TerminalQuickCommandDialog.tsx` covered by #3064 | Low | +| PR Y | Onboarding notification settings ref | A ref mirror Effect keeps notification handlers pointed at the latest settings. | `NotificationStep.tsx` covered by #3065 | Low | ## Merge Risk Scale @@ -125,6 +126,7 @@ These are candidate batches, not final conclusions. Each item needs code inspect | #3062 | `nwparker/react-perf-kanban-selection` | Workspace board selection pruning moves out of an Effect | Low | Merged | `pnpm exec oxlint src/renderer/src/components/sidebar/use-workspace-kanban-selection.ts`; `pnpm run typecheck:web`. | | #3063 | `nwparker/react-perf-kanban-column` | Workspace board column width mirror Effects move to render-time synchronization | Low | Merged | `pnpm exec oxlint src/renderer/src/components/sidebar/use-workspace-kanban-column-resize.ts`; `pnpm run typecheck:web`. | | #3064 | `nwparker/react-perf-quick-command-dialog` | Terminal quick-command dialog resets draft state during render | Low | Merged | `pnpm exec oxlint src/renderer/src/components/terminal-quick-commands/TerminalQuickCommandDialog.tsx`; `pnpm run typecheck:web`. | +| #3065 | `nwparker/react-perf-notification-step-ref` | Onboarding notification settings ref mirror moves out of an Effect | Low | Merged | `pnpm exec oxlint src/renderer/src/components/onboarding/NotificationStep.tsx`; `pnpm run typecheck:web`. | ## Reproduction Commands diff --git a/src/renderer/src/components/onboarding/NotificationStep.tsx b/src/renderer/src/components/onboarding/NotificationStep.tsx index 0866ee653da..fd640ead637 100644 --- a/src/renderer/src/components/onboarding/NotificationStep.tsx +++ b/src/renderer/src/components/onboarding/NotificationStep.tsx @@ -50,10 +50,14 @@ export function NotificationStep({ const [isPickingSound, setIsPickingSound] = useState(false) const [showMacSettingsPreview, setShowMacSettingsPreview] = useState(false) const [selectPortalRoot, setSelectPortalRoot] = useState(null) + const syncedNotificationSettingsRef = useRef(notificationSettings) - useEffect(() => { + if (syncedNotificationSettingsRef.current !== notificationSettings) { + syncedNotificationSettingsRef.current = notificationSettings + // Why: handlers optimistically update the ref before persisted settings + // flow back through props, so local re-renders must not overwrite it. notificationSettingsRef.current = notificationSettings - }, [notificationSettings]) + } useEffect(() => { // Why: onboarding sits above body-level portals, so the select menu must