From 84e69bf6fdc0c5d6abfc0d5bc675e706a6f7d415 Mon Sep 17 00:00:00 2001 From: Dan Ditomaso Date: Sat, 25 Apr 2026 21:45:03 -0400 Subject: [PATCH] refactor(web): ResetNodeDb + RefreshKeys dialogs read SDK; safer adapter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ResetNodeDbDialog: connection.resetNodes() → meshClient.nodes.reset(); on success it now also calls meshClient.chat.clearAll() instead of the legacy useMessages().deleteAllMessages(). PKI-error tracking and the in-memory nodeDB still get cleared via removeAllNodeErrors / removeAllNodes since those subsystems have not yet migrated. Test rewritten to mock useActiveClient via vi.hoisted. - RefreshKeysDialog: drops useNodeDB().getNode in favour of useNodeAsProto for the missing-key node display. Test wraps render in a MeshRegistryProvider with an empty registry so the adapter resolves cleanly with no active client. Adapter hardening (useNodesAsProto.ts) - Switched off useNodes() / useMeshDevice() (which both throw outside a MeshProvider/MeshRegistryProvider with an active client) onto useActiveClient() + a no-op signal fallback. The hooks now return [] / undefined when there is no active client instead of throwing, which fixes RefreshKeysDialog's "no error → render null" path under tests that don't connect a device. Web tests: 295 still green; production Vite build clean. --- .../RefreshKeysDialog.test.tsx | 14 ++++- .../RefreshKeysDialog/RefreshKeysDialog.tsx | 6 +- .../ResetNodeDbDialog.test.tsx | 55 ++++++++----------- .../ResetNodeDbDialog/ResetNodeDbDialog.tsx | 22 +++++--- .../web/src/core/hooks/useNodesAsProto.ts | 44 ++++++++++++--- 5 files changed, 87 insertions(+), 54 deletions(-) diff --git a/packages/web/src/components/Dialog/RefreshKeysDialog/RefreshKeysDialog.test.tsx b/packages/web/src/components/Dialog/RefreshKeysDialog/RefreshKeysDialog.test.tsx index d9501fc6..fbfdec39 100644 --- a/packages/web/src/components/Dialog/RefreshKeysDialog/RefreshKeysDialog.test.tsx +++ b/packages/web/src/components/Dialog/RefreshKeysDialog/RefreshKeysDialog.test.tsx @@ -1,4 +1,6 @@ import { CurrentDeviceContext, useDeviceStore, useMessageStore } from "@core/stores"; +import { MeshRegistry } from "@meshtastic/sdk"; +import { MeshRegistryProvider } from "@meshtastic/sdk-react"; import { render } from "@testing-library/react"; import { afterEach, beforeEach, expect, test, vi } from "vitest"; import { RefreshKeysDialog } from "./RefreshKeysDialog.tsx"; @@ -43,10 +45,16 @@ test("does not render dialog if no error exists for active chat", () => { handleNodeRemove: vi.fn(), }); + // Empty MeshRegistry so the SDK adapter hooks (useNodeAsProto) do not + // throw when looking up the missing-key node — they return undefined. + const registry = new MeshRegistry(); + const { container } = render( - - - , + + + + + , ); expect(container.firstChild).toBeNull(); diff --git a/packages/web/src/components/Dialog/RefreshKeysDialog/RefreshKeysDialog.tsx b/packages/web/src/components/Dialog/RefreshKeysDialog/RefreshKeysDialog.tsx index 864b6379..1b24ac1b 100644 --- a/packages/web/src/components/Dialog/RefreshKeysDialog/RefreshKeysDialog.tsx +++ b/packages/web/src/components/Dialog/RefreshKeysDialog/RefreshKeysDialog.tsx @@ -6,6 +6,7 @@ import { DialogHeader, DialogTitle, } from "@components/UI/Dialog.tsx"; +import { useNodeAsProto } from "@core/hooks/useNodesAsProto.ts"; import { useMessages, useNodeDB } from "@core/stores"; import { LockKeyholeOpenIcon } from "lucide-react"; import { useTranslation } from "react-i18next"; @@ -19,18 +20,17 @@ export interface RefreshKeysDialogProps { export const RefreshKeysDialog = ({ open, onOpenChange }: RefreshKeysDialogProps) => { const { t } = useTranslation("dialog"); const { activeChat } = useMessages(); - const { nodeErrors, getNode } = useNodeDB(); + const { nodeErrors } = useNodeDB(); const { handleCloseDialog, handleNodeRemove } = useRefreshKeysDialog(); const nodeErrorNum = nodeErrors.get(activeChat); + const nodeWithError = useNodeAsProto(nodeErrorNum?.node ?? 0); if (!nodeErrorNum) { return null; } - const nodeWithError = getNode(nodeErrorNum.node); - const text = { title: t("refreshKeys.title", { interpolation: { escapeValue: false }, diff --git a/packages/web/src/components/Dialog/ResetNodeDbDialog/ResetNodeDbDialog.test.tsx b/packages/web/src/components/Dialog/ResetNodeDbDialog/ResetNodeDbDialog.test.tsx index 33b9b7ac..9c8a6db2 100644 --- a/packages/web/src/components/Dialog/ResetNodeDbDialog/ResetNodeDbDialog.test.tsx +++ b/packages/web/src/components/Dialog/ResetNodeDbDialog/ResetNodeDbDialog.test.tsx @@ -1,25 +1,24 @@ -// ResetNodeDbDialog.test.tsx import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { ResetNodeDbDialog } from "./ResetNodeDbDialog.tsx"; const mockResetNodes = vi.fn(); -const mockDeleteAllMessages = vi.fn(); +const mockClearAll = vi.fn(); const mockRemoveAllNodeErrors = vi.fn(); const mockRemoveAllNodes = vi.fn(); +const { mockUseActiveClient } = vi.hoisted(() => ({ + mockUseActiveClient: vi.fn(), +})); + +vi.mock("@meshtastic/sdk-react", () => ({ + useActiveClient: mockUseActiveClient, +})); + vi.mock("@core/stores", () => ({ CurrentDeviceContext: { _currentValue: { deviceId: 1234 }, }, - useDevice: () => ({ - connection: { - resetNodes: mockResetNodes, - }, - }), - useMessages: () => ({ - deleteAllMessages: mockDeleteAllMessages, - }), useNodeDB: () => ({ removeAllNodeErrors: mockRemoveAllNodeErrors, removeAllNodes: mockRemoveAllNodes, @@ -30,63 +29,55 @@ describe("ResetNodeDbDialog", () => { const mockOnOpenChange = vi.fn(); beforeEach(() => { - mockOnOpenChange.mockClear(); - mockResetNodes.mockClear(); - mockDeleteAllMessages.mockClear(); - mockRemoveAllNodeErrors.mockClear(); - mockRemoveAllNodes.mockClear(); + vi.clearAllMocks(); + mockUseActiveClient.mockReturnValue({ + nodes: { reset: mockResetNodes }, + chat: { clearAll: mockClearAll }, + }); }); - it("calls resetNodes, closes dialog, and after resolve clears messages and node DB (with true flag)", async () => { - // Control the promise returned by resetNodes - let resolveReset: (() => void) | undefined; + it("calls reset(), clears chat + legacy errors/nodes after resolve", async () => { + let resolveReset: ((value: { status: "ok"; value: number }) => void) | undefined; mockResetNodes.mockImplementation( () => - new Promise((resolve) => { + new Promise((resolve) => { resolveReset = resolve; }), ); + mockClearAll.mockResolvedValue(undefined); render(); fireEvent.click(screen.getByRole("button", { name: "Reset Node Database" })); - // Called immediately expect(mockResetNodes).toHaveBeenCalledTimes(1); - // DialogWrapper awaits onConfirm (which returns undefined), so close happens on next microtask await waitFor(() => { - expect(mockOnOpenChange).toHaveBeenCalledTimes(1); expect(mockOnOpenChange).toHaveBeenCalledWith(false); }); - // Nothing else should have happened yet (the promise hasn't resolved) - expect(mockDeleteAllMessages).not.toHaveBeenCalled(); + expect(mockClearAll).not.toHaveBeenCalled(); expect(mockRemoveAllNodeErrors).not.toHaveBeenCalled(); expect(mockRemoveAllNodes).not.toHaveBeenCalled(); - // Resolve the reset - resolveReset?.(); + resolveReset?.({ status: "ok", value: 1 }); - // Now the .then() chain should fire await waitFor(() => { - expect(mockDeleteAllMessages).toHaveBeenCalledTimes(1); + expect(mockClearAll).toHaveBeenCalledTimes(1); expect(mockRemoveAllNodeErrors).toHaveBeenCalledTimes(1); - expect(mockRemoveAllNodes).toHaveBeenCalledTimes(1); expect(mockRemoveAllNodes).toHaveBeenCalledWith(true); }); }); - it("calls onOpenChange(false) and does not call resetNodes when cancel is clicked", async () => { + it("does not call reset when cancel is clicked", async () => { render(); fireEvent.click(screen.getByRole("button", { name: "Cancel" })); await waitFor(() => { - expect(mockOnOpenChange).toHaveBeenCalledTimes(1); expect(mockOnOpenChange).toHaveBeenCalledWith(false); }); expect(mockResetNodes).not.toHaveBeenCalled(); - expect(mockDeleteAllMessages).not.toHaveBeenCalled(); + expect(mockClearAll).not.toHaveBeenCalled(); expect(mockRemoveAllNodeErrors).not.toHaveBeenCalled(); expect(mockRemoveAllNodes).not.toHaveBeenCalled(); }); diff --git a/packages/web/src/components/Dialog/ResetNodeDbDialog/ResetNodeDbDialog.tsx b/packages/web/src/components/Dialog/ResetNodeDbDialog/ResetNodeDbDialog.tsx index 9b274369..984661e5 100644 --- a/packages/web/src/components/Dialog/ResetNodeDbDialog/ResetNodeDbDialog.tsx +++ b/packages/web/src/components/Dialog/ResetNodeDbDialog/ResetNodeDbDialog.tsx @@ -1,5 +1,6 @@ import { toast } from "@core/hooks/useToast.ts"; -import { useDevice, useMessages, useNodeDB } from "@core/stores"; +import { useNodeDB } from "@core/stores"; +import { useActiveClient } from "@meshtastic/sdk-react"; import { useTranslation } from "react-i18next"; import { DialogWrapper } from "../DialogWrapper.tsx"; @@ -10,22 +11,25 @@ export interface ResetNodeDbDialogProps { export const ResetNodeDbDialog = ({ open, onOpenChange }: ResetNodeDbDialogProps) => { const { t } = useTranslation("dialog"); - const { connection } = useDevice(); + const meshClient = useActiveClient(); + // PKI-error tracking still lives on the legacy nodeDB store; clear it + // here until that subsystem is migrated to the SDK. const { removeAllNodeErrors, removeAllNodes } = useNodeDB(); - const { deleteAllMessages } = useMessages(); const handleResetNodeDb = () => { - connection - ?.resetNodes() + if (!meshClient) return; + meshClient.nodes + .reset() + .then((result) => { + if (result.status === "error") throw result.error; + return meshClient.chat.clearAll(); + }) .then(() => { - deleteAllMessages(); removeAllNodeErrors(); removeAllNodes(true); }) .catch((error) => { - toast({ - title: t("resetNodeDb.failedTitle"), - }); + toast({ title: t("resetNodeDb.failedTitle") }); console.error("Failed to reset Node DB:", error); }); }; diff --git a/packages/web/src/core/hooks/useNodesAsProto.ts b/packages/web/src/core/hooks/useNodesAsProto.ts index 46616fd5..7bb0523a 100644 --- a/packages/web/src/core/hooks/useNodesAsProto.ts +++ b/packages/web/src/core/hooks/useNodesAsProto.ts @@ -1,25 +1,55 @@ import { create } from "@bufbuild/protobuf"; import type { Node as SdkNode } from "@meshtastic/sdk"; import { Protobuf } from "@meshtastic/sdk"; -import { useMeshDevice, useNodes } from "@meshtastic/sdk-react"; +import { useActiveClient, useSignal } from "@meshtastic/sdk-react"; import { useMemo } from "react"; /** - * Adapter hooks that surface SDK-managed nodes in the legacy + * Adapter hooks that surface SDK-managed nodes in the * `Protobuf.Mesh.NodeInfo` shape consumed by web components today. * * Lets components migrate off the Zustand `useNodeDB().getNodes/getNode` * API one at a time without rewriting their templates. Removed once * every consumer reads `Node` from the SDK directly. + * + * Reads through `useActiveClient()` so that, when no client is active + * (e.g. inside isolated component tests or before any device connects), + * the hooks return safe empty values instead of throwing. */ +const EMPTY_NODES: ReadonlyArray = []; + +function useSdkNodesSafe(): ReadonlyArray { + const client = useActiveClient(); + // Always pass *something* to useSignal so the hook count stays stable + // across the with-client / without-client branches. + return useSignal( + client?.nodes.list ?? { + value: EMPTY_NODES, + peek: () => EMPTY_NODES, + subscribe: () => () => {}, + }, + ); +} + +function useMyNodeNumSafe(): number | undefined { + const client = useActiveClient(); + return useSignal( + client?.device.myNodeNum ?? { + value: undefined, + peek: () => undefined, + subscribe: () => () => {}, + }, + ); +} + export function useNodesAsProto(): Protobuf.Mesh.NodeInfo[] { - const nodes = useNodes(); + const nodes = useSdkNodesSafe(); return useMemo(() => nodes.map(toNodeInfo), [nodes]); } export function useNodeAsProto(nodeNum: number): Protobuf.Mesh.NodeInfo | undefined { - const nodes = useNodes(); + const nodes = useSdkNodesSafe(); return useMemo(() => { const found = nodes.find((n) => n.num === nodeNum); return found ? toNodeInfo(found) : undefined; @@ -29,11 +59,11 @@ export function useNodeAsProto(nodeNum: number): Protobuf.Mesh.NodeInfo | undefi /** * "My node" — the node info for the locally-connected device, if its * NodeInfo packet has been observed yet. Returns undefined while the - * device is still configuring. + * device is still configuring or there is no active client. */ export function useMyNodeAsProto(): Protobuf.Mesh.NodeInfo | undefined { - const { myNodeNum } = useMeshDevice(); - const nodes = useNodes(); + const myNodeNum = useMyNodeNumSafe(); + const nodes = useSdkNodesSafe(); return useMemo(() => { if (myNodeNum === undefined) return undefined; const found = nodes.find((n) => n.num === myNodeNum);