Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/expo/app/(app)/_layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ export default function AppLayout() {
options={{
headerShown: false,
presentation: 'card',
animation: 'slide_from_bottom',
animation: 'default',
}}
/>
<Stack.Screen
Expand Down
2 changes: 1 addition & 1 deletion apps/expo/app/(app)/weather-alerts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export default function WeatherAlertsScreen() {
return (
<SafeAreaView className="flex-1" edges={['bottom']}>
<LargeTitleHeader title={t('weather.weatherAlertsTitle')} />
<ScrollView className="flex-1 mt-20" contentInsetAdjustmentBehavior="automatic">
<ScrollView className="flex-1" contentInsetAdjustmentBehavior="automatic">
<View className="flex-row items-center justify-between p-4">
<Text
variant="subhead"
Expand Down
70 changes: 26 additions & 44 deletions apps/expo/features/weather/components/WeatherAlertsTile.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
import type { AlertMethods } from '@packrat/ui/nativewindui';
import { Alert, ListItem, Text } from '@packrat/ui/nativewindui';
import { ListItem, Text } from '@packrat/ui/nativewindui';
import { Icon } from 'expo-app/components/Icon';
import { useColorScheme } from 'expo-app/lib/hooks/useColorScheme';
import { useTranslation } from 'expo-app/lib/hooks/useTranslation';
import { useRouter } from 'expo-router';
import { useRef } from 'react';
import { Platform, View } from 'react-native';
import { useWeatherAlerts } from '../hooks/useWeatherAlert';

export function WeatherAlertsTile() {
const router = useRouter();
const alertRef = useRef<AlertMethods>(null);
const { t } = useTranslation();

const { alerts, loading } = useWeatherAlerts();
Expand All @@ -22,47 +19,32 @@ export function WeatherAlertsTile() {
};

return (
<>
<ListItem
className="ios:pl-0 pl-2"
titleClassName="text-lg"
leftView={
<View className="px-3">
<View className="h-6 w-6 items-center justify-center rounded-md bg-amber-500">
<Icon name="weather-rainy" size={15} color="white" />
</View>
<ListItem
className="ios:pl-0 pl-2"
titleClassName="text-lg"
leftView={
<View className="px-3">
<View className="h-6 w-6 items-center justify-center rounded-md bg-amber-500">
<Icon name="weather-rainy" size={15} color="white" />
</View>
}
rightView={
<View className="flex-1 flex-row items-center justify-center gap-2 px-4">
<Text className="mr-2">
{loading ? '...' : t('weather.activeCount', { count: weatherAlertCount })}
</Text>
<ChevronRight />
</View>
}
item={{
title: t('weather.weatherAlerts'),
}}
onPress={handlePress}
target="Cell"
index={0}
removeSeparator={Platform.OS === 'ios'}
/>
<Alert
title={t('weather.noTripsYet')}
message={t('weather.createTripForAlerts')}
materialIcon={{ name: 'information-outline' }}
materialWidth={370}
buttons={[
{
text: t('weather.gotIt'),
style: 'default',
},
]}
ref={alertRef}
/>
</>
</View>
}
rightView={
<View className="flex-1 flex-row items-center justify-center gap-2 px-4">
<Text className="mr-2">
{loading ? '...' : t('weather.activeCount', { count: weatherAlertCount })}
</Text>
<ChevronRight />
</View>
}
item={{
title: t('weather.weatherAlerts'),
}}
onPress={handlePress}
target="Cell"
index={0}
removeSeparator={Platform.OS === 'ios'}
/>
);
}

Expand Down
34 changes: 29 additions & 5 deletions apps/expo/features/weather/hooks/useWeatherAlert.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as Sentry from '@sentry/react-native';
import { getWeatherData } from 'expo-app/features/weather/lib/weatherService';
import { useAtomValue } from 'jotai';
import { useEffect, useState } from 'react';
Expand Down Expand Up @@ -232,40 +233,63 @@ export function generateAlerts({
export function useWeatherAlerts() {
const activeLocation = useAtomValue(activeLocationAtom);

// Use primitive values as effect dependencies to avoid re-running on new
// object references emitted by the derived Jotai atom's .find() call.
const locationId = activeLocation?.id;
const locationName = activeLocation?.name;

const [alerts, setAlerts] = useState<WeatherAlert[]>([]);
const [loading, setLoading] = useState(true);
const [error, setError] = useState<string | null>(null);

useEffect(() => {
if (!activeLocation?.id) {
if (!locationId) {
setLoading(false);
return;
Comment on lines +246 to 248
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clear stale alerts when location is missing or fetch fails.

The hook can keep showing previous-location alerts when locationId becomes falsy or when a fetch errors.
That exposes incorrect alert data to users.

Suggested fix
   useEffect(() => {
     if (!locationId) {
+      setAlerts([]);
+      setError(null);
       setLoading(false);
       return;
     }
@@
       } catch (err) {
         if (cancelled) return;
         console.error('Weather alerts error:', err);
+        setAlerts([]);
         setError('Failed to fetch weather alerts');
         Sentry.captureException(err, {
           tags: { feature: 'weatherAlerts', action: 'fetchAlerts' },
           extra: { locationId, locationName },
         });

Also applies to: 275-281

🤖 Prompt for 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.

In `@apps/expo/features/weather/hooks/useWeatherAlert.ts` around lines 246 - 248,
When locationId becomes falsy or a fetch errors the hook currently leaves prior
alerts visible; update the early-return branch that checks locationId (and the
error branch around lines 275–281) to explicitly clear the alerts state before
returning — e.g., call the hook's alert-state setter to empty the list
(setAlerts([]) or setWeatherAlerts([]) depending on the existing state variable)
and then setLoading(false) and return, and do the same inside the fetch
catch/failure path so stale alerts are never shown for the wrong location.

}

const locationId = activeLocation.id;
let cancelled = false;

async function fetchAlerts() {
setLoading(true);
setError(null);

Sentry.addBreadcrumb({
category: 'weather',
message: 'Fetching weather alerts',
level: 'info',
data: { locationId, locationName },
});

try {
const data = await getWeatherData(locationId);
if (cancelled) return;

const formatted = generateAlerts({
// safe-cast: getWeatherData returns WeatherApiForecastResponse; WeatherApiData is a structural subset used only by this alert generator.
data: data as unknown as WeatherApiData,
activeLocation,
activeLocation: { name: locationName },
});
setAlerts(formatted);
} catch (err) {
if (cancelled) return;
console.error('Weather alerts error:', err);
setError('Failed to fetch weather alerts');
Sentry.captureException(err, {
tags: { feature: 'weatherAlerts', action: 'fetchAlerts' },
extra: { locationId, locationName },
});
} finally {
setLoading(false);
if (!cancelled) setLoading(false);
}
}

fetchAlerts();
}, [activeLocation]);

return () => {
cancelled = true;
};
}, [locationId, locationName]);

Comment on lines 241 to 293
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify this hook is using manual server-state management instead of TanStack Query.
fd -a "useWeatherAlert.ts" | xargs -I{} rg -n "useQuery|`@tanstack/react-query`|useEffect\\(|useState\\(|getWeatherData\\(" "{}"

Repository: PackRat-AI/PackRat

Length of output: 197


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the file and print the relevant section with line numbers
FILE="apps/expo/features/weather/hooks/useWeatherAlert.ts"
if [ ! -f "$FILE" ]; then
  echo "Missing: $FILE" >&2
  exit 1
fi

echo "=== Imports (first 120 lines) ==="
sed -n '1,120p' "$FILE" | nl -ba

echo "=== Hook implementation (220-320) ==="
sed -n '220,320p' "$FILE" | nl -ba

echo "=== Search within file for react-query usage ==="
rg -n "`@tanstack/react-query`|useQuery|useMutation|useInfiniteQuery" "$FILE" || true

echo "=== Search within file for manual fetch lifecycle ==="
rg -n "useEffect\\(|useState\\(|getWeatherData\\(|cancelled|setLoading\\(|setError\\(" "$FILE"

Repository: PackRat-AI/PackRat

Length of output: 158


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the file and print the relevant section with line numbers
FILE="apps/expo/features/weather/hooks/useWeatherAlert.ts"
if [ ! -f "$FILE" ]; then
  echo "Missing: $FILE" >&2
  exit 1
fi

echo "=== Imports (first 120 lines) ==="
sed -n '1,120p' "$FILE" | nl -ba

echo "=== Hook implementation (220-320) ==="
sed -n '220,320p' "$FILE" | nl -ba

echo "=== Search within file for react-query usage ==="
rg -n "`@tanstack/react-query`|useQuery|useMutation|useInfiniteQuery" "$FILE" || true

echo "=== Search within file for manual fetch lifecycle ==="
rg -n "useEffect\\(|useState\\(|getWeatherData\\(|cancelled|setLoading\\(|setError\\(" "$FILE"

Repository: PackRat-AI/PackRat

Length of output: 158


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="apps/expo/features/weather/hooks/useWeatherAlert.ts"
test -f "$FILE" || { echo "Missing: $FILE" >&2; exit 1; }

echo "=== Imports (first 140 lines) ==="
sed -n '1,140p' "$FILE" | cat -n

echo "=== Hook implementation (200-330) ==="
sed -n '200,330p' "$FILE" | cat -n

echo "=== react-query usage in file ==="
rg -n "`@tanstack/react-query`|useQuery|useMutation|useInfiniteQuery" "$FILE" || true

echo "=== manual lifecycle usage in file ==="
rg -n "useEffect\\(|useState\\(|getWeatherData\\(|cancelled|setLoading\\(|setError\\(" "$FILE"

Repository: PackRat-AI/PackRat

Length of output: 8565


Refactor useWeatherAlerts to use TanStack React Query for server-state lifecycle
In apps/expo/features/weather/hooks/useWeatherAlert.ts (lines 241-293), useWeatherAlerts manually manages server state with useState/useEffect + a cancelled flag while calling getWeatherData. Replace this with useQuery keyed by locationId (enabled only when locationId exists) and derive alerts, loading, and error from the query result (optionally selectgenerateAlerts(...)).

🤖 Prompt for 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.

In `@apps/expo/features/weather/hooks/useWeatherAlert.ts` around lines 241 - 293,
Refactor the hook useWeatherAlert to replace manual useState/useEffect logic
with TanStack React Query: create a useQuery keyed by locationId (only enabled
when locationId is truthy) that calls getWeatherData(locationId), use the
query's status/error/data to derive loading and error, and use the query's
select option (or map the data) to call generateAlerts({ data: data as
WeatherApiData, activeLocation: { name: locationName } }) so setAlerts is no
longer needed; ensure Sentry breadcrumb/exception reporting is invoked inside
the queryFn/ onError handlers and return alerts, isLoading, and error from the
hook.

return {
alerts,
Expand Down