fix: module config save crash, missing sendKeyVerification, and all 43 tsc errors in apps/web#1239
Open
Jarod1230 wants to merge 8 commits into
Open
fix: module config save crash, missing sendKeyVerification, and all 43 tsc errors in apps/web#1239Jarod1230 wants to merge 8 commits into
Jarod1230 wants to merge 8 commits into
Conversation
…/removeChange store API RemoteHardware, StatusMessage, Tak and TrafficManagement still destructured setChange/removeChange from useDevice(), which no longer exist on the device store — saving any of these module configs threw a TypeError at runtime. Migrate them to the useConfigEditor/setModuleSection pattern already used by the other module config pages (e.g. Paxcounter, MQTT). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
NodeDetailsDialog and ClientNotificationDialog call connection.sendKeyVerification(), but the method was lost when the legacy MeshDevice facade was rebuilt on top of MeshClient — initiating or answering a key verification crashed with a TypeError. Reintroduce it as a thin wrapper that sends the keyVerification admin message, matching the old @meshtastic/core signature. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
better-result 2.9.2 exports the Result union type, not ResultType — every type-only import of ResultType failed to resolve, which cascaded into 'result.error is of type unknown' errors at the transport callsites. Rename the imports across sdk, sdk-react, transport-web-serial and sdk-preview, and retype the sdk-preview slice against @meshtastic/sdk (the shim MeshDevice the app actually passes in) instead of the stale JSR @meshtastic/core. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The app already resolves @meshtastic/protobufs to the workspace package (root package.json), but @meshtastic/sdk still pulled the JSR release — two incompatible protobuf type trees coexisted, breaking assignability wherever both met (LoRa serialHalOnly, sqlocal User, sdk-preview events). Point the sdk at the workspace package, finish the last two @meshtastic/core imports in the app, and add the new hasXeddsaSigned field to the NodeInfo test fixture. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
react-error-boundary types the caught value as unknown; ErrorPage assumed Error, so App/PageLayout FallbackComponent props didn't typecheck and a thrown non-Error would render nothing useful. Normalize before rendering. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pnpm format was failing on main for 12 files (e2e helpers, playwright config and a few sources); this is the oxfmt --fix output with no manual edits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR migrates four module config components to signal-based editing, replaces ChangesConfiguration and SDK updates
Runtime fixes and release maintenance
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Component as Module Config Component
participant Editor as useConfigEditor
participant Signal as useSignal
participant Form as DynamicForm
Component->>Editor: useConfigEditor()
Component->>Signal: Read editor modules
Signal-->>Component: Effective module section
Component->>Form: Provide effective values
Form-->>Component: Submit configuration
Component->>Editor: setModuleSection(section, data)
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/PageComponents/ModuleConfig/RemoteHardware.tsx`:
- Around line 36-48: `RemoteHardware` can render without a config editor because
`useWaitForConfig()` only guarantees module config, so the form currently
accepts edits that are never persisted. Update the `RemoteHardware` component to
guard against a missing `useConfigEditor()` result by disabling the form
controls or rendering an explicit error/empty state until `editor` exists. Make
sure the submit path in `onSubmit` and any editable inputs are blocked when
`editor` is undefined, so no local-only changes can be made.
In `@packages/sdk/package.json`:
- Line 51: The release flow for packages/sdk is using raw npm publish, so the
`@meshtastic/protobufs` workspace:* dependency may be published without being
rewritten. Update the release workflow to use pnpm publish or pnpm pack for the
sdk package, or add a publish-time rewrite step before publishing. Make sure the
fix is applied in the publishing path that handles packages/sdk and its
dependency declarations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 578cb98d-c666-443c-9042-ab2d4f32c879
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (53)
apps/web/src/components/Dialog/ClientNotificationDialog/ClientNotificationDialog.tsxapps/web/src/components/PageComponents/ModuleConfig/RemoteHardware.tsxapps/web/src/components/PageComponents/ModuleConfig/StatusMessage.tsxapps/web/src/components/PageComponents/ModuleConfig/Tak.tsxapps/web/src/components/PageComponents/ModuleConfig/TrafficManagement.tsxapps/web/src/components/UI/ErrorPage.tsxapps/web/src/components/generic/Filter/useFilterNode.test.tsapps/web/src/pages/Connections/useConnections.tsapps/web/src/sdk-preview/adapters/fromMeshDevice.tsapps/web/src/sdk-preview/core/client/MeshClientPort.tsapps/web/src/sdk-preview/features/config/__tests__/ConfigEditor.test.tsapps/web/src/sdk-preview/features/config/application/ConfigUseCases.tsapps/web/src/sdk-preview/features/config/domain/ConfigEditor.tsapps/web/src/sdk-preview/features/config/domain/ModuleConfig.tsapps/web/src/sdk-preview/features/config/domain/RadioConfig.tsapps/web/src/sdk-preview/features/config/infrastructure/ConfigMapper.tsapps/web/src/sdk-preview/features/config/infrastructure/configBuilders.tsapps/web/src/validation/moduleConfig/tak.tse2e/fixtures/peer.tse2e/global-setup.tse2e/global-teardown.tse2e/pages/ConnectionPage.tse2e/pages/MessagesPage.tse2e/tests/messaging.broadcast.spec.tse2e/tests/messaging.direct.spec.tspackages/sdk-react/src/hooks/useChat.tspackages/sdk-react/src/hooks/useDirectChat.tspackages/sdk-react/src/hooks/useFavoriteNode.tspackages/sdk-react/src/hooks/useIgnoreNode.tspackages/sdk/package.jsonpackages/sdk/src/features/channels/ChannelsClient.tspackages/sdk/src/features/channels/application/ChannelUseCases.tspackages/sdk/src/features/chat/ChatClient.send.test.tspackages/sdk/src/features/chat/ChatClient.tspackages/sdk/src/features/chat/application/SendTextUseCase.tspackages/sdk/src/features/chat/application/SendWaypointUseCase.tspackages/sdk/src/features/config/ConfigClient.tspackages/sdk/src/features/config/application/ConfigUseCases.tspackages/sdk/src/features/config/domain/ConfigEditor.tspackages/sdk/src/features/files/FilesClient.tspackages/sdk/src/features/nodes/NodesClient.test.tspackages/sdk/src/features/nodes/NodesClient.tspackages/sdk/src/features/nodes/application/FavoriteNodeUseCase.tspackages/sdk/src/features/nodes/application/IgnoreNodeUseCase.tspackages/sdk/src/features/nodes/application/RemoveNodeUseCase.tspackages/sdk/src/features/nodes/application/SetOwnerUseCase.tspackages/sdk/src/features/position/PositionClient.tspackages/sdk/src/features/position/application/PositionUseCases.tspackages/sdk/src/features/traceroute/TraceRouteClient.tspackages/sdk/src/features/traceroute/application/TraceRouteUseCase.tspackages/sdk/src/shim/legacyMeshDevice.tspackages/transport-web-serial/src/transport.tsplaywright.config.ts
|
@Jarod1230 is attempting to deploy a commit to the Meshtastic Team on Vercel. A member of the Team first needs to authorize it. |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes two runtime crashes and clears all 43
tsc --noEmiterrors inapps/web(the PR CI does not run a full typecheck, so these had accumulated silently):RemoteHardware,StatusMessage,TakandTrafficManagementstill destructuredsetChange/removeChangefromuseDevice(), which no longer exist on the device store — submitting any of these forms threw aTypeError. They are now migrated to theuseConfigEditor/setModuleSectionpattern already used by the other module config pages (Paxcounter, MQTT, …).NodeDetailsDialogandClientNotificationDialogcallconnection.sendKeyVerification(), but the method was lost when the legacyMeshDevicefacade was rebuilt on top ofMeshClient. It is reintroduced as a thin wrapper sending thekeyVerificationadmin message, matching the old@meshtastic/coresignature.ResultTypedoes not exist in better-result 2.9.2 (the exported type isResult) — every type-only import of it failed to resolve, cascading intoresult.error is of type unknownerrors at the serial transport callsites. Renamed acrosssdk,sdk-react,transport-web-serialandsdk-preview.@meshtastic/sdkstill pulled the JSR@meshtastic/protobufsrelease while the root already resolves it to the workspace package — breaking assignability wherever both met (LoRaserialHalOnly, sqlocalUser,sdk-previewevent types). The sdk now consumesworkspace:*, and the last two@meshtastic/coreimports in the app are switched to@meshtastic/sdk.ErrorPageassumedError, but react-error-boundary types the caught value asunknown; non-Error throws are now normalized before rendering.The last commit is
oxfmtoutput for 12 files on whichpnpm formatwas already failing onmain(no manual edits).Changes Made
useConfigEditor(fixes save crash)sendKeyVerification()to the legacyMeshDeviceshim (fixes key verification crash)ResultType→Resultimports from better-result (34 files, type-only)packages/sdk: depend on@meshtastic/protobufsviaworkspace:*instead of JSR 2.7.20; add newhasXeddsaSignedfield to theNodeInfotest fixtureErrorPage: acceptunknownthrown valuesoxfmtformatting for files that were failingpnpm formatonmainTesting Done
pnpm test -- --run: 388/388 tests pass (69 files)cd apps/web && npx tsc --noEmit: 0 errors (was 43 onmain)pnpm check(oxlint + oxfmt): passes; oxlint warnings unchanged frommainapps/webvite production build: greenChecklist
🤖 Generated with Claude Code using the Claude Fable 5 model
Summary by CodeRabbit
Errorinstances).