diff --git a/plans/engine-listener.md b/plans/engine-listener.md new file mode 100644 index 000000000000..4bb015d4e2d6 --- /dev/null +++ b/plans/engine-listener.md @@ -0,0 +1,69 @@ +# Engine Listener Migration + +## Summary + +Add a typed engine action subscription layer so feature components can react directly to ephemeral engine events without inventing one-field Zustand stores or `init/shared.tsx` forwarding shims. + +This is for screen-local refreshes, prompts, and transient session reactions. It is not a replacement for durable shared caches or notification-backed global state. + +## Current Primitive + +- [x] Add `shared/engine/action-listener.tsx` with: + - `subscribeToEngineAction(type, listener)` + - `useEngineActionListener(type, listener)` + - `notifyEngineActionListeners(action)` +- [x] Dispatch those listeners from `shared/constants/init/shared.tsx` after existing global/store fanout +- [x] Convert `people` off the temporary refresh bridge and onto direct engine subscription + +## Migration Rules + +Use the engine listener layer when all of these are true: + +- The engine action is an ephemeral nudge or prompt for one feature +- The UI can safely miss the event while unmounted because it reloads on focus/mount or because the prompt only matters while visible +- The action does not need to mutate durable shared state for other screens + +Keep store-owned engine handling when any of these are true: + +- The action updates long-lived shared caches, badges, unread counts, or background state +- The app must retain the effect even while the screen is unmounted +- Several unrelated features need the same derived state + +## Candidate Audit Order + +- [x] `people` +- [ ] `tracker` transient identify/session events + - Re-check `identify3ShowTracker` and `showTrackerSet` first + - Keep durable profile caches in the store unless a later refactor proves otherwise +- [ ] `pinentry` + - Evaluate whether the prompt ownership can move to a feature-local listener plus flow handles + - Keep the current store if the response/session lifecycle still needs a global owner +- [ ] `archive` + - Re-check whether archive progress/jobs truly need store persistence or can become screen-owned reload state +- [ ] `unlock-folders` + - Compare existing direct engine handling against the new listener layer and decide whether unification adds value + +## Explicit Non-Targets For Now + +- `config` +- `notifications` +- `chat` +- `teams` +- `fs` +- `users` + +These still own durable or broadly shared state and should not be converted mechanically. + +## Rollout Notes + +- Prefer `navigation` focus/blur for screen lifecycle like “mark viewed” or draft cleanup +- Prefer `useEngineActionListener` for mounted-screen reactions to engine pushes +- Do not move durable notification state into component listeners +- Delete temporary feature-local bridges such as increment-only refresh modules once direct subscriptions land + +## Validation + +- Confirm the feature still reacts while mounted +- Confirm focus/mount reload still covers missed events while unmounted +- Confirm no global store or init forwarding remains for the migrated action +- Update or delete tests that previously only exercised store plumbing diff --git a/plans/inbox-load-fail.md b/plans/inbox-load-fail.md deleted file mode 100644 index e3d57be327fd..000000000000 --- a/plans/inbox-load-fail.md +++ /dev/null @@ -1,585 +0,0 @@ -# requestInboxUnbox Outstanding Session Investigation - -## Context - -Issue observed in Electron: - -- `outstandingSessionDebugger` repeatedly logged `chat.1.local.requestInboxUnbox` -- the logs appeared in the renderer -- the problem showed up after startup / after entering Chat - -The issue stopped reproducing after restarting the Go daemon. - -## Repro Flow Used - -1. Start the app clean. -2. Let startup settle. -3. Clear renderer and node logs. -4. Click into Chat. -5. Wait for `outstandingSessionDebugger`. -6. Pick the reported renderer `sessionID`. -7. Correlate that session to the transport `seqid` from renderer logs. - -Example stuck pairs we tracked: - -- `sessionID: 180` -> `seqid: 57` -- `sessionID: 181` -> `seqid: 58` -- `sessionID: 183` -> `seqid: 60` - -## What We Confirmed - -### 1. The outstanding sessions were real renderer engine sessions - -For a stuck case we saw: - -- renderer `session start {sessionID: ...}` -- renderer `invokeNow {seqid: ..., sessionID: ...}` -- no matching renderer `session end {sessionID: ...}` - -So this was not just noisy transport logging. The renderer engine session really remained open. - -### 2. The inner UI callback path was working - -For `chat.1.chatUi.chatInboxConversation` we saw: - -- incoming invoke in renderer -- renderer sending `response.result()` -- node receiving that response - -So the nested UI callback ack path was functioning during investigation. - -### 3. The generic JS request path was functioning - -For successful `requestInboxUnbox` calls we saw the full path: - -- renderer started engine session -- renderer invoked outer RPC with a `seqid` -- main received invoke -- node wrote invoke to daemon -- node received outer response from daemon -- renderer matched the response -- renderer ended the engine session - -So the JS RPC path was capable of closing these sessions normally. - -### 4. Some specific outer requests never produced a closing response during the bad runs - -For stuck cases like `181/58` and `183/60`, the important pattern was: - -- renderer started session -- renderer sent invoke -- main received invoke -- node wrote invoke to daemon -- no corresponding renderer session end - -During some bad runs, the decisive missing line for the stuck `seqid` was: - -- no `node received response from daemon ... seqid: ` - -That means the stuck session was not explained by renderer bookkeeping before send. - -### 5. The issue disappeared after restarting the Go daemon - -After restarting the daemon, the problem stopped reproducing. - -That materially weakens the hypothesis that the root cause is a deterministic bug in the new JS RPC implementation alone. - -## What We Tried On The JS Side - -These were investigated and then removed: - -- temp renderer/session logs in the engine layer -- temp transport logs in renderer and node -- temp bridge logs across Electron IPC -- temporary response normalization changes (`undefined` -> `null`) -- temporary bridge framing / chunk normalization experiments -- temporary `requestInboxUnbox` special-casing - -None of those produced a confirmed fix. - -The temporary instrumentation has been cleaned up. - -## Strongest Current Interpretation - -The best-supported explanation from the investigation is: - -- the renderer session stayed outstanding because the outer `requestInboxUnbox` RPC did not complete for some requests -- in bad runs, the request had already crossed renderer -> main -> daemon write -- the missing completion signal was at or after daemon handling, not before renderer send -- restarting the daemon made the issue disappear - -That makes a daemon-side or daemon-state issue the strongest next lead. - -## Service Log Follow-Up - -We checked: - -- `/Users/ChrisNojima/Library/Logs/keybase.service.log` - -Important caveat: - -- this log was later identified as coming from the wrong machine / wrong daemon instance for the bad repro -- so it should not be treated as evidence that the bad run itself completed normally -- it is still useful for understanding the structure of the Go path and the kinds of daemon-side interactions that can delay `RequestInboxUnbox` - -### What The Service Log Added - -The service log did **not** show a direct reproduction of: - -- `+ Server: RequestInboxUnbox` -- no matching `- Server: RequestInboxUnbox -> ok` - -In the ranges inspected, every `RequestInboxUnbox` entry returned. - -However, the service log did show a strong interaction between: - -- `Server: GetThreadNonblock(...)` -- inbox localizer suspension -- concurrent `RequestInboxUnbox` - -### Repeated Pattern In Service Logs - -In several runs, the sequence was: - -1. `GetThreadNonblock(...)` starts. -2. `SuspendComponent: canceled background task` appears. -3. `localizerPipeline: suspend` runs. -4. concurrent `RequestInboxUnbox` starts. -5. its job reaches: - - `localizerPipeline: localizeJobPulled: waiting for resume` -6. only after `GetThreadNonblock(...) -> ok` does the unbox localizer resume and the outer `RequestInboxUnbox` return. - -Representative examples: - -- around `2026-04-08 10:39:02 -04:00` -- around `2026-04-08 10:46:56 -04:00` -- around `2026-04-08 17:14:03 -04:00` - -### Concrete Example: Slow But Completing Unbox - -At `2026-04-08 10:39:02 -04:00`: - -- `GetThreadNonblock(000030...)` starts -- it suspends the localizer -- `RequestInboxUnbox` for the same conv starts -- the unbox job logs `waiting for resume` -- `GetThreadNonblock` returns after about `609ms` -- then `RequestInboxUnbox` returns after about `611ms` - -This is a daemon-side confirmation that `RequestInboxUnbox` can be blocked behind thread-load suspension. - -### Concrete Example: Cancellation / Re-enqueue Storm - -At `2026-04-08 10:46:56 -04:00`: - -- `GetThreadNonblock(000030...)` suspends the localizer -- multiple lines appear: - - `localizerPipeline: localizeJobPulled: canceled a live job` - - `localizerPipeline: localizeConversations: context is done, bailing` - - `localizerPipeline: localizeJobPulled: re-enqueuing canceled job` - - `localizerPipeline: localizeJobPulled: waiting for resume` -- a concurrent `RequestInboxUnbox` for the same conv also enters `waiting for resume` -- it eventually returns, but only after resume - -This did not wedge in the captured log, but it is the clearest daemon-side evidence of a fragile path. - -### Updated Go-Side Interpretation - -The best current Go-side explanation is now narrower: - -- `RequestInboxUnbox` is not independently blocked on UI callback ack in the daemon -- it is blocked on inbox localizer progress -- `GetThreadNonblock` can suspend and cancel localizer work -- concurrent thread loading and inbox unboxing share the same localizer machinery -- a daemon-side wedge in suspend/resume or cancel/re-enqueue handling remains a strong candidate for the runs where the outer response never came back - -### Specific Go Areas Now Most Suspicious - -1. `GetThreadNonblock` suspending the inbox source/localizer during thread load -2. `localizerPipeline.suspend/resume` -3. `localizerPipeline.localizeJobPulled` -4. cancel/re-enqueue behavior after `context canceled` -5. whether a resumed or retried job can be left waiting forever if resume signaling or pending/completed bookkeeping goes wrong - -## Correct-Machine Service Logs - -We later checked the actual machine logs: - -- `/Users/ChrisNojima/Downloads/logs/keybase.service.log` -- `/Users/ChrisNojima/Downloads/logs/keybase.service.log-20260408T180719-0400-20260408T181728-0400` - -These logs materially strengthen the same Go-side suspicion. - -### What These Logs Confirm - -They show that on the real machine: - -- `GetThreadNonblock(...)` does suspend the inbox source/localizer -- concurrent inbox-unbox jobs do enter `localizerPipeline: localizeJobPulled: waiting for resume` -- live localizer jobs can be canceled during that suspension -- canceled jobs can be re-enqueued and later resumed - -So the previously suspected thread-load/localizer interaction is not just theoretical. - -### Concrete Example: Thread Load Cancels Live Localizer Work - -Around `2026-04-08 17:34:59 -04:00` in the rotated log: - -1. `Server: GetThreadNonblock(...)` starts. -2. `SuspendComponent: canceled background task` appears. -3. `localizerPipeline: suspend` runs. -4. live localizer workers log: - - `localizerPipeline: localizeJobPulled: canceled a live job` - - `RemoteClient: chat.1.remote.getMessagesRemote -> ERROR: context canceled` - - `localizerPipeline: failed to transform message ... context canceled` -5. a concurrent inbox-unbox trace enters: - - `localizerPipeline: localizeJobPulled: waiting for resume` -6. only after `GetThreadNonblock(...) -> ok [time=205.738625ms]` do we see: - - `localizerPipeline: resume` - - `localizerPipeline: localizeJobPulled: resume, proceeding` - -That is direct daemon evidence that inbox unbox work can be paused behind thread loading and only released on resume. - -### Concrete Example: Slow RequestInboxUnbox Without Obvious Error - -Around `2026-04-08 18:21:46 -04:00` in the current log: - -- two `RequestInboxUnbox` traces (`tu-3DgB67sGd` and `WTZ2nhFPsZAr`) both enter: - - `localizerPipeline: localizeJobPulled: waiting for resume` -- both then resume and spend roughly `736ms` in `localizeConversations` -- both outer RPCs finally return in about `737ms` - -This shows two important things: - -- the outer RPC really is waiting on localizer progress -- the visible stall can be a combination of suspend/resume delay plus expensive per-conversation localization work - -### What We Have Not Yet Seen - -In the sampled correct-machine log windows, we have not yet isolated: - -- a `RequestInboxUnbox` that entered and never produced `-> ok` -- a `resume: spurious resume call without suspend` - -So the logs do not yet prove the exact permanent wedge. They do prove the fragile path that could plausibly cause it. - -## Narrowed Go Hypothesis - -Given the code and the correct-machine logs, the strongest daemon-side wedge theory is now: - -1. `RequestInboxUnbox` calls `UIInboxLoader.UpdateConvs` -2. `UpdateConvs` calls `LoadNonblock` -3. `LoadNonblock` does not return until the localizer callback channel closes -4. localizer jobs marked cancelable can block in `localizeJobPulled: waiting for resume` -5. `resume()` only releases those waiters when `suspendCount` drops all the way back to zero - -That means an unmatched or leaked suspend would strand every future waiting localizer job indefinitely. - -If that happens: - -- the localizer callback channel never finishes -- `LoadNonblock` never returns -- `RequestInboxUnbox` never returns an outer response -- because `RequestInboxUnbox` swallows normal `UpdateConvs` errors, the renderer would just observe an outstanding request rather than a useful failure - -### Most Specific Code-Level Candidate Now - -The cleanest permanent-hang mechanism is: - -- `GetThreadNonblock` enters `defer h.suspendInboxSource(ctx)()` -- the inbox localizer `suspendCount` is incremented -- cancelable localizer jobs queue up in `waiting for resume` -- for some path or nesting combination, the matching `resume()` does not drive `suspendCount` back to zero -- the waiters are never closed -- the outstanding `RequestInboxUnbox` stays open until daemon restart resets that state - -This is now the main Go-side hypothesis to verify against the next bad-run log. - -## Code Audit Follow-Up - -After digging further into the Go code, the strongest explanation shifted slightly: - -- the most likely wedge is no longer just a leaked `suspendCount` -- a stuck `GetThreadNonblock` path can itself keep the inbox localizer suspended forever - -### Why GetThreadNonblock Is So Dangerous Here - -`GetThreadNonblock` does: - -- `defer h.suspendInboxSource(ctx)()` - -That means inbox-source resume does **not** happen when thread payloads are first sent to the UI. -It only happens when `UIThreadLoader.LoadNonblock(...)` fully returns. - -So if `LoadNonblock(...)` wedges anywhere, the inbox localizer remains suspended the whole time. - -### Important Detail: Thread Load Continues After UI Delivery - -`UIThreadLoader.LoadNonblock(...)` does more work after sending the full thread to the UI: - -1. waits for the local/full goroutines with `wg.Wait()` -2. logs `thread payloads transferred, checking for resolve` -3. resolves skipped unboxeds / validation work -4. performs final status clearing -5. only then returns - -This matters because: - -- the UI may already look "done" -- but the inbox source is still suspended -- so concurrent `RequestInboxUnbox` jobs can still sit in `waiting for resume` - -### Conversation Lock Coupling - -The thread loader also grabs a conversation lock for the full lifetime of `LoadNonblock(...)`: - -- `ConvSource.AcquireConversationLock(...)` -- defer release only when `LoadNonblock(...)` returns - -The same underlying lock is used by conversation-source operations that the inbox localizer calls, including: - -- `ConvSource.GetMessages(...)` -- `ConvSource.GetMessagesWithRemotes(...)` -- other pull/push/expunge paths - -So thread loading and inbox localization are contending on the same per-conversation lock. - -### Lock Behavior That Makes This Risky - -`ConversationLockTab.Acquire(...)` ultimately blocks on a plain `sync.Mutex`: - -- it does not select on `ctx.Done()` -- there is no timeout once it is waiting on the mutex itself - -So if one trace grabs the conversation lock and wedges before release: - -- later thread work for that conv can block forever -- later inbox-localizer work for that conv can block forever -- and if the wedged holder is `GetThreadNonblock`, inbox-source resume never fires - -That is a very strong match for: - -- issue appears after entering Chat -- some `requestInboxUnbox` requests never complete -- daemon restart clears the problem - -## Revised Leading Hypotheses - -From code inspection, the leading Go-side possibilities are now: - -1. `GetThreadNonblock` hangs before its deferred inbox-source resume runs -2. a conversation lock is held indefinitely by thread-loading or message-fetch code -3. a localizer worker hangs inside `localizeConversation(...)` after the initial inbox callback -4. a true suspend-count / waiter-state leak in the localizer pipeline - -### Strongest Current Candidate - -The strongest candidate is now: - -- `GetThreadNonblock` suspends the inbox source -- `UIThreadLoader.LoadNonblock(...)` acquires the conversation lock -- thread loading wedges somewhere under that lock or in its post-send validation phase -- deferred inbox-source resume never runs -- `RequestInboxUnbox` jobs remain stuck in `waiting for resume` - -This is a simpler explanation than a pure `suspendCount` imbalance and fits both the logs and the code structure better. - -## New Strong Code-Only Candidate: Blocking Thread-Status UI RPC - -There is an even more specific way `GetThreadNonblock` can wedge: - -- `UIThreadLoader.LoadNonblock(...)` uses `setUIStatus(...)` -- `setUIStatus(...)` eventually calls `chatUI.ChatThreadStatus(context.Background(), status)` -- `cancelStatusFn()` then waits for that goroutine to report whether the status was displayed - -This is important because: - -- `ChatThreadStatus(...)` is not fire-and-forget; it is a blocking RPC call through `ChatUiClient` -- it is invoked with `context.Background()`, not the request context -- if that UI RPC blocks, the goroutine never reports back on `resCh` -- `cancelStatusFn()` blocks forever waiting on `resCh` -- `UIThreadLoader.LoadNonblock(...)` then never returns -- `GetThreadNonblock` never reaches its deferred inbox-source resume -- `RequestInboxUnbox` jobs can remain stuck in `waiting for resume` - -### Why This Fits The User-Visible Symptom - -This is especially plausible because `LoadNonblock(...)` can already have sent the thread to the UI before the wedge happens. - -So the UI can look mostly usable while the daemon is still: - -- inside `GetThreadNonblock` -- holding the inbox source suspended -- blocking later inbox-unbox RPCs - -### Related Risk In Final Status Clearing - -At the end of `LoadNonblock(...)`, final status clearing also calls: - -- `chatUI.ChatThreadStatus(context.Background(), validated)` -- or `chatUI.ChatThreadStatus(context.Background(), none)` - -Those are also blocking UI RPCs with an uncancelable background context. - -So even after successful thread delivery and validation work, a wedged status callback could still keep `GetThreadNonblock` open indefinitely. - -### Relative Strength Of This Hypothesis - -This now looks at least as strong as the pure lock/suspend-count theories because: - -- it directly explains why entering Chat could trigger the stuck state -- it directly explains how the daemon can stay stuck even after visible UI progress -- it uses an explicit uncancelable blocking RPC call in the exact thread-load path that suspends inbox unboxing - -## Further Code Audit Narrowing - -After tracing the desktop/UI callback plumbing more carefully, the thread-status theory became narrower. - -### Important Weakening: JS Auto-Acks Thread Callbacks - -On the JS side, listener-backed incoming calls are acknowledged immediately: - -- the listener sends `response.result()` before it schedules the actual handler body -- `chatThreadStatus` in the desktop store is only a synchronous state update -- `chatThreadFull` / `chatThreadCached` are also listener callbacks on the same path - -So a slow or expensive JS status handler is **not** enough by itself to wedge the Go call. - -For the status-RPC theory to be the root cause, the failure has to be lower level: - -- the service-side RPC transport never gets the callback delivered to the frontend session -- or the frontend session is not actually able to dispatch the callback at all - -That keeps this as a real possibility, but weaker than it first looked from Go alone. - -### Another Candidate We Can Mostly De-Prioritize: waitForOnline - -`UIThreadLoader.waitForOnline(...)` only waits about one second total: - -- it loops 40 times -- each loop waits `25ms` -- then it proceeds anyway - -So `GetThreadNonblock` is not likely to wedge forever just because the loader was waiting to come online. - -### Stronger Code-Only Candidate: Post-Send Validation Before Resume - -There is a more convincing path inside `UIThreadLoader.LoadNonblock(...)`: - -1. the full thread is sent to the UI -2. `wg.Wait()` completes -3. the loader enters `thread payloads transferred, checking for resolve` -4. it resolves skipped unboxeds / validation work -5. only after that does `LoadNonblock(...)` return -6. only then does `GetThreadNonblock` run its deferred inbox-source resume - -This is important because it matches a user-visible state where: - -- the thread already appears loaded -- but the inbox localizer is still suspended -- and concurrent `RequestInboxUnbox` calls are still stuck in `waiting for resume` - -### Why The Post-Send Work Is Risky - -That post-send path does: - -- `NewBoxer(...).ResolveSkippedUnboxeds(...)` -- `ConvSource.TransformSupersedes(...)` -- notifier/update work on the resulting messages - -`ResolveSkippedUnboxeds(...)` re-validates sender keys for quick-unboxed messages through: - -- `ResolveSkippedUnboxed(...)` -- `ValidSenderKey(...)` -- `CtxUPAKFinder(ctx, ...).CheckKIDForUID(...)` - -So even after the thread is already visible in the UI, the loader can still be blocked doing sender-key validation and related follow-up work before resume happens. - -### Conversation Lock Scope Still Matters - -This is all still happening while `UIThreadLoader.LoadNonblock(...)` holds the per-conversation lock for its full lifetime. - -So the critical section is not just: - -- pull local thread -- pull remote thread - -It also includes: - -- JSON presentation/send -- post-send skip-resolution -- final status-clearing path - -That makes the thread-loader critical section broader than it first appears. - -### Additional Thread-Loader Bug - -There is also a separate bug in `UIThreadLoader.singleFlightConv(...)`: - -- `activeConvLoads[convID] = cancel` is written -- but entries are never removed - -This is probably not the root cause of the outstanding `requestInboxUnbox` sessions, but it is real thread-loader state leakage and could make cancellation behavior harder to reason about over time. - -## Revised Ranking After More Code Reading - -From code alone, the current ranking is: - -1. `GetThreadNonblock` wedges in post-send work before deferred inbox-source resume -2. `GetThreadNonblock` wedges somewhere else while holding the conversation lock -3. UI callback transport wedges on `ChatThreadStatus` / `ChatThreadFull` / `ChatThreadCached` -4. localizer worker hangs after resume inside `localizeConversation(...)` -5. pure localizer suspend-count imbalance / waiter leak - -## Additional Code Smells Worth Remembering - -These are not yet proven root cause, but they increase risk: - -- `UIInboxLoader.LoadNonblock(...)` has a 1-minute timeout only for the first unverified inbox result; after that, draining `localizeCb` has no timeout. -- several localizer username lookups use `UIDMap.MapUIDsToUsernamePackages(..., networkTimeBudget=0, ...)`, which means no explicit timeout is applied at that layer. -- `UIDMap.MapUIDsToUsernamePackages(...)` holds the UID-map mutex across the server lookup path, so one slow miss can serialize later username lookups behind it. -- `UIThreadLoader.LoadNonblock(...)` appears to check `err != nil` instead of `fullErr != nil` after `ChatThreadFull(...)`, which is likely a bug, though not the main outstanding-session explanation. - -## Things That Were Noise / Not Root Cause - -- "Inbox asked for too much work" was not the root issue for the stuck sessions. -- The fact that unboxes happen on startup is expected and not itself the bug. -- `chatInboxConversation` lacking a useful `sessionID` in logs did not explain the stuck outer requests by itself. -- The generic Electron bridge was not universally broken; many requests completed normally through it. - -## Useful Facts For The Next Debug Session - -If the issue reproduces again, capture: - -1. renderer `sessionID` -2. matching renderer outer `seqid` -3. whether node logged: - - `main received invoke ... seqid` - - `node wrote invoke to daemon ... seqid` - - `node received response from daemon ... seqid` -4. whether renderer later logs: - - `response matched invocation ... seqid` - - `session end ... sessionID` - -The most valuable bad-run pattern is: - -- renderer session start -- renderer invoke -- node wrote invoke to daemon -- no daemon response for that same outer `seqid` -- no renderer session end - -## Next Go-Side Questions - -1. Why would `chat.1.local.requestInboxUnbox` sometimes not produce an outer response for a subset of requests? -2. Is there any daemon-side state that can wedge this path until daemon restart? -3. Is `RequestInboxUnbox` blocked on loader/UI state in a way that can fail to return? -4. Is there any batching / callback / channel drain behavior in the inbox loader that can prevent the outer RPC from finishing? -5. Are there daemon logs around the stuck outer request showing the handler entered but never returned? - -## Current Status - -- no confirmed root-cause fix -- JS-side debug instrumentation removed -- daemon restart stopped reproduction -- next investigation should focus on Go/daemon behavior for stuck outer `requestInboxUnbox` calls diff --git a/plans/store-split.md b/plans/store-split.md new file mode 100644 index 000000000000..701c6038af8f --- /dev/null +++ b/plans/store-split.md @@ -0,0 +1,237 @@ +# Zustand Store Cleanup Series + +## Summary + +Optimize for store reduction, not store proliferation. Avoid splitting `modal-header` into several tiny stores. If a `modal-header` field can move to route params or feature-local screen state, do that; otherwise leave it alone for now. + +For this repo, assume most RPCs hit a local service and are cheap. Default toward reloading in the owning component instead of keeping a convenience cache unless the data truly needs to survive navigation or serve unrelated entry points. + +Cross-cutting rule: when an engine action is only a screen-local refresh or prompt nudge, prefer the typed engine listener layer over a one-field store or an `init/shared.tsx` forwarding shim. + +Recommended implementation order: + +- [x] `settings-email` +- [x] `settings-phone` +- [x] `people` +- [ ] `recover-password` +- [ ] `settings-password` +- [ ] `tracker` +- [ ] `team-building` +- [ ] `modal-header` only for param/local-state extraction, not store splitting + +## Key Changes + +### 1. Shrink `settings-email` to shared contact data only + +Current shape: + +- `emails` is a shared cache used by account settings and notification settings. +- `addedEmail` is transient account-settings banner state. +- `editEmail` mixes shared mutations with one-shot UI behavior. + +Planned change: + +- Keep `emails` in the store for now. +- Delete `addedEmail`, `setAddedEmail`, and `resetAddedEmail`. +- Move the “verification email sent” banner trigger into the account-settings flow using route params or local screen state. +- Move verify success UI handling out of `dispatch.editEmail`; keep only the mutation if it still has multiple callers. +- Preserve notification-fed `emails` updates and verified-status updates. + +Public/API impact: + +- Remove store APIs related to `addedEmail`. +- Update account settings and add-email flow to use explicit success context instead of hidden global banner state. + +### 2. Shrink `settings-phone` to shared phone data only + +Current shape: + +- `phones` is a shared cache used by account settings and startup routing. +- `addedPhone` is transient success-banner state. +- `editPhone` is a thin RPC wrapper. + +Planned change: + +- Keep `phones` in the store for now. +- Delete `addedPhone`, `setAddedPhone`, and `clearAddedPhone`. +- Move the “phone added” success banner to route params or local screen-owned state in the add/verify flow. +- Move delete/searchability RPCs out of the store if they remain account-settings-only operations. +- Keep pure helpers like `makePhoneError` where they are if still shared. + +Public/API impact: + +- Remove store APIs related to `addedPhone`. +- Potentially remove `dispatch.editPhone` if all remaining callers can use local hooks/RPCs directly. + +### 3. Delete `people` store or reduce it to direct feature wiring + +Current shape: + +- `refreshCount` exists only to trigger reloads in `people/container.tsx`. +- `markViewed` is a feature RPC, not durable global state. + +Planned change: + +- Remove `refreshCount` from Zustand. +- Subscribe the People feature directly to `homeUIRefresh` via the typed engine listener layer. +- Move `markViewed` into the People feature layer. +- Delete `shared/stores/people.tsx` if nothing durable remains. + +Public/API impact: + +- Remove init wiring that only forwards `homeUIRefresh` into the People store. +- Update People feature code to own its own refresh/mark-viewed behavior. + +Cross-cutting impact: + +- Add a typed engine action listener primitive so later store-pruning passes can move ephemeral engine nudges straight into feature code. + +### 4. Delete `recover-password` store by moving callbacks to `flow-handles` + +Current shape: + +- The store mostly holds runtime callbacks in `dispatch.dynamic`. +- `resetEmailSent` is screen state. +- `startRecoverPassword` is orchestration plus navigation. + +Planned change: + +- Move live callbacks into `flow-handles` using typed wrappers for: + - cancel + - submit device select + - submit no device + - submit paper key + - submit password + - submit reset password +- Move `resetEmailSent` into route params or local screen state. +- Move orchestration into a recover-password feature module, not a store. +- Update screens to read explicit params/handlers rather than selecting `dispatch.dynamic.*`. + +Public/API impact: + +- Delete `shared/stores/recover-password.tsx` if no durable state remains. +- Add recover-password-local typed helpers around `flow-handles`. + +### 5. Leave `modal-header` monolithic for now, but extract route-owned state + +Current shape: + +- `modal-header` mixes several unrelated concerns, but it should not be replaced with a set of tiny stores. + +Planned change: + +- Do not split `modal-header` into new stores. +- Only pull fields out when they are clearly route-owned or screen-owned: + - move transient titles or one-shot payloads to route params where the destination screen already owns them + - move screen-local flags like edit-avatar “has image” into the owning screen if the header only reflects local form state + - move per-screen action handlers to local route/screen code only when they do not need shared cross-screen coordination +- Leave device badges, FS filter, and other shared header state in `modal-header` unless there is a clean route/local owner. + +Public/API impact: + +- Reduce `useModalHeaderState.setState(...)` writes opportunistically. +- Do not introduce new tiny stores as replacements. + +### 6. Re-evaluate `settings-password` as a merge candidate only if there is a real home + +Current shape: + +- Only stores `randomPW`. +- Read by several settings screens and updated by notifications. + +Planned change: + +- Do not localize this to one screen. +- Merge it only if there is an obviously correct account/session store with the same lifecycle. +- Otherwise leave it as an intentionally retained tiny notification-backed cache. + +Public/API impact: + +- None unless a clear merge target emerges. + +### 7. Split `tracker` by behavior only where it reduces real store coupling + +Current shape: + +- Mixes durable profile caches with transient tracker UI/session behavior and proof suggestions. + +Planned change: + +- Keep durable caches (`usernameToDetails`, `usernameToNonUserDetails`) together. +- Pull transient tracker-session behavior out where feasible: + - `showTrackerSet` is the best candidate for a feature-local/session mechanism + - move `proofSuggestions` closer to profile/proofs UI if it is not broadly shared +- Keep follow/ignore/load orchestration with the cache until a later pass proves a narrower owner. + +Public/API impact: + +- Preserve existing read APIs until cache extraction is complete. +- Prefer thin feature helpers over exposing more transient state from the store. + +### 8. Prune only the duplicated or screen-owned parts of `team-building` + +Current shape: + +- `TBProvider` is already feature-scoped, which is good. +- Some fields appear duplicated with screen-local state. + +Planned change: + +- Keep the provider. +- Remove store fields that are already screen-owned or duplicated: + - `selectedService` is the clearest candidate, since `team-building/index.tsx` already owns it locally + - re-check `searchQuery` and `searchLimit` for the same pattern +- Keep multi-screen builder session state in the provider: + - `teamSoFar` + - `selectedRole` + - `sendNotification` + - shared session search results/recommendations if multiple subtrees still need them + +Public/API impact: + +- Preserve `TBProvider`/`useTBContext`. +- Remove only duplicate screen-owned state from the provider. + +## Test Plan + +For each commit: + +- Update or delete the matching store unit test. +- Add feature-level coverage where behavior moved out of stores. + +Critical scenarios: + +- Settings account: + - adding an email still shows the verification banner + - verifying/resending email still updates visible row state + - adding/verifying a phone still shows the success banner + - notification settings still shows the email section correctly +- People: + - People reloads after `homeUIRefresh` + - mark-viewed behavior still occurs on the intended path +- Recover password: + - device select, paper key, password, and reset prompt flows still survive navigation + - cancel paths still resolve the active RPC correctly + - runtime handlers still clear on reset/logout +- Modal header: + - any field moved out to params/local state still renders/clears correctly + - unchanged shared header behaviors still work as before +- Tracker: + - profile loads still populate details + - tracker open/close still works + - follow/ignore still update result state +- Team building: + - selection survives builder navigation + - search/recommendation behavior remains consistent + - finish/cancel resets only the intended builder session state + +## Assumptions And Defaults + +- Prefer route params or local feature state over creating new one-field stores. +- Prefer the typed engine listener layer over store plumbing when an engine event only nudges mounted feature UI and can be safely missed while unmounted. +- Keep notification-backed shared caches unless they are clearly just convenience state for one screen. +- `modal-header` is not a restructuring target beyond extracting route-owned or screen-owned values. +- `settings-password` may remain unchanged if there is no clean merge target. +- `team-building` remains a feature-scoped provider; only duplicated screen-owned fields should move out. +- Delete dead store APIs in the same commit that removes their last caller. +- Prefer colocated `C.useRPC` calls over convenience wrapper hooks when the RPC belongs to one feature and the abstraction is only saving a few lines. diff --git a/shared/chat/conversation/messages/cards/team-journey/container.tsx b/shared/chat/conversation/messages/cards/team-journey/container.tsx index 2e5842867319..6ee6363850ca 100644 --- a/shared/chat/conversation/messages/cards/team-journey/container.tsx +++ b/shared/chat/conversation/messages/cards/team-journey/container.tsx @@ -35,7 +35,7 @@ const TeamJourneyConnected = (ownProps: OwnProps) => { const setMemberPublicity = Teams.useTeamsState(s => s.dispatch.setMemberPublicity) const _onPublishTeam = (teamID: string) => { - navigateAppend('profileShowcaseTeamOffer') + navigateAppend({name: 'profileShowcaseTeamOffer', params: {}}) setMemberPublicity(teamID, true) } const onAuthorClick = () => _onAuthorClick(teamID) diff --git a/shared/chat/conversation/rekey/container.tsx b/shared/chat/conversation/rekey/container.tsx index 0e5e820b8dad..f8006311307c 100644 --- a/shared/chat/conversation/rekey/container.tsx +++ b/shared/chat/conversation/rekey/container.tsx @@ -12,7 +12,7 @@ const Container = () => { const onBack = C.Router2.navigateUp const navigateAppend = C.Router2.navigateAppend const onEnterPaperkey = () => { - navigateAppend('chatEnterPaperkey') + navigateAppend({name: 'chatEnterPaperkey', params: {}}) } const rekeyShowPendingRekeyStatus = C.useRPC(T.RPCGen.rekeyShowPendingRekeyStatusRpcPromise) const onRekey = () => { diff --git a/shared/common-adapters/reload.tsx b/shared/common-adapters/reload.tsx index 52d779d1ec3a..46654854e6f5 100644 --- a/shared/common-adapters/reload.tsx +++ b/shared/common-adapters/reload.tsx @@ -215,9 +215,10 @@ const ReloadContainer = (ownProps: OwnProps) => { } const navigateAppend = C.Router2.navigateAppend + const switchTab = C.Router2.switchTab const _onFeedback = (loggedIn: boolean) => { if (loggedIn) { - navigateAppend(C.Tabs.settingsTab) + switchTab(C.Tabs.settingsTab) navigateAppend({name: settingsFeedbackTab, params: {}}) } else { navigateAppend({name: 'feedback', params: {}}) diff --git a/shared/constants/init/shared.tsx b/shared/constants/init/shared.tsx index c0c2eb4f99fc..406c3247d0d5 100644 --- a/shared/constants/init/shared.tsx +++ b/shared/constants/init/shared.tsx @@ -21,13 +21,13 @@ import type * as UseArchiveStateType from '@/stores/archive' import type * as UseChatStateType from '@/stores/chat' import type * as UseFSStateType from '@/stores/fs' import type * as UseNotificationsStateType from '@/stores/notifications' -import type * as UsePeopleStateType from '@/stores/people' import type * as UsePinentryStateType from '@/stores/pinentry' import type * as UseSettingsPasswordStateType from '@/stores/settings-password' import type * as UseTeamsStateType from '@/stores/teams' import type * as UseTracker2StateType from '@/stores/tracker' import type * as UnlockFoldersType from '@/stores/unlock-folders' import type * as UseUsersStateType from '@/stores/users' +import {notifyEngineActionListeners} from '@/engine/action-listener' import {getTBStore} from '@/stores/team-building' import {getSelectedConversation} from '@/constants/chat/common' import {emitDeepLink} from '@/router-v2/linking' @@ -42,7 +42,6 @@ import {useDarkModeState} from '@/stores/darkmode' import {useFollowerState} from '@/stores/followers' import {useFSState} from '@/stores/fs' import {useModalHeaderState} from '@/stores/modal-header' -import {usePeopleState} from '@/stores/people' import {useProvisionState} from '@/stores/provision' import {useShellState} from '@/stores/shell' import {useSettingsEmailState} from '@/stores/settings-email' @@ -376,30 +375,10 @@ export const initSharedSubscriptions = () => { } } - // Clear "just signed up email" when you leave the people tab after signup - if (prev && Util.getTab(prev) === Tabs.peopleTab && next && Util.getTab(next) !== Tabs.peopleTab) { - clearSignupEmail() - } - - if (prev && Util.getTab(prev) === Tabs.peopleTab && next && Util.getTab(next) !== Tabs.peopleTab) { - usePeopleState.getState().dispatch.markViewed() - } - if (prev && Util.getTab(prev) === Tabs.teamsTab && next && Util.getTab(next) !== Tabs.teamsTab) { useTeamsState.getState().dispatch.clearNavBadges() } - // Clear "check your inbox" in settings when you leave the settings tab - if ( - prev && - Util.getTab(prev) === Tabs.settingsTab && - next && - Util.getTab(next) !== Tabs.settingsTab && - useSettingsEmailState.getState().addedEmail - ) { - useSettingsEmailState.getState().dispatch.resetAddedEmail() - } - onConvoRouteChanged(prev, next) }) ) @@ -503,12 +482,6 @@ export const _onEngineIncoming = (action: EngineGen.Actions) => { useFSState.getState().dispatch.onEngineIncomingImpl(action) } break - case 'keybase.1.homeUI.homeUIRefresh': - { - const {usePeopleState} = require('@/stores/people') as typeof UsePeopleStateType - usePeopleState.getState().dispatch.onEngineIncomingImpl(action) - } - break case 'keybase.1.NotifyEmailAddress.emailAddressVerified': { const emailAddress = action.payload.params.emailAddress @@ -668,4 +641,5 @@ export const _onEngineIncoming = (action: EngineGen.Actions) => { default: } useConfigState.getState().dispatch.onEngineIncoming(action) + notifyEngineActionListeners(action) } diff --git a/shared/constants/router.tsx b/shared/constants/router.tsx index 0c13e3cdd193..b608b3411ad0 100644 --- a/shared/constants/router.tsx +++ b/shared/constants/router.tsx @@ -38,11 +38,11 @@ type IsExactlyRecord = string extends keyof T ? true : false type NavigatorParamsFromProps

= P extends Record ? IsExactlyRecord

extends true - ? undefined + ? {} : keyof P extends never - ? undefined + ? {} : P - : undefined + : {} type LazyInnerComponent> = COM extends React.LazyExoticComponent ? Inner : never @@ -289,15 +289,9 @@ export function navigateAppend(path: NavigateAppendType, replace?: boolean) { if (!ns) { return } - let routeName: string | undefined - let params: object | undefined - if (typeof path === 'string') { - routeName = path - } else { - const nextPath = path as {name: string | number | symbol; params?: object} - routeName = typeof nextPath.name === 'string' ? nextPath.name : String(nextPath.name) - params = nextPath.params - } + const nextPath = path as {name: string | number | symbol; params: object} + const routeName = typeof nextPath.name === 'string' ? nextPath.name : String(nextPath.name) + const params = nextPath.params if (!routeName) { DEBUG_NAV && console.log('[Nav] navigateAppend no routeName bail', routeName) return @@ -313,7 +307,7 @@ export function navigateAppend(path: NavigateAppendType, replace?: boolean) { if (replace) { if (visible?.name === routeName) { - params && n.dispatch(CommonActions.setParams(params)) + n.dispatch(CommonActions.setParams(params)) return } else { n.dispatch(StackActions.replace(routeName, params)) @@ -594,12 +588,7 @@ export const setChatRootParams = (params: Partial { if (i !== chatTabIndex) return route - const currentParams = - currentChatRoot?.name === 'chatRoot' && - currentChatRoot.params && - typeof currentChatRoot.params === 'object' - ? currentChatRoot.params - : undefined + const currentParams = currentChatRoot?.name === 'chatRoot' ? currentChatRoot.params : undefined return { ...route, state: { diff --git a/shared/crypto/sub-nav/index.desktop.tsx b/shared/crypto/sub-nav/index.desktop.tsx index 1fdfbddc7004..4a734f2d66ee 100644 --- a/shared/crypto/sub-nav/index.desktop.tsx +++ b/shared/crypto/sub-nav/index.desktop.tsx @@ -7,7 +7,6 @@ import { useNavigationBuilder, TabRouter, createNavigatorFactory, - type NavigationContainerRef, } from '@react-navigation/core' import type {TypedNavigator, NavigatorTypeBagBase} from '@react-navigation/native' import {routeMapToScreenElements} from '@/router-v2/routes' @@ -59,10 +58,7 @@ function LeftTabNavigator({ }) const selectedTab = state.routes[state.index]?.name ?? '' - const onSelectTab = Common.useSubnavTabAction( - navigation as unknown as NavigationContainerRef, - state - ) + const onSelectTab = Common.useSubnavTabAction(navigation, state) return ( @@ -95,7 +91,7 @@ const styles = Kb.Styles.styleSheetCreate(() => ({ type NavType = NavigatorTypeBagBase & { ParamList: { - [key in keyof typeof cryptoSubRoutes]: undefined + [key in keyof typeof cryptoSubRoutes]: {} } } diff --git a/shared/desktop/renderer/remote-event-handler.desktop.tsx b/shared/desktop/renderer/remote-event-handler.desktop.tsx index 9c05a334b794..0b267353f9ec 100644 --- a/shared/desktop/renderer/remote-event-handler.desktop.tsx +++ b/shared/desktop/renderer/remote-event-handler.desktop.tsx @@ -109,7 +109,7 @@ export const eventFromRemoteWindows = (action: RemoteGen.Actions) => { if (action.payload.path) { FSConstants.navToPath(action.payload.path) } else { - navigateAppend(Tabs.fsTab) + switchTab(Tabs.fsTab) } break } diff --git a/shared/devices/add-device.tsx b/shared/devices/add-device.tsx index 7c5b0da7b2fb..5eb8935aaef4 100644 --- a/shared/devices/add-device.tsx +++ b/shared/devices/add-device.tsx @@ -44,7 +44,7 @@ export default function AddDevice(ownProps: OwnProps) { const onAddPaperKey = () => { if (!canAddPaperKeyRef.current) return canAddPaperKeyRef.current = false - navigateAppend('devicePaperKey') + navigateAppend({name: 'devicePaperKey', params: {}}) setTimeout(() => { canAddPaperKeyRef.current = true }, 1000) diff --git a/shared/engine/action-listener.test.ts b/shared/engine/action-listener.test.ts new file mode 100644 index 000000000000..3bb0eeb3df42 --- /dev/null +++ b/shared/engine/action-listener.test.ts @@ -0,0 +1,66 @@ +/// +import {resetAllStores} from '@/util/zustand' + +import { + clearAllEngineActionListeners, + notifyEngineActionListeners, + subscribeToEngineAction, +} from './action-listener' + +afterEach(() => { + jest.restoreAllMocks() + resetAllStores() +}) + +test('engine action listeners only fire for matching action types', () => { + const homeListener = jest.fn() + const badgeListener = jest.fn() + + const unsubscribeHome = subscribeToEngineAction('keybase.1.homeUI.homeUIRefresh', homeListener) + subscribeToEngineAction('keybase.1.NotifyBadges.badgeState', badgeListener) + + notifyEngineActionListeners({ + payload: {params: {}}, + type: 'keybase.1.homeUI.homeUIRefresh', + } as never) + + expect(homeListener).toHaveBeenCalledTimes(1) + expect(badgeListener).not.toHaveBeenCalled() + + unsubscribeHome() + + notifyEngineActionListeners({ + payload: {params: {}}, + type: 'keybase.1.homeUI.homeUIRefresh', + } as never) + + expect(homeListener).toHaveBeenCalledTimes(1) +}) + +test('resetAllStores clears engine action listeners', () => { + const homeListener = jest.fn() + + subscribeToEngineAction('keybase.1.homeUI.homeUIRefresh', homeListener) + resetAllStores() + + notifyEngineActionListeners({ + payload: {params: {}}, + type: 'keybase.1.homeUI.homeUIRefresh', + } as never) + + expect(homeListener).not.toHaveBeenCalled() +}) + +test('clearAll removes all registered listeners', () => { + const homeListener = jest.fn() + + subscribeToEngineAction('keybase.1.homeUI.homeUIRefresh', homeListener) + clearAllEngineActionListeners() + + notifyEngineActionListeners({ + payload: {params: {}}, + type: 'keybase.1.homeUI.homeUIRefresh', + } as never) + + expect(homeListener).not.toHaveBeenCalled() +}) diff --git a/shared/engine/action-listener.tsx b/shared/engine/action-listener.tsx new file mode 100644 index 000000000000..83bee52d0253 --- /dev/null +++ b/shared/engine/action-listener.tsx @@ -0,0 +1,66 @@ +import * as React from 'react' +import type * as EngineGen from '@/constants/rpc' +import logger from '@/logger' +import {registerExternalResetter} from '@/util/zustand' + +type AnyListener = (action: EngineGen.Actions) => void + +declare global { + var __hmr_engineActionListeners: Map> | undefined +} + +const listenersByType: Map> = __DEV__ + ? (globalThis.__hmr_engineActionListeners ??= new Map()) + : new Map() + +const getListeners = (type: EngineGen.ActionType) => { + let listeners = listenersByType.get(type) + if (!listeners) { + listeners = new Set() + listenersByType.set(type, listeners) + } + return listeners +} + +export const subscribeToEngineAction = ( + type: T, + listener: (action: EngineGen.ActionOf) => void +) => { + const listeners = getListeners(type) + const untypedListener = listener as unknown as AnyListener + listeners.add(untypedListener) + return () => { + listeners.delete(untypedListener) + if (!listeners.size) { + listenersByType.delete(type) + } + } +} + +export const useEngineActionListener = ( + type: T, + listener: (action: EngineGen.ActionOf) => void +) => { + const onAction = React.useEffectEvent(listener) + React.useEffect(() => subscribeToEngineAction(type, action => onAction(action)), [type]) +} + +export const notifyEngineActionListeners = (action: EngineGen.Actions) => { + const listeners = listenersByType.get(action.type) + if (!listeners?.size) { + return + } + for (const listener of [...listeners]) { + try { + listener(action) + } catch (error) { + logger.error(`Error in engine action listener for ${action.type}`, error) + } + } +} + +export const clearAllEngineActionListeners = () => { + listenersByType.clear() +} + +registerExternalResetter('engine-action-listeners', clearAllEngineActionListeners) diff --git a/shared/login/join-or-login.tsx b/shared/login/join-or-login.tsx index 1e6a9870c277..ec38d5fbaf20 100644 --- a/shared/login/join-or-login.tsx +++ b/shared/login/join-or-login.tsx @@ -29,7 +29,7 @@ const Intro = () => { requestAutoInvite() } const showProxySettings = () => { - navigateAppend('proxySettingsModal') + navigateAppend({name: 'proxySettingsModal', params: {}}) } const [showing, setShowing] = React.useState(true) Kb.useInterval(checkIsOnline, showing ? 5000 : undefined) diff --git a/shared/login/relogin/container.tsx b/shared/login/relogin/container.tsx index 8366eeef8c32..0378c2a44566 100644 --- a/shared/login/relogin/container.tsx +++ b/shared/login/relogin/container.tsx @@ -19,7 +19,7 @@ const ReloginContainer = () => { } const navigateAppend = C.Router2.navigateAppend const onFeedback = () => { - navigateAppend('signupSendFeedbackLoggedOut') + navigateAppend({name: 'signupSendFeedbackLoggedOut', params: {}}) } const onLogin = useConfigState(s => s.dispatch.login) const requestAutoInvite = useRequestAutoInvite() diff --git a/shared/login/reset/waiting.tsx b/shared/login/reset/waiting.tsx index 7efff9cd5d02..b932f503b016 100644 --- a/shared/login/reset/waiting.tsx +++ b/shared/login/reset/waiting.tsx @@ -21,7 +21,7 @@ const Waiting = ({endTime: routeEndTime, pipelineStarted, username}: Props) => { const [hasSentAgain, setHasSentAgain] = React.useState(false) const [sendAgainSuccess, setSendAgainSuccess] = React.useState(false) const nav = useSafeNavigation() - const onClose = () => nav.safeNavigateAppend('login', true) + const onClose = () => nav.safeNavigateAppend({name: 'login', params: {}}, true) const onSendAgain = () => { setHasSentAgain(true) setSendAgainSuccess(false) diff --git a/shared/people/announcement.tsx b/shared/people/announcement.tsx index dbff0f8dfd2a..90a069c49df9 100644 --- a/shared/people/announcement.tsx +++ b/shared/people/announcement.tsx @@ -37,13 +37,13 @@ const Container = (ownProps: OwnProps) => { case T.RPCGen.AppLinkType.files: switchTab(C.isMobile ? C.Tabs.settingsTab : C.Tabs.fsTab) if (C.isMobile) { - navigateAppend(Settings.settingsFsTab) + navigateAppend({name: Settings.settingsFsTab, params: {}}) } break case T.RPCGen.AppLinkType.wallet: switchTab(C.Tabs.settingsTab) if (C.isMobile) { - navigateAppend(Settings.settingsWalletsTab) + navigateAppend({name: Settings.settingsWalletsTab, params: {}}) } break case T.RPCGen.AppLinkType.git: @@ -55,7 +55,7 @@ const Container = (ownProps: OwnProps) => { case T.RPCGen.AppLinkType.devices: switchTab(C.isMobile ? C.Tabs.settingsTab : C.Tabs.devicesTab) if (C.isMobile) { - navigateAppend(Settings.settingsDevicesTab) + navigateAppend({name: Settings.settingsDevicesTab, params: {}}) } break case T.RPCGen.AppLinkType.settings: diff --git a/shared/people/container.tsx b/shared/people/container.tsx index 1b3f14f71612..048abdc183bb 100644 --- a/shared/people/container.tsx +++ b/shared/people/container.tsx @@ -1,21 +1,23 @@ import * as C from '@/constants' -import {ignorePromise} from '@/constants/utils' +import {ignorePromise, RPCError, isNetworkErr} from '@/constants/utils' import * as React from 'react' import * as Kb from '@/common-adapters' import type {IconType} from '@/common-adapters/icon.constants-gen' // do NOT pull in all of common-adapters +import {useNavigation} from '@react-navigation/native' import {isMobile} from '@/constants/platform' import type {DebouncedFunc} from 'lodash' import debounce from 'lodash/debounce' import invert from 'lodash/invert' import isEqual from 'lodash/isEqual' +import logger from '@/logger' import People from '.' import * as T from '@/constants/types' +import {useEngineActionListener} from '@/engine/action-listener' import {useFollowerState} from '@/stores/followers' -import {usePeopleState} from '@/stores/people' import {useCurrentUserState} from '@/stores/current-user' import type {e164ToDisplay as e164ToDisplayType} from '@/util/phone-numbers' import {navToProfile} from '@/constants/router' -import {useSignupEmail} from './signup-email' +import {clearSignupEmail, useSignupEmail} from './signup-email' const getPeopleDataWaitingKey = 'getPeopleData' const waitToRefresh = 1000 * 60 * 5 @@ -26,6 +28,24 @@ const todoTypeEnumToType = invert(T.RPCGen.HomeScreenTodoType) as { [K in T.People.TodoTypeEnum]: T.People.TodoType } +const markViewed = () => { + const f = async () => { + try { + await T.RPCGen.homeHomeMarkViewedRpcPromise() + } catch (error) { + if (!(error instanceof RPCError)) { + throw error + } + if (isNetworkErr(error.code)) { + logger.warn('Network error calling homeMarkViewed') + } else { + throw error + } + } + } + ignorePromise(f()) +} + const todoTypeToInstructions: {[K in T.People.TodoType]: string} = { addEmail: 'Add an email address for security purposes, and to get important notifications.', addPhoneNumber: 'Add your phone number so your friends can find you.', @@ -396,12 +416,11 @@ const PeopleReloadable = () => { setResentEmail, skipTodo, } = usePeoplePageState() - const refreshCount = usePeopleState(s => s.refreshCount) + const navigation = useNavigation() const username = useCurrentUserState(s => s.username) const signupEmail = useSignupEmail() const waiting = C.Waiting.useAnyWaiting(getPeopleDataWaitingKey) const lastRefreshRef = React.useRef(0) - const lastSeenRefreshRef = React.useRef(refreshCount) const didInitialLoadRef = React.useRef(false) const getData = React.useEffectEvent((markViewed = true, force = false) => { @@ -412,13 +431,6 @@ const PeopleReloadable = () => { } }) - React.useEffect(() => { - if (refreshCount !== lastSeenRefreshRef.current) { - lastSeenRefreshRef.current = refreshCount - getData(false, true) - } - }, [refreshCount]) - React.useEffect(() => { if (!didInitialLoadRef.current) { didInitialLoadRef.current = true @@ -431,6 +443,16 @@ const PeopleReloadable = () => { const onReload = (isRetry?: boolean) => getData(false, isRetry === true || !followSuggestions.length) C.Router2.useSafeFocusEffect(onReload) + useEngineActionListener('keybase.1.homeUI.homeUIRefresh', () => { + getData(false, true) + }) + + React.useEffect(() => { + return navigation.addListener('blur', () => { + clearSignupEmail() + markViewed() + }) + }, [navigation]) return ( diff --git a/shared/people/routes.tsx b/shared/people/routes.tsx index 21a986557ff1..779d67f09eed 100644 --- a/shared/people/routes.tsx +++ b/shared/people/routes.tsx @@ -10,7 +10,7 @@ import {defineRouteMap} from '@/constants/types/router' const HeaderAvatar = () => { const myUsername = useCurrentUserState(s => s.username) const navigateAppend = C.Router2.navigateAppend - const onClick = () => navigateAppend('accountSwitcher') + const onClick = () => navigateAppend({name: 'accountSwitcher', params: {}}) return } @@ -29,7 +29,7 @@ const AccountSignOutButton = () => { return ( navigateAppend(settingsLogOutTab)} + onClick={() => navigateAppend({name: settingsLogOutTab, params: {}})} style={{color: Kb.Styles.globalColors.red, padding: 8}} > Sign out diff --git a/shared/people/todo.tsx b/shared/people/todo.tsx index 79387726a343..2cc1ed67977f 100644 --- a/shared/people/todo.tsx +++ b/shared/people/todo.tsx @@ -116,8 +116,8 @@ const SettingsAccountTask = ({ const {navigateAppend, switchTab} = useRouterNavigation() const onConfirm = () => { switchTab(C.Tabs.settingsTab) - navigateAppend(settingsAccountTab) - destination && navigateAppend(destination) + navigateAppend({name: settingsAccountTab, params: {}}) + destination && navigateAppend({name: destination, params: {}}) } return } @@ -214,13 +214,12 @@ const GitRepoTask = (props: TodoOwnProps) => { const VerifyAllEmailTask = (props: TodoOwnProps) => { const editEmail = useSettingsEmailState(s => s.dispatch.editEmail) const onConfirm = (email: string) => { - editEmail({email, verify: true}) - props.setResentEmail(email) + editEmail({email, onSuccess: () => props.setResentEmail(email), verify: true}) } const {navigateAppend, switchTab} = useRouterNavigation() const onManage = () => { switchTab(C.Tabs.settingsTab) - navigateAppend(settingsAccountTab) + navigateAppend({name: settingsAccountTab, params: {}}) } const meta = props.metadata?.type === 'email' ? props.metadata : undefined @@ -255,7 +254,7 @@ const VerifyAllPhoneNumberTask = (props: TodoOwnProps) => { } const onManage = () => { switchTab(C.Tabs.settingsTab) - navigateAppend(settingsAccountTab) + navigateAppend({name: settingsAccountTab, params: {}}) } const buttons: Array = [ ...(props.metadata @@ -284,7 +283,7 @@ const LegacyEmailVisibilityTask = (props: TodoOwnProps) => { const {navigateAppend, switchTab} = useRouterNavigation() const onConfirm = (email: string) => { switchTab(C.Tabs.settingsTab) - navigateAppend(settingsAccountTab) + navigateAppend({name: settingsAccountTab, params: {}}) editEmail({email, makeSearchable: true}) } const onDismiss = useOnSkipTodo(props.skipTodo, 'legacyEmailVisibility') diff --git a/shared/profile/generic/proofs-list.tsx b/shared/profile/generic/proofs-list.tsx index 9f1684f3c096..1e043119f73f 100644 --- a/shared/profile/generic/proofs-list.tsx +++ b/shared/profile/generic/proofs-list.tsx @@ -234,7 +234,7 @@ const Container = ({platform, reason = 'profile'}: Props) => { setStepSafe({error: '', kind: 'enterUsername', platform: service, username: ''}) return case 'pgp': - navigateAppend('profilePgp') + navigateAppend({name: 'profilePgp', params: {}}) return default: break diff --git a/shared/profile/pgp/choice/index.desktop.tsx b/shared/profile/pgp/choice/index.desktop.tsx index 731eb82e9ef1..73a84d794a43 100644 --- a/shared/profile/pgp/choice/index.desktop.tsx +++ b/shared/profile/pgp/choice/index.desktop.tsx @@ -67,7 +67,7 @@ export default function Choice() { setStepSafe({kind: 'info'}) } const onShowImport = () => { - navigateAppend('profileImport') + navigateAppend({name: 'profileImport', params: {}}) } const onUpdate = (next: Partial) => { diff --git a/shared/profile/user/actions/index.tsx b/shared/profile/user/actions/index.tsx index c362484001b4..636d2f4597d7 100644 --- a/shared/profile/user/actions/index.tsx +++ b/shared/profile/user/actions/index.tsx @@ -29,7 +29,7 @@ const Container = (ownProps: OwnProps) => { const _onAddToTeam = (username: string) => navigateAppend({name: 'profileAddToTeam', params: {username}}) const _onBrowsePublicFolder = (username: string) => FS.navToPath(T.FS.stringToPath(`/keybase/public/${username}`)) - const _onEditProfile = () => navigateAppend('profileEdit') + const _onEditProfile = () => navigateAppend({name: 'profileEdit', params: {}}) const changeFollow = useTrackerState(s => s.dispatch.changeFollow) const _onFollow = changeFollow diff --git a/shared/profile/user/teams/index.tsx b/shared/profile/user/teams/index.tsx index 9995ef83133b..1b6e5bad29fe 100644 --- a/shared/profile/user/teams/index.tsx +++ b/shared/profile/user/teams/index.tsx @@ -27,7 +27,7 @@ const Container = (ownProps: OwnProps) => { const teamShowcase = d.teamShowcase || noTeams const {clearModals, navigateAppend} = C.Router2 const _onEdit = () => { - navigateAppend('profileShowcaseTeamOffer') + navigateAppend({name: 'profileShowcaseTeamOffer', params: {}}) } const onJoinTeam = (teamname: string) => navigateAppend({name: 'teamJoinTeamDialog', params: {initialTeamname: teamname}}) const onViewTeam = (teamname: string) => { diff --git a/shared/provision/username-or-email.tsx b/shared/provision/username-or-email.tsx index cf383ba41ea4..51958953bfea 100644 --- a/shared/provision/username-or-email.tsx +++ b/shared/provision/username-or-email.tsx @@ -47,7 +47,7 @@ const UsernameOrEmailContainer = (op: OwnProps) => { const navigateUp = C.Router2.navigateUp const onBack = useSafeSubmit(navigateUp, hasError) const navigateAppend = C.Router2.navigateAppend - const onForgotUsername = () => navigateAppend('forgotUsername') + const onForgotUsername = () => navigateAppend({name: 'forgotUsername', params: {}}) const requestAutoInvite = useRequestAutoInvite() const _setUsername = useProvisionState(s => s.dispatch.dynamic.setUsername) const _onSubmit = (username: string) => { diff --git a/shared/router-v2/account-switcher/index.tsx b/shared/router-v2/account-switcher/index.tsx index bbd323b0f596..b408bab81368 100644 --- a/shared/router-v2/account-switcher/index.tsx +++ b/shared/router-v2/account-switcher/index.tsx @@ -36,7 +36,7 @@ const Container = () => { const onSelectAccountLoggedOut = useConfigState(s => s.dispatch.logoutAndTryToLogInAs) const navigateAppend = C.Router2.navigateAppend const onSignOut = () => { - navigateAppend(settingsLogOutTab) + navigateAppend({name: settingsLogOutTab, params: {}}) } const accountRows = prepareAccountRows(_accountRows, you) diff --git a/shared/router-v2/common.d.ts b/shared/router-v2/common.d.ts index b4f12f2ca58d..f726c4cc625f 100644 --- a/shared/router-v2/common.d.ts +++ b/shared/router-v2/common.d.ts @@ -1,5 +1,6 @@ -import type {NavigationContainerRef} from '@react-navigation/core' import type {NavState} from '@/constants/router' +import type {ParamListBase} from '@react-navigation/native' +import type {NavigationContainerRef} from '@react-navigation/core' import type * as Styles from '@/styles' import type {NativeStackNavigationOptions} from '@react-navigation/native-stack' export const tabBarStyle: Styles.StylesCrossPlatform @@ -16,7 +17,9 @@ export const defaultNavigationOptions: NativeStackNavigationOptions & { headerTitleContainerStyle?: Styles.StylesCrossPlatform } +type SubnavNavigation = Pick, 'dispatch' | 'emit'> + export function useSubnavTabAction( - navigation: NavigationContainerRef, + navigation: SubnavNavigation, state: NavState ): (t: string) => void diff --git a/shared/router-v2/common.native.tsx b/shared/router-v2/common.native.tsx index 36d399998fbc..a6c0acb0ea2a 100644 --- a/shared/router-v2/common.native.tsx +++ b/shared/router-v2/common.native.tsx @@ -1,6 +1,7 @@ import type * as React from 'react' import * as Kb from '@/common-adapters' import {TabActions, type NavigationContainerRef} from '@react-navigation/core' +import type {ParamListBase} from '@react-navigation/native' import type {HeaderOptions} from '@react-navigation/elements' import {HeaderLeftButton} from '@/common-adapters/header-buttons' import type {NavState} from '@/constants/router' @@ -85,7 +86,9 @@ const styles = Kb.Styles.styleSheetCreate(() => ({ }, })) -export const useSubnavTabAction = (navigation: NavigationContainerRef, state: NavState) => { +type SubnavNavigation = Pick, 'dispatch' | 'emit'> + +export const useSubnavTabAction = (navigation: SubnavNavigation, state: NavState) => { const onSelectTab = (tab: string) => { const routes = state && 'routes' in state ? state.routes : undefined const route = routes?.find((r: {name?: string; key?: string}) => r.name === tab) diff --git a/shared/router-v2/route-params.tsx b/shared/router-v2/route-params.tsx index a6706d17be2b..85c0682b4fec 100644 --- a/shared/router-v2/route-params.tsx +++ b/shared/router-v2/route-params.tsx @@ -16,40 +16,36 @@ type _ExtractParams = { ? U extends (args: infer V) => any ? V extends {route: {params: infer W}} ? W - : undefined - : undefined - : undefined + : V extends {route: {params?: infer W}} + ? W + : {} + : {} + : {} } type Tabs = { - 'tabs.chatTab': undefined - 'tabs.cryptoTab': undefined - 'tabs.devicesTab': undefined - 'tabs.folderTab': undefined - 'tabs.loginTab': undefined - 'tabs.peopleTab': undefined - 'tabs.searchTab': undefined - 'tabs.settingsTab': undefined - 'tabs.teamsTab': undefined - 'tabs.gitTab': undefined - 'tabs.fsTab': undefined - 'tabs.walletsTab': undefined + 'tabs.chatTab': {} + 'tabs.cryptoTab': {} + 'tabs.devicesTab': {} + 'tabs.folderTab': {} + 'tabs.loginTab': {} + 'tabs.peopleTab': {} + 'tabs.searchTab': {} + 'tabs.settingsTab': {} + 'tabs.teamsTab': {} + 'tabs.gitTab': {} + 'tabs.fsTab': {} + 'tabs.walletsTab': {} } type _AllScreens = typeof routes & typeof modalRoutes & typeof loggedOutRoutes export type RootParamList = _ExtractParams<_AllScreens> & - Tabs & {loading: undefined; loggedOut: undefined; loggedIn: undefined} + Tabs & {loading: {}; loggedOut: {}; loggedIn: {}} export type RouteKeys = keyof RootParamList -export type NoParamRouteKeys = { - [K in RouteKeys]: RootParamList[K] extends undefined ? K : never -}[RouteKeys] -export type ParamRouteKeys = Exclude export type NavigateAppendArg = RouteName extends RouteName - ? RootParamList[RouteName] extends undefined - ? RouteName - : {name: RouteName; params: RootParamList[RouteName]} + ? {name: RouteName; params: RootParamList[RouteName]} : never export type NavigateAppendType = NavigateAppendArg export type RootRouteProps = RouteProp diff --git a/shared/router-v2/tab-bar.desktop.tsx b/shared/router-v2/tab-bar.desktop.tsx index e17ce124643e..8a3b140723b5 100644 --- a/shared/router-v2/tab-bar.desktop.tsx +++ b/shared/router-v2/tab-bar.desktop.tsx @@ -64,7 +64,7 @@ const Header = () => { const {navigateAppend, switchTab} = C.Router2 const onSettings = () => switchTab(Tabs.settingsTab) - const onSignOut = () => navigateAppend(settingsLogOutTab) + const onSignOut = () => navigateAppend({name: settingsLogOutTab, params: {}}) const onAddAccount = () => { logoutToLoggedOutFlow() } @@ -171,7 +171,7 @@ function TabBar(props: Props) { const {navigation, state} = props const username = useCurrentUserState(s => s.username) const onHotKey = (cmd: string) => { - navigation.navigate(keysMap[cmd] as Tabs.Tab) + navigation.navigate({name: keysMap[cmd] as Tabs.Tab, params: {}}) } Kb.useHotKey(hotKeys, onHotKey) diff --git a/shared/settings/account/add-modals.tsx b/shared/settings/account/add-modals.tsx index 7392e7f52e13..2feee672ace7 100644 --- a/shared/settings/account/add-modals.tsx +++ b/shared/settings/account/add-modals.tsx @@ -7,8 +7,8 @@ import {EnterPhoneNumberBody} from '@/signup/phone-number' import VerifyBody from '@/signup/phone-number/verify-body' import {useAddPhoneNumber, usePhoneVerification} from '@/signup/phone-number/use-verification' import {useAddEmail} from './use-add-email' -import {useSettingsPhoneState} from '@/stores/settings-phone' import {useDefaultPhoneCountry} from '@/util/phone-numbers' +import {settingsAccountTab} from '@/constants/settings' export const Email = () => { const nav = useSafeNavigation() @@ -22,6 +22,7 @@ export const Email = () => { const {clearError, error: emailError, submitEmail, waiting} = useAddEmail() const clearModals = C.Router2.clearModals + const navigateAppend = C.Router2.navigateAppend // clean on edit React.useEffect(() => { @@ -36,8 +37,12 @@ export const Email = () => { return } setSubmittedEmail(emailTrimmed) - submitEmail(emailTrimmed, searchable, () => { + submitEmail(emailTrimmed, searchable, addedEmail => { clearModals() + // Wait for the modal reset so replace=true updates the account route instead of duplicating it. + setTimeout(() => { + navigateAppend({name: settingsAccountTab, params: {addedEmailBannerEmail: addedEmail}}, true) + }, 0) }) } return ( @@ -185,13 +190,16 @@ type VerifyPhoneProps = { export const VerifyPhone = ({initialResend, phoneNumber}: VerifyPhoneProps) => { const [code, onChangeCode] = React.useState('') - const setAddedPhone = useSettingsPhoneState(s => s.dispatch.setAddedPhone) const clearModals = C.Router2.clearModals + const navigateAppend = C.Router2.navigateAppend const {error, resendVerificationForPhone, verifyPhoneNumber} = usePhoneVerification({ initialResend, onSuccess: () => { - setAddedPhone(true) clearModals() + // Wait for the modal reset so replace=true updates the account route instead of duplicating it. + setTimeout(() => { + navigateAppend({name: settingsAccountTab, params: {addedPhoneBanner: true}}, true) + }, 0) }, phoneNumber, }) diff --git a/shared/settings/account/confirm-delete.tsx b/shared/settings/account/confirm-delete.tsx index 9aa082b3c5a0..d2a5a7bb2e51 100644 --- a/shared/settings/account/confirm-delete.tsx +++ b/shared/settings/account/confirm-delete.tsx @@ -1,7 +1,11 @@ +import * as C from '@/constants' import * as Kb from '@/common-adapters' +import * as React from 'react' +import * as T from '@/constants/types' +import logger from '@/logger' +import {makePhoneError} from '@/stores/settings-phone' import * as PhoneUtil from '@/util/phone-numbers' import {useSafeNavigation} from '@/util/safe-navigation' -import {useSettingsPhoneState} from '@/stores/settings-phone' import {useSettingsEmailState} from '@/stores/settings-email' type OwnProps = { @@ -19,16 +23,26 @@ const DeleteModal = (props: OwnProps) => { const lastEmail = props.lastEmail ?? false const onCancel = () => nav.safeNavigateUp() - const editPhone = useSettingsPhoneState(s => s.dispatch.editPhone) + const deletePhoneNumber = C.useRPC(T.RPCGen.phoneNumbersDeletePhoneNumberRpcPromise) const editEmail = useSettingsEmailState(s => s.dispatch.editEmail) + const [error, setError] = React.useState('') const onConfirm = () => { if (itemType === 'phone') { - editPhone(itemAddress, true) + setError('') + deletePhoneNumber( + [{phoneNumber: itemAddress}], + () => { + nav.safeNavigateUp() + }, + error_ => { + logger.warn('Error deleting phone number', error_) + setError(makePhoneError(error_)) + } + ) } else { editEmail({delete: true, email: itemAddress}) + nav.safeNavigateUp() } - - nav.safeNavigateUp() } const icon = @@ -73,6 +87,7 @@ const DeleteModal = (props: OwnProps) => { icon={icon} prompt={prompt} description={description} + error={error} onCancel={onCancel} onConfirm={onConfirm} confirmText="Yes, delete" diff --git a/shared/settings/account/email-phone-row.tsx b/shared/settings/account/email-phone-row.tsx index cae6785f555a..c06bdfc56f58 100644 --- a/shared/settings/account/email-phone-row.tsx +++ b/shared/settings/account/email-phone-row.tsx @@ -20,8 +20,8 @@ const Badge = (p: {backgroundColor: string; menuItem?: boolean}) => ( /> ) -const EmailPhoneRow = (p: {contactKey: string}) => { - const props = useData(p.contactKey) +const EmailPhoneRow = (p: {contactKey: string; onEmailVerificationSuccess: (email: string) => void}) => { + const props = useData(p.contactKey, p.onEmailVerificationSuccess) const {address, onDelete, onMakePrimary, onToggleSearchable, onVerify, moreThanOneEmail} = props const {primary, searchable, superseded, type, verified, lastVerifyEmailDate} = props @@ -196,7 +196,7 @@ const styles = Kb.Styles.styleSheetCreate( }) as const ) -const useData = (contactKey: string) => { +const useData = (contactKey: string, onEmailVerificationSuccess: (email: string) => void) => { const _emailRow = useSettingsEmailState(s => s.emails.get(contactKey) ?? null) const _phoneRow = useSettingsPhoneState(s => s.phones?.get(contactKey) || null) const moreThanOneEmail = useSettingsEmailState(s => s.emails.size > 1) @@ -207,8 +207,7 @@ const useData = (contactKey: string) => { const _onMakeSearchable = () => { editEmail({email: contactKey, makeSearchable: true}) } - - const editPhone = useSettingsPhoneState(s => s.dispatch.editPhone) + const setVisibilityPhoneNumber = C.useRPC(T.RPCGen.phoneNumbersSetVisibilityPhoneNumberRpcPromise) const navigateAppend = C.Router2.navigateAppend const dispatchProps = { @@ -222,14 +221,25 @@ const useData = (contactKey: string) => { editEmail({email: contactKey, makePrimary: true}) }, onVerify: () => { - editEmail({email: contactKey, verify: true}) + editEmail({email: contactKey, onSuccess: () => onEmailVerificationSuccess(contactKey), verify: true}) }, }, phone: { _onDelete: (address: string, searchable: boolean) => navigateAppend({name: 'settingsDeleteAddress', params: {address, searchable, type: 'phone'}}), _onToggleSearchable: (setSearchable: boolean) => { - editPhone(contactKey, undefined, setSearchable) + setVisibilityPhoneNumber( + [ + { + phoneNumber: contactKey, + visibility: setSearchable + ? T.RPCGen.IdentityVisibility.public + : T.RPCGen.IdentityVisibility.private, + }, + ], + () => {}, + () => {} + ) }, _onVerify: (phoneNumber: string) => { navigateAppend({name: 'settingsVerifyPhone', params: {initialResend: true, phoneNumber}}) diff --git a/shared/settings/account/index.tsx b/shared/settings/account/index.tsx index 3bed427286ee..a915a82f40d3 100644 --- a/shared/settings/account/index.tsx +++ b/shared/settings/account/index.tsx @@ -1,14 +1,19 @@ import * as C from '@/constants' import * as Kb from '@/common-adapters' +import * as React from 'react' import * as T from '@/constants/types' -import type * as React from 'react' import EmailPhoneRow from './email-phone-row' +import logger from '@/logger' import {openURL} from '@/util/misc' import {loadSettings} from '../load-settings' +import {useNavigation, type NavigationProp} from '@react-navigation/native' +import {useIsFocused} from '@react-navigation/core' +import {useConfigState} from '@/stores/config' import {usePWState} from '@/stores/settings-password' -import {useSettingsPhoneState} from '@/stores/settings-phone' +import {makePhoneError, useSettingsPhoneState} from '@/stores/settings-phone' import {useSettingsEmailState} from '@/stores/settings-email' -import {settingsPasswordTab} from '@/constants/settings' +import {type settingsAccountTab, settingsPasswordTab} from '@/constants/settings' +import type {SettingsAccountRouteParams} from '../routes' export const SettingsSection = ({children}: {children: React.ReactNode}) => ( @@ -32,7 +37,7 @@ const AddButton = (props: AddButtonProps) => ( /> ) -const EmailPhone = () => { +const EmailPhone = ({onEmailVerificationSuccess}: {onEmailVerificationSuccess: (email: string) => void}) => { const navigateAppend = C.Router2.navigateAppend const _emails = useSettingsEmailState(s => s.emails) const _phones = useSettingsPhoneState(s => s.phones) @@ -42,10 +47,10 @@ const EmailPhone = () => { const waiting = C.Waiting.useAnyWaiting(C.waitingKeySettingsLoadSettings) const readMoreUrlProps = Kb.useClickURL('https://keybase.io/docs/chat/phones-and-emails') const onAddEmail = () => { - navigateAppend('settingsAddEmail') + navigateAppend({name: 'settingsAddEmail', params: {}}) } const onAddPhone = () => { - navigateAppend('settingsAddPhone') + navigateAppend({name: 'settingsAddPhone', params: {}}) } return ( @@ -59,18 +64,14 @@ const EmailPhone = () => { find you by phone number or email.{' '} Read more{' '} - + {!!contactKeys.length && ( {contactKeys.map(ck => ( - + ))} )} @@ -85,7 +86,7 @@ const EmailPhone = () => { const Password = () => { const navigateAppend = C.Router2.navigateAppend const onSetPassword = () => { - navigateAppend(settingsPasswordTab) + navigateAppend({name: settingsPasswordTab, params: {}}) } const hasPassword = usePWState(s => !s.randomPW) let passwordLabel: string @@ -146,7 +147,7 @@ const WebAuthTokenLogin = () => { const DeleteAccount = () => { const navigateAppend = C.Router2.navigateAppend const onDeleteAccount = () => { - navigateAppend('deleteConfirm') + navigateAppend({name: 'deleteConfirm', params: {}}) } return ( @@ -169,26 +170,25 @@ const DeleteAccount = () => { ) } -const AccountSettings = () => { - const {addedEmail, resetAddedEmail} = useSettingsEmailState( - C.useShallow(s => ({ - addedEmail: s.addedEmail, - resetAddedEmail: s.dispatch.resetAddedEmail, - })) - ) - const { - _phones, - addedPhone, - clearAddedPhone, - editPhone, - } = useSettingsPhoneState( - C.useShallow(s => ({ - _phones: s.phones, - addedPhone: s.addedPhone, - clearAddedPhone: s.dispatch.clearAddedPhone, - editPhone: s.dispatch.editPhone, - })) - ) +type Props = {route: {params?: SettingsAccountRouteParams}} + +const AccountSettings = ({route}: Props) => { + const addedEmailFromRoute = route.params?.addedEmailBannerEmail + const addedPhoneFromRoute = !!route.params?.addedPhoneBanner + const navigation = + useNavigation< + NavigationProp< + Record, + typeof settingsAccountTab + > + >() + const isFocused = useIsFocused() + const emails = useSettingsEmailState(s => s.emails) + const phones = useSettingsPhoneState(s => s.phones) + const setGlobalError = useConfigState(s => s.dispatch.setGlobalError) + const deletePhoneNumber = C.useRPC(T.RPCGen.phoneNumbersDeletePhoneNumberRpcPromise) + const [addedEmail, setAddedEmail] = React.useState(addedEmailFromRoute ?? '') + const [addedPhone, setAddedPhone] = React.useState(addedPhoneFromRoute) const {loadHasRandomPw} = usePWState( C.useShallow(s => ({ loadHasRandomPw: s.dispatch.loadHasRandomPw, @@ -196,10 +196,47 @@ const AccountSettings = () => { ) const {navigateAppend, switchTab} = C.Router2 const _onClearSupersededPhoneNumber = (phone: string) => { - editPhone(phone, true) + deletePhoneNumber( + [{phoneNumber: phone}], + () => {}, + error => { + logger.warn('Error deleting superseded phone number', error) + setGlobalError(new Error(makePhoneError(error))) + } + ) } - const onClearAddedEmail = resetAddedEmail - const onClearAddedPhone = clearAddedPhone + React.useEffect(() => { + if (!addedEmailFromRoute) { + return + } + setAddedEmail(addedEmailFromRoute) + navigation.setParams({addedEmailBannerEmail: undefined}) + }, [addedEmailFromRoute, navigation]) + React.useEffect(() => { + if (!addedPhoneFromRoute) { + return + } + setAddedPhone(true) + navigation.setParams({addedPhoneBanner: undefined}) + }, [addedPhoneFromRoute, navigation]) + React.useEffect(() => { + if (isFocused) { + return + } + setAddedEmail('') + setAddedPhone(false) + }, [isFocused]) + React.useEffect(() => { + if (!addedEmail) { + return + } + const addedEmailRow = emails.get(addedEmail) + if (!addedEmailRow || addedEmailRow.isVerified) { + setAddedEmail('') + } + }, [addedEmail, emails]) + const onClearAddedEmail = () => setAddedEmail('') + const onClearAddedPhone = () => setAddedPhone(false) const onReload = () => { loadSettings() loadHasRandomPw() @@ -207,14 +244,14 @@ const AccountSettings = () => { const onStartPhoneConversation = () => { switchTab(C.Tabs.chatTab) navigateAppend({name: 'chatNewChat', params: {namespace: 'chat'}}) - clearAddedPhone() + setAddedPhone(false) } - const _supersededPhoneNumber = _phones && [..._phones.values()].find(p => p.superseded) + const _supersededPhoneNumber = phones && [...phones.values()].find(p => p.superseded) const supersededKey = _supersededPhoneNumber?.e164 const onClearSupersededPhoneNumber = () => supersededKey && _onClearSupersededPhoneNumber(supersededKey) const supersededPhoneNumber = _supersededPhoneNumber ? _supersededPhoneNumber.displayNumber : undefined const onAddPhone = () => { - navigateAppend('settingsAddPhone') + navigateAppend({name: 'settingsAddPhone', params: {}}) } return ( @@ -255,7 +292,7 @@ const AccountSettings = () => { )} - + diff --git a/shared/settings/account/use-add-email.tsx b/shared/settings/account/use-add-email.tsx index 86da4cdb1f31..ce77552ffacd 100644 --- a/shared/settings/account/use-add-email.tsx +++ b/shared/settings/account/use-add-email.tsx @@ -1,7 +1,6 @@ import * as C from '@/constants' import * as React from 'react' import * as T from '@/constants/types' -import {useSettingsEmailState} from '@/stores/settings-email' import type {RPCError} from '@/util/errors' import {isValidEmail} from '@/util/simple-validators' @@ -22,7 +21,6 @@ const makeAddEmailError = (err: RPCError): string => { export const useAddEmail = () => { const addEmail = C.useRPC(T.RPCGen.emailsAddEmailRpcPromise) - const setAddedEmail = useSettingsEmailState(s => s.dispatch.setAddedEmail) const waiting = C.Waiting.useAnyWaiting(C.addEmailWaitingKey) const [error, setError] = React.useState('') @@ -49,7 +47,6 @@ export const useAddEmail = () => { C.addEmailWaitingKey, ], () => { - setAddedEmail(email) onSuccess(email) }, error_ => { diff --git a/shared/settings/advanced.tsx b/shared/settings/advanced.tsx index 4ad06b9bfa58..c97a56d69264 100644 --- a/shared/settings/advanced.tsx +++ b/shared/settings/advanced.tsx @@ -350,8 +350,8 @@ const Developer = () => { } const processorProfileInProgress = C.Waiting.useAnyWaiting(processorProfileInProgressKey) const navigateAppend = C.Router2.navigateAppend - const onDBNuke = () => navigateAppend('dbNukeConfirm') - const onMakeIcons = () => navigateAppend('makeIcons') + const onDBNuke = () => navigateAppend({name: 'dbNukeConfirm', params: {}}) + const onMakeIcons = () => navigateAppend({name: 'makeIcons', params: {}}) const onClearLogs = () => { const f = async () => { await clearLocalLogs() diff --git a/shared/settings/archive/modal.tsx b/shared/settings/archive/modal.tsx index cfb9499446c3..f8c9d6658b0d 100644 --- a/shared/settings/archive/modal.tsx +++ b/shared/settings/archive/modal.tsx @@ -231,7 +231,7 @@ const ArchiveModal = (p: Props) => { setTimeout(() => { switchTab(C.Tabs.settingsTab) setTimeout(() => { - navigateAppend(settingsArchiveTab) + navigateAppend({name: settingsArchiveTab, params: {}}) }, 200) }, 200) } diff --git a/shared/settings/delete-confirm/index.tsx b/shared/settings/delete-confirm/index.tsx index 3390274fe342..aa362fd2fa63 100644 --- a/shared/settings/delete-confirm/index.tsx +++ b/shared/settings/delete-confirm/index.tsx @@ -51,7 +51,7 @@ const DeleteConfirm = () => { return } if (Kb.Styles.isMobile && hasPassword) { - navigateAppend('checkPassphraseBeforeDeleteAccount') + navigateAppend({name: 'checkPassphraseBeforeDeleteAccount', params: {}}) } else { deleteAccountForever() } diff --git a/shared/settings/files/index.desktop.tsx b/shared/settings/files/index.desktop.tsx index 12e2cba10480..bc92dd99855c 100644 --- a/shared/settings/files/index.desktop.tsx +++ b/shared/settings/files/index.desktop.tsx @@ -63,7 +63,7 @@ const FinderIntegration = () => { ) const navigateAppend = C.Router2.navigateAppend const onShowKextPermissionPopup = () => { - navigateAppend('kextPermission') + navigateAppend({name: 'kextPermission', params: {}}) } const displayingMountDir = preferredMountDirs[0] || '' const openMount = displayingMountDir diff --git a/shared/settings/load-settings.tsx b/shared/settings/load-settings.tsx index d5689bbace31..8770fd103a4a 100644 --- a/shared/settings/load-settings.tsx +++ b/shared/settings/load-settings.tsx @@ -23,7 +23,7 @@ export const loadSettings = () => { } maybeLoadAppLinkOnce = true switchTab(Tabs.settingsTab) - navigateAppend('settingsAddPhone') + navigateAppend({name: 'settingsAddPhone', params: {}}) } const f = async () => { diff --git a/shared/settings/notifications/hooks.tsx b/shared/settings/notifications/hooks.tsx index f810cd173236..5e73a08d3a48 100644 --- a/shared/settings/notifications/hooks.tsx +++ b/shared/settings/notifications/hooks.tsx @@ -8,7 +8,7 @@ const useNotifications = (notificationSettings: UseNotificationSettingsResult) = const showEmailSection = useSettingsEmailState(s => s.emails.size > 0) const navigateAppend = C.Router2.navigateAppend const onClickYourAccount = () => { - navigateAppend(settingsAccountTab) + navigateAppend({name: settingsAccountTab, params: {}}) } return { diff --git a/shared/settings/root-desktop-tablet.tsx b/shared/settings/root-desktop-tablet.tsx index 9603bf4c48e7..ba077fe96950 100644 --- a/shared/settings/root-desktop-tablet.tsx +++ b/shared/settings/root-desktop-tablet.tsx @@ -9,6 +9,7 @@ import {useNavigationBuilder, TabRouter, createNavigatorFactory} from '@react-na import type {TypedNavigator, NavigatorTypeBagBase} from '@react-navigation/native' import {settingsDesktopTabRoutes} from './routes' import {settingsAccountTab} from '@/constants/settings' +import type {SettingsAccountRouteParams} from './routes' function LeftTabNavigator({ initialRouteName, @@ -72,7 +73,11 @@ const styles = Kb.Styles.styleSheetCreate(() => ({ })) type NavType = NavigatorTypeBagBase & { - ParamList: {[K in keyof typeof settingsDesktopTabRoutes]: undefined} + ParamList: { + [K in keyof typeof settingsDesktopTabRoutes]: K extends typeof settingsAccountTab + ? SettingsAccountRouteParams | undefined + : undefined + } } export const createLeftTabNavigator = createNavigatorFactory(LeftTabNavigator) as unknown as () => TypedNavigator diff --git a/shared/settings/root-phone.tsx b/shared/settings/root-phone.tsx index aefb55cf3fc7..223fadfc9111 100644 --- a/shared/settings/root-phone.tsx +++ b/shared/settings/root-phone.tsx @@ -71,7 +71,7 @@ function SettingsNav() { { icon: 'iconfont-nav-2-crypto', onClick: () => { - navigateAppend(Settings.settingsCryptoTab) + navigateAppend({name: Settings.settingsCryptoTab, params: {}}) }, text: 'Crypto', }, @@ -79,7 +79,7 @@ function SettingsNav() { badgeNumber: badgeNumbers.get(C.Tabs.devicesTab), icon: 'iconfont-nav-2-devices', onClick: () => { - navigateAppend(Settings.settingsDevicesTab) + navigateAppend({name: Settings.settingsDevicesTab, params: {}}) }, text: 'Devices', }, @@ -94,7 +94,7 @@ function SettingsNav() { { icon: 'iconfont-nav-2-wallets', onClick: () => { - navigateAppend(Settings.settingsWalletsTab) + navigateAppend({name: Settings.settingsWalletsTab, params: {}}) }, text: 'Wallet', }, @@ -106,31 +106,31 @@ function SettingsNav() { { badgeNumber: badgeNumbers.get(C.Tabs.settingsTab), onClick: () => { - navigateAppend(Settings.settingsAccountTab) + navigateAppend({name: Settings.settingsAccountTab, params: {}}) }, text: 'Account', }, { onClick: () => { - navigateAppend(Settings.settingsAdvancedTab) + navigateAppend({name: Settings.settingsAdvancedTab, params: {}}) }, text: 'Advanced', }, { onClick: () => { - navigateAppend(Settings.settingsArchiveTab) + navigateAppend({name: Settings.settingsArchiveTab, params: {}}) }, text: 'Backup', }, { onClick: () => { - navigateAppend(Settings.settingsChatTab) + navigateAppend({name: Settings.settingsChatTab, params: {}}) }, text: 'Chat', }, { onClick: () => { - navigateAppend(Settings.settingsDisplayTab) + navigateAppend({name: Settings.settingsDisplayTab, params: {}}) }, text: 'Display', }, @@ -142,20 +142,20 @@ function SettingsNav() { }, { onClick: () => { - navigateAppend(Settings.settingsFsTab) + navigateAppend({name: Settings.settingsFsTab, params: {}}) }, text: 'Files', }, { onClick: () => { - navigateAppend(Settings.settingsContactsTab) + navigateAppend({name: Settings.settingsContactsTab, params: {}}) }, text: contactsLabel, }, { badgeNumber: badgeNotifications ? 1 : 0, onClick: () => { - navigateAppend(Settings.settingsNotificationsTab) + navigateAppend({name: Settings.settingsNotificationsTab, params: {}}) }, text: 'Notifications', }, @@ -163,7 +163,7 @@ function SettingsNav() { ? [ { onClick: () => { - navigateAppend(Settings.settingsScreenprotectorTab) + navigateAppend({name: Settings.settingsScreenprotectorTab, params: {}}) }, text: 'Screen protector', } as const, @@ -176,13 +176,13 @@ function SettingsNav() { data: [ { onClick: () => { - navigateAppend(Settings.settingsAboutTab) + navigateAppend({name: Settings.settingsAboutTab, params: {}}) }, text: 'About', }, { onClick: () => { - navigateAppend(Settings.settingsLogOutTab) + navigateAppend({name: Settings.settingsLogOutTab, params: {}}) }, text: 'Sign out', textColor: Kb.Styles.globalColors.red, diff --git a/shared/settings/routes.tsx b/shared/settings/routes.tsx index b0d66a250bff..6a3182b8f533 100644 --- a/shared/settings/routes.tsx +++ b/shared/settings/routes.tsx @@ -11,6 +11,11 @@ import {usePWState} from '@/stores/settings-password' import {e164ToDisplay} from '@/util/phone-numbers' import type {Props as FeedbackRouteParams} from './feedback/container' +export type SettingsAccountRouteParams = { + addedEmailBannerEmail?: string + addedPhoneBanner?: boolean +} + const PushPromptSkipButton = () => { const rejectPermissions = usePushState(s => s.dispatch.rejectPermissions) const clearModals = C.Router2.clearModals diff --git a/shared/settings/use-delete-account.tsx b/shared/settings/use-delete-account.tsx index 72380cac3af3..ab4f43570210 100644 --- a/shared/settings/use-delete-account.tsx +++ b/shared/settings/use-delete-account.tsx @@ -24,7 +24,7 @@ export const useDeleteAccount = () => { () => { setJustDeletedSelf(username) clearModals() - navigateAppend(C.Tabs.loginTab) + navigateAppend({name: C.Tabs.loginTab, params: {}}) }, error => { logger.warn('Error deleting account', error) diff --git a/shared/settings/use-request-logout.tsx b/shared/settings/use-request-logout.tsx index 2fb5342ba60a..b0dc8e3971e2 100644 --- a/shared/settings/use-request-logout.tsx +++ b/shared/settings/use-request-logout.tsx @@ -1,5 +1,5 @@ import * as C from '@/constants' -import {navigateAppend} from '@/constants/router' +import {navigateAppend, switchTab} from '@/constants/router' import {settingsPasswordTab} from '@/constants/settings' import * as T from '@/constants/types' import {isMobile} from '@/constants/platform' @@ -8,10 +8,10 @@ import {useLogoutState} from '@/stores/logout' const navigateToLogoutPassword = () => { if (isMobile) { - navigateAppend(settingsPasswordTab) + navigateAppend({name: settingsPasswordTab, params: {}}) } else { - navigateAppend(Tabs.settingsTab) - navigateAppend(settingsPasswordTab) + switchTab(Tabs.settingsTab) + navigateAppend({name: settingsPasswordTab, params: {}}) } } diff --git a/shared/signup/common.tsx b/shared/signup/common.tsx index 67290c566fae..ba19dfd90fc0 100644 --- a/shared/signup/common.tsx +++ b/shared/signup/common.tsx @@ -17,7 +17,10 @@ export const InfoIcon = (props: InfoIconProps) => { const {attachTo, hidePopup} = p const onDocumentation = () => openURL('https://book.keybase.io/docs') const onFeedback = () => { - navigateAppend(loggedIn ? 'signupSendFeedbackLoggedIn' : 'signupSendFeedbackLoggedOut') + navigateAppend({ + name: loggedIn ? 'signupSendFeedbackLoggedIn' : 'signupSendFeedbackLoggedOut', + params: {}, + }) } return ( diff --git a/shared/signup/email.tsx b/shared/signup/email.tsx index 825594385820..5868fb85b75d 100644 --- a/shared/signup/email.tsx +++ b/shared/signup/email.tsx @@ -17,13 +17,13 @@ const ConnectedEnterEmail = () => { const onSkip = () => { _onSkip() - _showPushPrompt ? navigateAppend('settingsPushPrompt', true) : clearModals() + _showPushPrompt ? navigateAppend({name: 'settingsPushPrompt', params: {}}, true) : clearModals() } const onCreate = (email: string, searchable: boolean) => { submitEmail(email, searchable, addedEmail => { setSignupEmail(addedEmail) - _showPushPrompt ? navigateAppend('settingsPushPrompt', true) : clearModals() + _showPushPrompt ? navigateAppend({name: 'settingsPushPrompt', params: {}}, true) : clearModals() }) } diff --git a/shared/signup/phone-number/index.tsx b/shared/signup/phone-number/index.tsx index 15507bcce34f..23146ee1dc5c 100644 --- a/shared/signup/phone-number/index.tsx +++ b/shared/signup/phone-number/index.tsx @@ -76,7 +76,7 @@ const ConnectedEnterPhoneNumber = () => { const navigateAppend = C.Router2.navigateAppend const {clearError, error, submitPhoneNumber, waiting} = useAddPhoneNumber() const onSkip = () => { - navigateAppend('signupEnterEmail', true) + navigateAppend({name: 'signupEnterEmail', params: {}}, true) } const [phoneNumber, onChangePhoneNumber] = React.useState('') diff --git a/shared/signup/routes.tsx b/shared/signup/routes.tsx index c5cfe50fde68..ec96ab5558c2 100644 --- a/shared/signup/routes.tsx +++ b/shared/signup/routes.tsx @@ -15,7 +15,7 @@ const EmailSkipButton = () => { type="BodyBigLink" onClick={() => { setSignupEmail(C.noEmail) - showPushPrompt ? navigateAppend('settingsPushPrompt', true) : clearModals() + showPushPrompt ? navigateAppend({name: 'settingsPushPrompt', params: {}}, true) : clearModals() }} > Skip @@ -29,7 +29,7 @@ const PhoneSkipButton = () => { { - navigateAppend('signupEnterEmail', true) + navigateAppend({name: 'signupEnterEmail', params: {}}, true) }} > Skip diff --git a/shared/stores/fs.tsx b/shared/stores/fs.tsx index 0b088871f8bd..694331c6c7bb 100644 --- a/shared/stores/fs.tsx +++ b/shared/stores/fs.tsx @@ -823,7 +823,7 @@ export const useFSState = Z.createZustand('fs', (set, get) => { if (result === 'kextPermissionError' || result === 'kextPermissionErrorRetry') { dispatch.driverKextPermissionError() if (result === 'kextPermissionError') { - navigateAppend('kextPermission') + navigateAppend({name: 'kextPermission', params: {}}) } return } diff --git a/shared/stores/people.tsx b/shared/stores/people.tsx deleted file mode 100644 index 03be28e5626e..000000000000 --- a/shared/stores/people.tsx +++ /dev/null @@ -1,64 +0,0 @@ -import type * as EngineGen from '@/constants/rpc' -import {ignorePromise, RPCError, isNetworkErr} from '@/constants/utils' -import logger from '@/logger' -import * as T from '@/constants/types' -import * as Z from '@/util/zustand' - -type Store = T.Immutable<{ - refreshCount: number -}> - -const initialStore: Store = { - refreshCount: 0, -} - -export type State = Store & { - dispatch: { - markViewed: () => void - onEngineIncomingImpl: (action: EngineGen.Actions) => void - resetState: () => void - } -} - -export const usePeopleState = Z.createZustand('people', set => { - const dispatch: State['dispatch'] = { - markViewed: () => { - const f = async () => { - try { - await T.RPCGen.homeHomeMarkViewedRpcPromise() - } catch (error) { - if (!(error instanceof RPCError)) { - throw error - } - if (isNetworkErr(error.code)) { - logger.warn('Network error calling homeMarkViewed') - } else { - throw error - } - } - } - ignorePromise(f()) - }, - onEngineIncomingImpl: action => { - switch (action.type) { - case 'keybase.1.homeUI.homeUIRefresh': - set(s => { - s.refreshCount++ - }) - break - default: - } - }, - resetState: () => { - set(s => ({ - ...s, - ...initialStore, - })) - }, - } - - return { - ...initialStore, - dispatch, - } -}) diff --git a/shared/stores/provision.tsx b/shared/stores/provision.tsx index 7e8839abb12e..98dd449ad0ce 100644 --- a/shared/stores/provision.tsx +++ b/shared/stores/provision.tsx @@ -220,7 +220,7 @@ export const useProvisionState = Z.createZustand('provision', (set, get) response.result({phrase: good, secret: null as unknown as Uint8Array}) }) }) - navigateAppend('codePage') + navigateAppend({name: 'codePage', params: {}}) }, 'keybase.1.provisionUi.chooseDeviceType': (_params, response) => { const {type} = get().codePageOtherDevice @@ -330,7 +330,7 @@ export const useProvisionState = Z.createZustand('provision', (set, get) // we ignore the return as we never autosubmit, but we want things to increment shouldAutoSubmit(!!previousErr, {type: 'promptSecret'}) - navigateAppend('codePage') + navigateAppend({name: 'codePage', params: {}}) }, 'keybase.1.provisionUi.PromptNewDeviceName': (params, response) => { if (isCanceled(response)) return @@ -352,7 +352,7 @@ export const useProvisionState = Z.createZustand('provision', (set, get) console.log('Provision: auto submit device name') get().dispatch.dynamic.setDeviceName?.(get().deviceName) } else { - navigateAppend('setPublicName') + navigateAppend({name: 'setPublicName', params: {}}) } }, 'keybase.1.provisionUi.chooseDevice': (params, response) => { @@ -378,7 +378,7 @@ export const useProvisionState = Z.createZustand('provision', (set, get) console.log('Provision: auto submit passphrase') get().dispatch.dynamic.submitDeviceSelect?.(get().codePageOtherDevice.name) } else { - navigateAppend('selectOtherDevice') + navigateAppend({name: 'selectOtherDevice', params: {}}) } }, 'keybase.1.provisionUi.chooseGPGMethod': cancelOnCallback, @@ -408,10 +408,10 @@ export const useProvisionState = Z.createZustand('provision', (set, get) } else { switch (type) { case T.RPCGen.PassphraseType.passPhrase: - navigateAppend('password') + navigateAppend({name: 'password', params: {}}) break case T.RPCGen.PassphraseType.paperKey: - navigateAppend('paperkey') + navigateAppend({name: 'paperkey', params: {}}) break default: throw new Error('Got confused about password entry. Please send a log to us!') diff --git a/shared/stores/settings-contacts.native.tsx b/shared/stores/settings-contacts.native.tsx index e63aa7954cc4..005877825291 100644 --- a/shared/stores/settings-contacts.native.tsx +++ b/shared/stores/settings-contacts.native.tsx @@ -223,7 +223,7 @@ export const useSettingsContactsState = Z.createZustand('settings-contact s.waitingToShowJoinedModal = false }) if (resolved.length) { - navigateAppend('settingsContactsJoined') + navigateAppend({name: 'settingsContactsJoined', params: {}}) } } } catch (_error) { diff --git a/shared/stores/settings-email.tsx b/shared/stores/settings-email.tsx index e813c7350358..3a84d1768d29 100644 --- a/shared/stores/settings-email.tsx +++ b/shared/stores/settings-email.tsx @@ -15,12 +15,10 @@ type Writeable = {-readonly [P in keyof T]: T[P]} type EmailRow = Writeable type Store = T.Immutable<{ - addedEmail: string // show banner with dismiss on account settings emails: Map }> const initialStore: Store = { - addedEmail: '', emails: new Map(), } @@ -31,17 +29,16 @@ export type State = Store & { delete?: boolean makePrimary?: boolean makeSearchable?: boolean + onSuccess?: () => void verify?: boolean }) => void notifyEmailAddressEmailsChanged: (list: ReadonlyArray) => void notifyEmailVerified: (email: string) => void - resetAddedEmail: () => void resetState: () => void - setAddedEmail: (email: string) => void } } -export const useSettingsEmailState = Z.createZustand('settings-email', (set, get) => { +export const useSettingsEmailState = Z.createZustand('settings-email', set => { const dispatch: State['dispatch'] = { editEmail: p => { const f = async () => { @@ -49,10 +46,6 @@ export const useSettingsEmailState = Z.createZustand('settings-email', (s // TODO: handle errors if (p.delete) { await T.RPCGen.emailsDeleteEmailRpcPromise({email: p.email}) - if (get().addedEmail === p.email) { - get().dispatch.resetAddedEmail() - return - } return } if (p.makePrimary) { @@ -62,7 +55,6 @@ export const useSettingsEmailState = Z.createZustand('settings-email', (s if (p.verify) { await T.RPCGen.emailsSendVerificationEmailRpcPromise({email: p.email}) set(s => { - s.addedEmail = p.email const old = s.emails.get(p.email) ?? { ...makeEmailRow(), email: p.email, @@ -71,6 +63,8 @@ export const useSettingsEmailState = Z.createZustand('settings-email', (s old.lastVerifyEmailDate = new Date().getTime() / 1000 s.emails.set(p.email, old) }) + p.onSuccess?.() + return } if (p.makeSearchable !== undefined) { await T.RPCGen.emailsSetVisibilityEmailRpcPromise({ @@ -98,20 +92,9 @@ export const useSettingsEmailState = Z.createZustand('settings-email', (s } else { logger.warn('emailVerified on unknown email?') } - s.addedEmail = '' - }) - }, - resetAddedEmail: () => { - set(s => { - s.addedEmail = '' }) }, resetState: Z.defaultReset, - setAddedEmail: email => { - set(s => { - s.addedEmail = email - }) - }, } return { ...initialStore, diff --git a/shared/stores/settings-phone.tsx b/shared/stores/settings-phone.tsx index b827f931c5f4..aefe01d833d9 100644 --- a/shared/stores/settings-phone.tsx +++ b/shared/stores/settings-phone.tsx @@ -50,58 +50,29 @@ export type PhoneRow = { } type Store = T.Immutable<{ - addedPhone: boolean phones?: Map }> const initialStore: Store = { - addedPhone: false, phones: undefined, } export type State = Store & { dispatch: { - clearAddedPhone: () => void - editPhone: (phone: string, del?: boolean, setSearchable?: boolean) => void notifyPhoneNumberPhoneNumbersChanged: (list?: ReadonlyArray) => void resetState: () => void - setAddedPhone: (added: boolean) => void setNumbers: (phoneNumbers?: ReadonlyArray) => void } } export const useSettingsPhoneState = Z.createZustand('settings-phone', set => { const dispatch: State['dispatch'] = { - clearAddedPhone: () => { - set(s => { - s.addedPhone = false - }) - }, - editPhone: (phoneNumber, del, setSearchable) => { - const f = async () => { - if (del) { - await RPCGen.phoneNumbersDeletePhoneNumberRpcPromise({phoneNumber}) - } - if (setSearchable !== undefined) { - await RPCGen.phoneNumbersSetVisibilityPhoneNumberRpcPromise({ - phoneNumber, - visibility: setSearchable ? RPCGen.IdentityVisibility.public : RPCGen.IdentityVisibility.private, - }) - } - } - void f() - }, notifyPhoneNumberPhoneNumbersChanged: list => { set(s => { s.phones = new Map((list ?? []).map(row => [row.phoneNumber, toPhoneRow(row)])) }) }, resetState: Z.defaultReset, - setAddedPhone: added => { - set(s => { - s.addedPhone = added - }) - }, setNumbers: phoneNumbers => { set(s => { s.phones = phoneNumbers?.reduce((map, row) => { diff --git a/shared/stores/teams.tsx b/shared/stores/teams.tsx index 517493c7c504..9821205125b2 100644 --- a/shared/stores/teams.tsx +++ b/shared/stores/teams.tsx @@ -1064,7 +1064,7 @@ export const useTeamsState = Z.createZustand('teams', (set, get) => { } }) - navigateAppend('teamAddToTeamConfirm') + navigateAppend({name: 'teamAddToTeamConfirm', params: {}}) } ignorePromise(f()) }, @@ -1718,9 +1718,9 @@ export const useTeamsState = Z.createZustand('teams', (set, get) => { }) if (subteamOf) { - navigateAppend('teamWizard2TeamInfo') + navigateAppend({name: 'teamWizard2TeamInfo', params: {}}) } else { - navigateAppend('teamWizard1TeamPurpose') + navigateAppend({name: 'teamWizard1TeamPurpose', params: {}}) } }, leaveTeam: (teamname, permanent, context) => { @@ -2384,7 +2384,7 @@ export const useTeamsState = Z.createZustand('teams', (set, get) => { const parentTeamMeta = getTeamMeta(get(), parentTeamID ?? '') // If it's just you, don't show the subteam members screen empty if (parentTeamMeta.memberCount > 1) { - navigateAppend('teamWizardSubteamMembers') + navigateAppend({name: 'teamWizardSubteamMembers', params: {}}) return } else { get().dispatch.startAddMembersWizard(T.Teams.newTeamWizardTeamID) @@ -2396,10 +2396,10 @@ export const useTeamsState = Z.createZustand('teams', (set, get) => { get().dispatch.startAddMembersWizard(T.Teams.newTeamWizardTeamID) return case 'project': - navigateAppend('teamWizard5Channels') + navigateAppend({name: 'teamWizard5Channels', params: {}}) return case 'community': - navigateAppend('teamWizard4TeamSize') + navigateAppend({name: 'teamWizard4TeamSize', params: {}}) return } }, @@ -2407,7 +2407,7 @@ export const useTeamsState = Z.createZustand('teams', (set, get) => { set(s => { s.newTeamWizard.channels = channels }) - navigateAppend('teamWizard6Subteams') + navigateAppend({name: 'teamWizard6Subteams', params: {}}) }, setTeamWizardNameDescription: p => { set(s => { @@ -2431,7 +2431,7 @@ export const useTeamsState = Z.createZustand('teams', (set, get) => { teamID: T.Teams.newTeamWizardTeamID, }) }) - navigateAppend('teamAddToTeamConfirm') + navigateAppend({name: 'teamAddToTeamConfirm', params: {}}) }, setTeamWizardSubteams: subteams => { set(s => { @@ -2444,7 +2444,7 @@ export const useTeamsState = Z.createZustand('teams', (set, get) => { s.newTeamWizard.isBig = isBig }) if (isBig) { - navigateAppend('teamWizard5Channels') + navigateAppend({name: 'teamWizard5Channels', params: {}}) } else { get().dispatch.startAddMembersWizard(T.Teams.newTeamWizardTeamID) } @@ -2453,7 +2453,7 @@ export const useTeamsState = Z.createZustand('teams', (set, get) => { set(s => { s.newTeamWizard.teamType = teamType }) - navigateAppend('teamWizard2TeamInfo') + navigateAppend({name: 'teamWizard2TeamInfo', params: {}}) }, setTeamsWithChosenChannels: teamsWithChosenChannels => { set(s => { @@ -2526,7 +2526,7 @@ export const useTeamsState = Z.createZustand('teams', (set, get) => { set(s => { s.addMembersWizard = T.castDraft({...addMembersWizardEmptyState, teamID}) }) - navigateAppend('teamAddToTeamFromWhere') + navigateAppend({name: 'teamAddToTeamFromWhere', params: {}}) }, teamChangedByID: c => { const {changes, teamID, latestHiddenSeqno, latestOffchainSeqno, latestSeqno} = c diff --git a/shared/stores/tests/people.test.ts b/shared/stores/tests/people.test.ts deleted file mode 100644 index 2233f904ef68..000000000000 --- a/shared/stores/tests/people.test.ts +++ /dev/null @@ -1,39 +0,0 @@ -/// -import * as T from '@/constants/types' -import {resetAllStores} from '@/util/zustand' -import {usePeopleState} from '../people' - -beforeEach(() => { - resetAllStores() -}) - -afterEach(() => { - jest.restoreAllMocks() - resetAllStores() -}) - -test('markViewed forwards to the home mark-viewed RPC', async () => { - const markViewedSpy = jest.spyOn(T.RPCGen, 'homeHomeMarkViewedRpcPromise').mockResolvedValue(undefined) - - usePeopleState.getState().dispatch.markViewed() - await Promise.resolve() - - expect(markViewedSpy).toHaveBeenCalledTimes(1) -}) - -test('resetState clears the refresh counter', () => { - usePeopleState.setState({refreshCount: 2} as never) - - usePeopleState.getState().dispatch.resetState() - - expect(usePeopleState.getState().refreshCount).toBe(0) -}) - -test('homeUIRefresh increments the refresh counter', () => { - usePeopleState.getState().dispatch.onEngineIncomingImpl({ - payload: {params: {}}, - type: 'keybase.1.homeUI.homeUIRefresh', - } as never) - - expect(usePeopleState.getState().refreshCount).toBe(1) -}) diff --git a/shared/stores/tests/settings-email.test.ts b/shared/stores/tests/settings-email.test.ts index e55e067f989e..7945bbefa20e 100644 --- a/shared/stores/tests/settings-email.test.ts +++ b/shared/stores/tests/settings-email.test.ts @@ -23,15 +23,4 @@ test('email change notifications populate the email map and verification updates useSettingsEmailState.getState().dispatch.notifyEmailVerified('alice@example.com') expect(useSettingsEmailState.getState().emails.get('alice@example.com')?.isVerified).toBe(true) - expect(useSettingsEmailState.getState().addedEmail).toBe('') -}) - -test('setAddedEmail stages the banner state until reset', () => { - useSettingsEmailState.getState().dispatch.setAddedEmail('alice@example.com') - - expect(useSettingsEmailState.getState().addedEmail).toBe('alice@example.com') - - useSettingsEmailState.getState().dispatch.resetAddedEmail() - - expect(useSettingsEmailState.getState().addedEmail).toBe('') }) diff --git a/shared/stores/tests/settings-phone.test.ts b/shared/stores/tests/settings-phone.test.ts index 99bba7469374..3455a4e178e9 100644 --- a/shared/stores/tests/settings-phone.test.ts +++ b/shared/stores/tests/settings-phone.test.ts @@ -41,10 +41,17 @@ test('setNumbers keeps the first non-superseded row for a phone number', () => { expect(row?.verified).toBe(false) }) -test('setAddedPhone and clearAddedPhone only update the success banner state', () => { - useSettingsPhoneState.getState().dispatch.setAddedPhone(true) - expect(useSettingsPhoneState.getState().addedPhone).toBe(true) +test('resetState clears the cached phone numbers', () => { + useSettingsPhoneState.getState().dispatch.setNumbers([ + { + phoneNumber: '+15555550123', + superseded: false, + verified: false, + visibility: T.RPCGen.IdentityVisibility.private, + } as any, + ]) + + useSettingsPhoneState.getState().dispatch.resetState() - useSettingsPhoneState.getState().dispatch.clearAddedPhone() - expect(useSettingsPhoneState.getState().addedPhone).toBe(false) + expect(useSettingsPhoneState.getState().phones).toBeUndefined() }) diff --git a/shared/teams/add-members-wizard/add-from-where.tsx b/shared/teams/add-members-wizard/add-from-where.tsx index 01717569d547..0f91bce2c333 100644 --- a/shared/teams/add-members-wizard/add-from-where.tsx +++ b/shared/teams/add-members-wizard/add-from-where.tsx @@ -12,8 +12,8 @@ const AddFromWhere = () => { const createTeamError = Teams.useTeamsState(s => (newTeam ? s.newTeamWizard.error : undefined)) const appendNewTeamBuilder = C.Router2.appendNewTeamBuilder const onContinueKeybase = () => appendNewTeamBuilder(teamID) - const onContinuePhone = () => nav.safeNavigateAppend('teamAddToTeamPhone') - const onContinueContacts = () => nav.safeNavigateAppend('teamAddToTeamContacts') + const onContinuePhone = () => nav.safeNavigateAppend({name: 'teamAddToTeamPhone', params: {}}) + const onContinueContacts = () => nav.safeNavigateAppend({name: 'teamAddToTeamContacts', params: {}}) const onContinueEmail = () => nav.safeNavigateAppend({name: 'teamAddToTeamEmail', params: {}}) return ( diff --git a/shared/teams/add-members-wizard/confirm.tsx b/shared/teams/add-members-wizard/confirm.tsx index af76820f54fd..7e65fb6bb5ba 100644 --- a/shared/teams/add-members-wizard/confirm.tsx +++ b/shared/teams/add-members-wizard/confirm.tsx @@ -201,8 +201,8 @@ const AddMoreMembers = () => { const makePopup = (p: Kb.Popup2Parms) => { const {attachTo, hidePopup} = p const onAddKeybase = () => appendNewTeamBuilder(teamID) - const onAddContacts = () => nav.safeNavigateAppend('teamAddToTeamContacts') - const onAddPhone = () => nav.safeNavigateAppend('teamAddToTeamPhone') + const onAddContacts = () => nav.safeNavigateAppend({name: 'teamAddToTeamContacts', params: {}}) + const onAddPhone = () => nav.safeNavigateAppend({name: 'teamAddToTeamPhone', params: {}}) const onAddEmail = () => nav.safeNavigateAppend({name: 'teamAddToTeamEmail', params: {}}) return ( { + reload(false, true) +}) +``` + +and let `shared/constants/init/shared.tsx` remain the single engine entrypoint that fans out to both global stores and typed feature listeners. + Move it to route params if it is: - Data screen A already knows and screen B only needs for that navigation @@ -81,6 +105,7 @@ Prefer reloading in components instead of keeping a store cache when: - The data comes from the local service and reload latency is acceptable - The cache only saves a small RPC but forces unrelated screens to coordinate through global state - The notification path only exists to keep that convenience cache warm +- You do not have a concrete reason that the cache must survive navigation or serve multiple unrelated entry points Prefer direct store imports instead of `shared/constants/init/shared.tsx` callback plumbing when: @@ -130,6 +155,7 @@ Look for: - Components calling `dispatch.*` - Notification handlers keeping the store in sync - Navigation calls that could carry explicit params instead +- Engine actions that only poke one mounted feature through a store-owned `onEngineIncomingImpl` ### 2. Build a keep-or-move table @@ -147,8 +173,10 @@ Also label cross-store callback seams: - `keep-init-plumbing` - `replace-direct-import` +- `replace-engine-bridge` Use `replace-direct-import` when a `dispatch.defer.*` field only forwards to a leaf-like store and there is no import-cycle risk. +Use `replace-engine-bridge` when a store field or action exists only to bounce an engine event into one mounted feature. ### 3. Move screen-owned RPCs into components @@ -174,11 +202,13 @@ Keep waiting keys when they drive UI. If the store only existed to wrap that RPC Use `React.useState`, `React.useEffect`, and existing screen hooks. In plain `.tsx` files, use `Kb.*` components rather than raw DOM elements. +When the only remaining engine dependency is a mounted-screen reaction, subscribe in the component with the typed engine listener layer and keep navigation lifecycle in focus/blur hooks rather than `init/shared.tsx`. + If a helper hook, pure helper, or constant is only used by one component or one file, define it in that file instead of creating a sibling module. Split code out only when it is shared across files or the extracted boundary is meaningfully clearer than simple colocation. If a store action or utility candidate is only used by one component or one file, move the code directly into that caller instead of creating a new util or leaving an imperative `dispatch.*` method on the store. Only extract a shared util when multiple files need the same behavior. -When pruning imperative `dispatch.*` helpers, check the caller count first. A single-caller helper should usually be inlined into that caller. A helper with several unrelated callers can move to `shared/util/*` or a small file-local helper module if it still needs store access. +When pruning imperative `dispatch.*` helpers, check the caller count first. A single-caller helper should usually be inlined into that caller. Even with several callers in one feature, prefer colocated `C.useRPC` calls over introducing a wrapper hook unless the shared abstraction is pulling its weight. A helper with several unrelated callers can move to `shared/util/*` or a small file-local helper module if it still needs store access. If a component reads multiple adjacent values from the same remaining store, prefer one selector with `C.useShallow(...)` over several subscriptions.