From 9c1f6ebf7e366d3d1a0419bfd7b671b0b1346d1d Mon Sep 17 00:00:00 2001 From: fegger Date: Mon, 18 May 2026 21:56:11 +0200 Subject: [PATCH] Fix React hook violations and optimize component behavior Update form state management in AddEventModal and improve GoogleTab OAuth handling Refactor EventListScreen to remove unused state calculation Add missing dependencies in JourneyList useMemo hooks Simplify events store initialization in EventsProvider --- apps/mobile/src/navigation/AppNavigator.tsx | 2 +- .../src/screens/CalendarImportScreen.tsx | 2 +- apps/mobile/src/screens/EventListScreen.tsx | 7 --- apps/web/src/app/add-event/AddEventModal.tsx | 10 ++- apps/web/src/app/calendar/GoogleTab.tsx | 63 ++++++++++--------- apps/web/src/app/event/JourneyList.tsx | 5 +- apps/web/src/hooks/useEventsStore.tsx | 6 +- 7 files changed, 48 insertions(+), 47 deletions(-) diff --git a/apps/mobile/src/navigation/AppNavigator.tsx b/apps/mobile/src/navigation/AppNavigator.tsx index 6058f7d..f0843b8 100644 --- a/apps/mobile/src/navigation/AppNavigator.tsx +++ b/apps/mobile/src/navigation/AppNavigator.tsx @@ -21,7 +21,7 @@ const byPrefixAndName = { fas: { bars: faBars } }; type HeaderMenuProps = { navigation: { - navigate: (screen: 'CalendarImport' | 'Settings') => void; + navigate: (_screen: 'CalendarImport' | 'Settings') => void; }; }; diff --git a/apps/mobile/src/screens/CalendarImportScreen.tsx b/apps/mobile/src/screens/CalendarImportScreen.tsx index 6308116..5b51ec3 100644 --- a/apps/mobile/src/screens/CalendarImportScreen.tsx +++ b/apps/mobile/src/screens/CalendarImportScreen.tsx @@ -61,7 +61,7 @@ function CalendarCheckbox({ }: { calendar: SelectableCalendar; selected: boolean; - onToggle: (id: string) => void; + onToggle: (_id: string) => void; colors: ReturnType; }) { const badgeColor = BADGE_COLORS[calendar.accountType] ?? BADGE_COLORS.other; diff --git a/apps/mobile/src/screens/EventListScreen.tsx b/apps/mobile/src/screens/EventListScreen.tsx index 2df02ca..a8a8d59 100644 --- a/apps/mobile/src/screens/EventListScreen.tsx +++ b/apps/mobile/src/screens/EventListScreen.tsx @@ -178,13 +178,6 @@ export function EventListScreen({ navigation }: ScreenProps) { const leaveCountdownLabel = leaveCountdown?.label; const leaveByLabel = leaveBy ? formatTime(leaveBy) : null; const trainLabel = selectedJourney?.trains.length ? selectedJourney.trains.join(', ') : 'Searching for train connection'; - const status = leaveCountdown - ? leaveCountdown.label === 'Now' - ? 'Leave now' - : `Leave in ${leaveCountdown.label}` - : journeysLoading || destStation.loading - ? 'Calculating departure time' - : 'No connection'; return ( diff --git a/apps/web/src/app/add-event/AddEventModal.tsx b/apps/web/src/app/add-event/AddEventModal.tsx index 1206697..a331135 100644 --- a/apps/web/src/app/add-event/AddEventModal.tsx +++ b/apps/web/src/app/add-event/AddEventModal.tsx @@ -33,8 +33,12 @@ const AddEventModal: React.FC = ({ isOpen, onClose, editEven const { addEvent, updateEvent } = useEventsStore(); - // Populate fields when opening in edit mode - useEffect(() => { + // Track previous props to reset form fields when they change (avoids setState in useEffect) + const [prevEditEvent, setPrevEditEvent] = useState(editEvent); + const [prevIsOpen, setPrevIsOpen] = useState(isOpen); + if (editEvent !== prevEditEvent || isOpen !== prevIsOpen) { + setPrevEditEvent(editEvent); + setPrevIsOpen(isOpen); if (editEvent) { setTitle(editEvent.title); setDestination(editEvent.destination); @@ -47,7 +51,7 @@ const AddEventModal: React.FC = ({ isOpen, onClose, editEven setEventTime(""); } setSuccess(false); - }, [editEvent, isOpen]); + } const handleSubmit = async (e: React.FormEvent) => { e.preventDefault(); diff --git a/apps/web/src/app/calendar/GoogleTab.tsx b/apps/web/src/app/calendar/GoogleTab.tsx index 23e6556..e654191 100644 --- a/apps/web/src/app/calendar/GoogleTab.tsx +++ b/apps/web/src/app/calendar/GoogleTab.tsx @@ -24,9 +24,28 @@ type GoogleTabProps = { }; const GoogleTab: React.FC = ({ onEventsLoaded, className = "" }) => { - const [status, setStatus] = useState("loading"); + // Read and clean OAuth return params synchronously at mount (avoids setState in effect) + const [oauthParams] = useState<{ error: string | null; connected: string | null }>(() => { + if (typeof window === "undefined") return { error: null, connected: null }; + const params = new URLSearchParams(window.location.search); + const error = params.get("google_error"); + const connected = params.get("google_connected"); + if (error || connected) { + const clean = new URL(window.location.href); + clean.searchParams.delete("google_connected"); + clean.searchParams.delete("google_error"); + window.history.replaceState(null, "", clean.toString()); + } + return { error, connected }; + }); + + const [status, setStatus] = useState( + oauthParams.error ? "not-connected" : "loading" + ); const [syncing, setSyncing] = useState(false); - const [error, setError] = useState(null); + const [error, setError] = useState( + oauthParams.error ? friendlyOAuthError(oauthParams.error) : null + ); const [syncedCount, setSyncedCount] = useState(null); const checkStatus = useCallback(async () => { @@ -45,33 +64,6 @@ const GoogleTab: React.FC = ({ onEventsLoaded, className = "" }) } }, []); - useEffect(() => { - // Handle OAuth return params - const params = new URLSearchParams(window.location.search); - const connected = params.get("google_connected"); - const oauthError = params.get("google_error"); - - if (connected || oauthError) { - const clean = new URL(window.location.href); - clean.searchParams.delete("google_connected"); - clean.searchParams.delete("google_error"); - window.history.replaceState(null, "", clean.toString()); - } - - if (oauthError) { - setError(friendlyOAuthError(oauthError)); - setStatus("not-connected"); - return; - } - - checkStatus().then(() => { - if (connected) { - syncEvents(); - } - }); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); - const syncEvents = useCallback(async () => { setSyncing(true); setError(null); @@ -98,6 +90,19 @@ const GoogleTab: React.FC = ({ onEventsLoaded, className = "" }) } }, [onEventsLoaded]); + useEffect(() => { + if (!oauthParams.error) { + // eslint-disable-next-line react-hooks/set-state-in-effect + checkStatus().then(() => { + if (oauthParams.connected) { + syncEvents(); + } + }); + } + // checkStatus and syncEvents are stable callbacks; oauthParams is a mount-time constant + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + const disconnect = async () => { await fetch("/api/auth/google/disconnect", { method: "POST" }); setStatus("not-connected"); diff --git a/apps/web/src/app/event/JourneyList.tsx b/apps/web/src/app/event/JourneyList.tsx index 89db160..9b19363 100644 --- a/apps/web/src/app/event/JourneyList.tsx +++ b/apps/web/src/app/event/JourneyList.tsx @@ -34,7 +34,10 @@ const JourneyList: React.FC = ({ className = "", }) => { const [expanded, setExpanded] = useState(false); - const targetArrivalTime = new Date(eventTime.getTime() - arrivalBufferMinutes * 60000); + const targetArrivalTime = useMemo( + () => new Date(eventTime.getTime() - arrivalBufferMinutes * 60000), + [eventTime, arrivalBufferMinutes], + ); const walkDurationMs = walkDurationSeconds * 1000; const rankedJourneys = useMemo( () => rankJourneys(journeys, targetArrivalTime, walkDurationMs), diff --git a/apps/web/src/hooks/useEventsStore.tsx b/apps/web/src/hooks/useEventsStore.tsx index 0d56cf0..2d6e377 100644 --- a/apps/web/src/hooks/useEventsStore.tsx +++ b/apps/web/src/hooks/useEventsStore.tsx @@ -45,13 +45,9 @@ const EventsContext = createContext(undefined); * Persists events to localStorage on every change (skips initial hydration write). */ export function EventsProvider({ children }: { children: ReactNode }) { - const [events, setEventsState] = useState([]); + const [events, setEventsState] = useState(loadFromStorage); const skipInitialWrite = useRef(true); - useEffect(() => { - setEventsState(loadFromStorage()); - }, []); - useEffect(() => { if (skipInitialWrite.current) { skipInitialWrite.current = false;