From e0a8a3ce75d6808fbf4913e352c8989617005b2b Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Wed, 20 Nov 2024 06:32:12 +0000 Subject: [PATCH] Hoist check to top of effect in `useUnsavedChanges` Per PR feedback [1], simplify control flow in effect body by using an early return. [1] https://github.com/hypothesis/h/pull/9098#discussion_r1848348464 --- .../group-forms/utils/unsaved-changes.ts | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/h/static/scripts/group-forms/utils/unsaved-changes.ts b/h/static/scripts/group-forms/utils/unsaved-changes.ts index f2b4f994a74..6b14efa91a7 100644 --- a/h/static/scripts/group-forms/utils/unsaved-changes.ts +++ b/h/static/scripts/group-forms/utils/unsaved-changes.ts @@ -23,21 +23,24 @@ export function hasUnsavedChanges() { * @param hasUnsavedChanges - True if current component has unsaved changes * @param window_ - Test seam */ -export function useUnsavedChanges(hasUnsavedData: boolean, window_ = window) { +export function useUnsavedChanges( + hasUnsavedChanges: boolean, + window_ = window, +) { useEffect(() => { - if (hasUnsavedData) { - unsavedCount += 1; - if (unsavedCount === 1) { - window_.addEventListener('beforeunload', preventUnload); - } + if (!hasUnsavedChanges) { + return () => {}; + } + + unsavedCount += 1; + if (unsavedCount === 1) { + window_.addEventListener('beforeunload', preventUnload); } return () => { - if (hasUnsavedData) { - unsavedCount -= 1; - if (unsavedCount === 0) { - window_.removeEventListener('beforeunload', preventUnload); - } + unsavedCount -= 1; + if (unsavedCount === 0) { + window_.removeEventListener('beforeunload', preventUnload); } }; - }, [hasUnsavedData, window_]); + }, [hasUnsavedChanges, window_]); }