From 9299f5aed980676a7ac673e22dbcb0ba04d144dd Mon Sep 17 00:00:00 2001 From: Jan-Henrik Bruhn Date: Sat, 27 Dec 2025 17:45:22 +0100 Subject: [PATCH] fix: Address Copilot review feedback on event subscriptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add error handling and documentation to event subscriptions based on Copilot review feedback. Changes: - Added try-catch blocks to all event callbacks for graceful error handling - Added comments documenting that subscriptions persist for app lifetime - Improved JSDoc for onPatternDeleted function with lifecycle details - Added error logging to help debug potential issues Benefits: - Prevents silent failures in event callbacks - Clear documentation about subscription lifecycle - Better developer experience with error messages 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 --- src/stores/storeEvents.ts | 14 +++++++++++--- src/stores/useMachineCacheStore.ts | 14 ++++++++++++-- src/stores/useMachineUploadStore.ts | 14 ++++++++++++-- src/stores/usePatternStore.ts | 14 ++++++++++++-- 4 files changed, 47 insertions(+), 9 deletions(-) diff --git a/src/stores/storeEvents.ts b/src/stores/storeEvents.ts index f4d93a2..862fb53 100644 --- a/src/stores/storeEvents.ts +++ b/src/stores/storeEvents.ts @@ -28,9 +28,17 @@ export const useEventStore = create((set) => ({ })); /** - * Subscribe to pattern deleted event - * @param callback - Function to call when event is emitted - * @returns Unsubscribe function + * Subscribe to the pattern deleted event. + * + * The subscription remains active until the returned unsubscribe function is called. + * If the unsubscribe function is not called, the listener will persist for the + * lifetime of the event store (typically the lifetime of the application). + * + * Call the returned unsubscribe function when the listener is no longer needed, + * especially for short-lived components or non-module-level subscriptions. + * + * @param callback - Function to call when the event is emitted. + * @returns Unsubscribe function that removes the listener when invoked. */ export const onPatternDeleted = (callback: () => void): (() => void) => { let prevCount = useEventStore.getState().patternDeletedCount; diff --git a/src/stores/useMachineCacheStore.ts b/src/stores/useMachineCacheStore.ts index 5023a2e..b18d3ed 100644 --- a/src/stores/useMachineCacheStore.ts +++ b/src/stores/useMachineCacheStore.ts @@ -194,7 +194,17 @@ export const useMachineCacheStore = create((set, get) => ({ }, })); -// Subscribe to pattern deleted event +// Subscribe to pattern deleted event. +// This subscription is intended to persist for the lifetime of the application, +// so the unsubscribe function returned by `onPatternDeleted` is intentionally +// not stored or called. onPatternDeleted(() => { - useMachineCacheStore.getState().clearResumeState(); + try { + useMachineCacheStore.getState().clearResumeState(); + } catch (error) { + console.error( + "[MachineCacheStore] Failed to clear resume state on pattern deleted event:", + error, + ); + } }); diff --git a/src/stores/useMachineUploadStore.ts b/src/stores/useMachineUploadStore.ts index 621200c..14c711f 100644 --- a/src/stores/useMachineUploadStore.ts +++ b/src/stores/useMachineUploadStore.ts @@ -128,7 +128,17 @@ export const useMachineUploadStore = create((set) => ({ }, })); -// Subscribe to pattern deleted event +// Subscribe to pattern deleted event. +// This subscription is intended to persist for the lifetime of the application, +// so the unsubscribe function returned by `onPatternDeleted` is intentionally +// not stored or called. onPatternDeleted(() => { - useMachineUploadStore.getState().reset(); + try { + useMachineUploadStore.getState().reset(); + } catch (error) { + console.error( + "[MachineUploadStore] Failed to reset on pattern deleted event:", + error, + ); + } }); diff --git a/src/stores/usePatternStore.ts b/src/stores/usePatternStore.ts index a05297b..610fe34 100644 --- a/src/stores/usePatternStore.ts +++ b/src/stores/usePatternStore.ts @@ -123,7 +123,17 @@ export const useUploadedPatternOffset = () => export const usePatternRotation = () => usePatternStore((state) => state.patternRotation); -// Subscribe to pattern deleted event +// Subscribe to pattern deleted event. +// This subscription is intended to persist for the lifetime of the application, +// so the unsubscribe function returned by `onPatternDeleted` is intentionally +// not stored or called. onPatternDeleted(() => { - usePatternStore.getState().clearUploadedPattern(); + try { + usePatternStore.getState().clearUploadedPattern(); + } catch (error) { + console.error( + "[PatternStore] Failed to clear uploaded pattern on pattern deleted event:", + error, + ); + } });