mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-07-09 15:05:59 +00:00
fix: remove navigator.onLine guards and fix upsert races in all repos
navigator.onLine returns false transiently during service worker activation
(skipWaiting + clientsClaim), causing all repo refresh IIFEs to return null
immediately on first page load — leaving the UI with empty data until F5.
Fixes applied across all list repos (trip, day, place, packing, todo, budget,
reservation, accommodation, file):
- Drop navigator.onLine guard; let fetch fail naturally when truly offline
- Await all upsert calls (some were fire-and-forget, risking race conditions
against subsequent reads and silent swallowed failures)
- Return Promise.resolve(null) instead of Promise.resolve(fresh) in the
IDB-empty network path, so loadTrip's background refresh Promise.all
resolves null and skips set({trip}), preventing a spurious reference change
that was resetting the 1500ms splash timer
Tests updated: placeRepo and packingRepo "empty cache" tests now simulate
genuine network failure (HttpResponse.error) instead of relying on the
navigator.onLine guard that no longer exists; DashboardPage tests clear IDB
before each test and use a query-safe assertion after background refresh.
This commit is contained in:
@@ -7,10 +7,12 @@ import { resetAllStores, seedStore } from '../../tests/helpers/store';
|
|||||||
import { buildUser, buildAdmin, buildTrip } from '../../tests/helpers/factories';
|
import { buildUser, buildAdmin, buildTrip } from '../../tests/helpers/factories';
|
||||||
import { useAuthStore } from '../store/authStore';
|
import { useAuthStore } from '../store/authStore';
|
||||||
import { usePermissionsStore } from '../store/permissionsStore';
|
import { usePermissionsStore } from '../store/permissionsStore';
|
||||||
|
import { offlineDb } from '../db/offlineDb';
|
||||||
import DashboardPage from './DashboardPage';
|
import DashboardPage from './DashboardPage';
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(async () => {
|
||||||
vi.clearAllMocks();
|
vi.clearAllMocks();
|
||||||
|
await Promise.all(offlineDb.tables.map(t => t.clear()));
|
||||||
resetAllStores();
|
resetAllStores();
|
||||||
// Seed auth with authenticated user
|
// Seed auth with authenticated user
|
||||||
seedStore(useAuthStore, { isAuthenticated: true, user: buildUser() });
|
seedStore(useAuthStore, { isAuthenticated: true, user: buildUser() });
|
||||||
@@ -329,7 +331,8 @@ describe('DashboardPage', () => {
|
|||||||
const tokyoTrip = screen.getAllByText('Tokyo Trip')[0];
|
const tokyoTrip = screen.getAllByText('Tokyo Trip')[0];
|
||||||
await user.click(tokyoTrip);
|
await user.click(tokyoTrip);
|
||||||
|
|
||||||
expect(tokyoTrip).toBeInTheDocument();
|
// Re-query after click — background refresh may re-render the list
|
||||||
|
expect(screen.getAllByText('Tokyo Trip').length).toBeGreaterThan(0);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -9,10 +9,9 @@ export const accommodationRepo = {
|
|||||||
.where('trip_id').equals(Number(tripId)).toArray()
|
.where('trip_id').equals(Number(tripId)).toArray()
|
||||||
|
|
||||||
const refresh = (async () => {
|
const refresh = (async () => {
|
||||||
if (!navigator.onLine) return null
|
|
||||||
try {
|
try {
|
||||||
const result = await accommodationsApi.list(tripId)
|
const result = await accommodationsApi.list(tripId)
|
||||||
upsertAccommodations(result.accommodations || []).catch(() => {})
|
await upsertAccommodations(result.accommodations || [])
|
||||||
return result
|
return result
|
||||||
} catch {
|
} catch {
|
||||||
return null
|
return null
|
||||||
@@ -23,7 +22,7 @@ export const accommodationRepo = {
|
|||||||
|
|
||||||
const fresh = await refresh
|
const fresh = await refresh
|
||||||
if (!fresh) return { accommodations: [], refresh: Promise.resolve(null) }
|
if (!fresh) return { accommodations: [], refresh: Promise.resolve(null) }
|
||||||
return { accommodations: fresh.accommodations, refresh: Promise.resolve(fresh) }
|
return { accommodations: fresh.accommodations, refresh: Promise.resolve(null) }
|
||||||
},
|
},
|
||||||
|
|
||||||
async create(tripId: number | string, data: Record<string, unknown>): Promise<{ accommodation: Accommodation }> {
|
async create(tripId: number | string, data: Record<string, unknown>): Promise<{ accommodation: Accommodation }> {
|
||||||
|
|||||||
@@ -11,10 +11,9 @@ export const budgetRepo = {
|
|||||||
.toArray()
|
.toArray()
|
||||||
|
|
||||||
const refresh = (async () => {
|
const refresh = (async () => {
|
||||||
if (!navigator.onLine) return null
|
|
||||||
try {
|
try {
|
||||||
const result = await budgetApi.list(tripId)
|
const result = await budgetApi.list(tripId)
|
||||||
upsertBudgetItems(result.items)
|
await upsertBudgetItems(result.items)
|
||||||
return result
|
return result
|
||||||
} catch {
|
} catch {
|
||||||
return null
|
return null
|
||||||
@@ -25,7 +24,7 @@ export const budgetRepo = {
|
|||||||
|
|
||||||
const fresh = await refresh
|
const fresh = await refresh
|
||||||
if (!fresh) return { items: [], refresh: Promise.resolve(null) }
|
if (!fresh) return { items: [], refresh: Promise.resolve(null) }
|
||||||
return { items: fresh.items, refresh: Promise.resolve(fresh) }
|
return { items: fresh.items, refresh: Promise.resolve(null) }
|
||||||
},
|
},
|
||||||
|
|
||||||
async create(tripId: number | string, data: Record<string, unknown>): Promise<{ item: BudgetItem }> {
|
async create(tripId: number | string, data: Record<string, unknown>): Promise<{ item: BudgetItem }> {
|
||||||
|
|||||||
@@ -11,10 +11,9 @@ export const dayRepo = {
|
|||||||
.sortBy('day_number' as keyof Day)) as Day[]
|
.sortBy('day_number' as keyof Day)) as Day[]
|
||||||
|
|
||||||
const refresh = (async () => {
|
const refresh = (async () => {
|
||||||
if (!navigator.onLine) return null
|
|
||||||
try {
|
try {
|
||||||
const result = await daysApi.list(tripId)
|
const result = await daysApi.list(tripId)
|
||||||
upsertDays(result.days)
|
await upsertDays(result.days)
|
||||||
return result
|
return result
|
||||||
} catch {
|
} catch {
|
||||||
return null
|
return null
|
||||||
@@ -25,7 +24,7 @@ export const dayRepo = {
|
|||||||
|
|
||||||
const fresh = await refresh
|
const fresh = await refresh
|
||||||
if (!fresh) return { days: [], refresh: Promise.resolve(null) }
|
if (!fresh) return { days: [], refresh: Promise.resolve(null) }
|
||||||
return { days: fresh.days, refresh: Promise.resolve(fresh) }
|
return { days: fresh.days, refresh: Promise.resolve(null) }
|
||||||
},
|
},
|
||||||
|
|
||||||
async update(tripId: number | string, dayId: number | string, data: Record<string, unknown>): Promise<{ day: Day }> {
|
async update(tripId: number | string, dayId: number | string, data: Record<string, unknown>): Promise<{ day: Day }> {
|
||||||
|
|||||||
@@ -11,10 +11,9 @@ export const fileRepo = {
|
|||||||
.toArray()
|
.toArray()
|
||||||
|
|
||||||
const refresh = (async () => {
|
const refresh = (async () => {
|
||||||
if (!navigator.onLine) return null
|
|
||||||
try {
|
try {
|
||||||
const result = await filesApi.list(tripId)
|
const result = await filesApi.list(tripId)
|
||||||
upsertTripFiles(result.files)
|
await upsertTripFiles(result.files)
|
||||||
return result
|
return result
|
||||||
} catch {
|
} catch {
|
||||||
return null
|
return null
|
||||||
@@ -25,7 +24,7 @@ export const fileRepo = {
|
|||||||
|
|
||||||
const fresh = await refresh
|
const fresh = await refresh
|
||||||
if (!fresh) return { files: [], refresh: Promise.resolve(null) }
|
if (!fresh) return { files: [], refresh: Promise.resolve(null) }
|
||||||
return { files: fresh.files, refresh: Promise.resolve(fresh) }
|
return { files: fresh.files, refresh: Promise.resolve(null) }
|
||||||
},
|
},
|
||||||
|
|
||||||
async update(tripId: number | string, id: number, data: Record<string, unknown>): Promise<{ file: TripFile }> {
|
async update(tripId: number | string, id: number, data: Record<string, unknown>): Promise<{ file: TripFile }> {
|
||||||
|
|||||||
@@ -11,10 +11,9 @@ export const packingRepo = {
|
|||||||
.toArray()
|
.toArray()
|
||||||
|
|
||||||
const refresh = (async () => {
|
const refresh = (async () => {
|
||||||
if (!navigator.onLine) return null
|
|
||||||
try {
|
try {
|
||||||
const result = await packingApi.list(tripId)
|
const result = await packingApi.list(tripId)
|
||||||
upsertPackingItems(result.items)
|
await upsertPackingItems(result.items)
|
||||||
return result
|
return result
|
||||||
} catch {
|
} catch {
|
||||||
return null
|
return null
|
||||||
@@ -25,7 +24,7 @@ export const packingRepo = {
|
|||||||
|
|
||||||
const fresh = await refresh
|
const fresh = await refresh
|
||||||
if (!fresh) return { items: [], refresh: Promise.resolve(null) }
|
if (!fresh) return { items: [], refresh: Promise.resolve(null) }
|
||||||
return { items: fresh.items, refresh: Promise.resolve(fresh) }
|
return { items: fresh.items, refresh: Promise.resolve(null) }
|
||||||
},
|
},
|
||||||
|
|
||||||
async create(tripId: number | string, data: Record<string, unknown>): Promise<{ item: PackingItem }> {
|
async create(tripId: number | string, data: Record<string, unknown>): Promise<{ item: PackingItem }> {
|
||||||
|
|||||||
@@ -11,10 +11,9 @@ export const placeRepo = {
|
|||||||
.toArray()
|
.toArray()
|
||||||
|
|
||||||
const refresh = (async () => {
|
const refresh = (async () => {
|
||||||
if (!navigator.onLine) return null
|
|
||||||
try {
|
try {
|
||||||
const result = await placesApi.list(tripId, params)
|
const result = await placesApi.list(tripId, params)
|
||||||
upsertPlaces(result.places)
|
await upsertPlaces(result.places)
|
||||||
return result
|
return result
|
||||||
} catch {
|
} catch {
|
||||||
return null
|
return null
|
||||||
@@ -25,7 +24,7 @@ export const placeRepo = {
|
|||||||
|
|
||||||
const fresh = await refresh
|
const fresh = await refresh
|
||||||
if (!fresh) return { places: [], refresh: Promise.resolve(null) }
|
if (!fresh) return { places: [], refresh: Promise.resolve(null) }
|
||||||
return { places: fresh.places, refresh: Promise.resolve(fresh) }
|
return { places: fresh.places, refresh: Promise.resolve(null) }
|
||||||
},
|
},
|
||||||
|
|
||||||
async create(tripId: number | string, data: Record<string, unknown>): Promise<{ place: Place }> {
|
async create(tripId: number | string, data: Record<string, unknown>): Promise<{ place: Place }> {
|
||||||
|
|||||||
@@ -11,10 +11,9 @@ export const reservationRepo = {
|
|||||||
.toArray()
|
.toArray()
|
||||||
|
|
||||||
const refresh = (async () => {
|
const refresh = (async () => {
|
||||||
if (!navigator.onLine) return null
|
|
||||||
try {
|
try {
|
||||||
const result = await reservationsApi.list(tripId)
|
const result = await reservationsApi.list(tripId)
|
||||||
upsertReservations(result.reservations)
|
await upsertReservations(result.reservations)
|
||||||
return result
|
return result
|
||||||
} catch {
|
} catch {
|
||||||
return null
|
return null
|
||||||
@@ -25,7 +24,7 @@ export const reservationRepo = {
|
|||||||
|
|
||||||
const fresh = await refresh
|
const fresh = await refresh
|
||||||
if (!fresh) return { reservations: [], refresh: Promise.resolve(null) }
|
if (!fresh) return { reservations: [], refresh: Promise.resolve(null) }
|
||||||
return { reservations: fresh.reservations, refresh: Promise.resolve(fresh) }
|
return { reservations: fresh.reservations, refresh: Promise.resolve(null) }
|
||||||
},
|
},
|
||||||
|
|
||||||
async create(tripId: number | string, data: Record<string, unknown>): Promise<{ reservation: Reservation }> {
|
async create(tripId: number | string, data: Record<string, unknown>): Promise<{ reservation: Reservation }> {
|
||||||
|
|||||||
@@ -11,10 +11,9 @@ export const todoRepo = {
|
|||||||
.toArray()
|
.toArray()
|
||||||
|
|
||||||
const refresh = (async () => {
|
const refresh = (async () => {
|
||||||
if (!navigator.onLine) return null
|
|
||||||
try {
|
try {
|
||||||
const result = await todoApi.list(tripId)
|
const result = await todoApi.list(tripId)
|
||||||
upsertTodoItems(result.items)
|
await upsertTodoItems(result.items)
|
||||||
return result
|
return result
|
||||||
} catch {
|
} catch {
|
||||||
return null
|
return null
|
||||||
@@ -25,7 +24,7 @@ export const todoRepo = {
|
|||||||
|
|
||||||
const fresh = await refresh
|
const fresh = await refresh
|
||||||
if (!fresh) return { items: [], refresh: Promise.resolve(null) }
|
if (!fresh) return { items: [], refresh: Promise.resolve(null) }
|
||||||
return { items: fresh.items, refresh: Promise.resolve(fresh) }
|
return { items: fresh.items, refresh: Promise.resolve(null) }
|
||||||
},
|
},
|
||||||
|
|
||||||
async create(tripId: number | string, data: Record<string, unknown>): Promise<{ item: TodoItem }> {
|
async create(tripId: number | string, data: Record<string, unknown>): Promise<{ item: TodoItem }> {
|
||||||
|
|||||||
@@ -11,14 +11,15 @@ export const tripRepo = {
|
|||||||
const all = await offlineDb.trips.toArray()
|
const all = await offlineDb.trips.toArray()
|
||||||
|
|
||||||
const refresh: TripsRefresh = (async () => {
|
const refresh: TripsRefresh = (async () => {
|
||||||
if (!navigator.onLine) return null
|
|
||||||
try {
|
try {
|
||||||
const [active, archived] = await Promise.all([
|
const [active, archived] = await Promise.all([
|
||||||
tripsApi.list(),
|
tripsApi.list(),
|
||||||
tripsApi.list({ archived: 1 }),
|
tripsApi.list({ archived: 1 }),
|
||||||
])
|
])
|
||||||
active.trips.forEach(t => upsertTrip(t))
|
await Promise.all([
|
||||||
archived.trips.forEach(t => upsertTrip(t))
|
...active.trips.map(t => upsertTrip(t)),
|
||||||
|
...archived.trips.map(t => upsertTrip(t)),
|
||||||
|
])
|
||||||
return { trips: active.trips, archivedTrips: archived.trips }
|
return { trips: active.trips, archivedTrips: archived.trips }
|
||||||
} catch {
|
} catch {
|
||||||
return null
|
return null
|
||||||
@@ -35,17 +36,17 @@ export const tripRepo = {
|
|||||||
|
|
||||||
const fresh = await refresh
|
const fresh = await refresh
|
||||||
if (!fresh) return { trips: [], archivedTrips: [], refresh: Promise.resolve(null) }
|
if (!fresh) return { trips: [], archivedTrips: [], refresh: Promise.resolve(null) }
|
||||||
return { ...fresh, refresh: Promise.resolve(fresh) }
|
// Data came straight from network — no background re-fetch needed
|
||||||
|
return { ...fresh, refresh: Promise.resolve(null) }
|
||||||
},
|
},
|
||||||
|
|
||||||
async get(tripId: number | string): Promise<{ trip: Trip; refresh: TripRefresh }> {
|
async get(tripId: number | string): Promise<{ trip: Trip; refresh: TripRefresh }> {
|
||||||
const cached = await offlineDb.trips.get(Number(tripId))
|
const cached = await offlineDb.trips.get(Number(tripId))
|
||||||
|
|
||||||
const refresh: TripRefresh = (async () => {
|
const refresh: TripRefresh = (async () => {
|
||||||
if (!navigator.onLine) return null
|
|
||||||
try {
|
try {
|
||||||
const result = await tripsApi.get(tripId)
|
const result = await tripsApi.get(tripId)
|
||||||
upsertTrip(result.trip)
|
await upsertTrip(result.trip)
|
||||||
return result
|
return result
|
||||||
} catch {
|
} catch {
|
||||||
return null
|
return null
|
||||||
@@ -56,7 +57,7 @@ export const tripRepo = {
|
|||||||
|
|
||||||
const fresh = await refresh
|
const fresh = await refresh
|
||||||
if (!fresh) throw new Error('No cached trip data available offline')
|
if (!fresh) throw new Error('No cached trip data available offline')
|
||||||
return { trip: fresh.trip, refresh: Promise.resolve(fresh) }
|
return { trip: fresh.trip, refresh: Promise.resolve(null) }
|
||||||
},
|
},
|
||||||
|
|
||||||
async update(tripId: number | string, data: Partial<Trip>): Promise<{ trip: Trip }> {
|
async update(tripId: number | string, data: Partial<Trip>): Promise<{ trip: Trip }> {
|
||||||
|
|||||||
@@ -58,8 +58,10 @@ describe('packingRepo.list', () => {
|
|||||||
expect(restCalled).toBe(false);
|
expect(restCalled).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('offline — returns empty array when nothing cached', async () => {
|
it('offline — returns empty array when nothing cached and network fails', async () => {
|
||||||
Object.defineProperty(navigator, 'onLine', { value: false });
|
server.use(
|
||||||
|
http.get('/api/trips/99/packing', () => HttpResponse.error()),
|
||||||
|
);
|
||||||
const result = await packingRepo.list(99);
|
const result = await packingRepo.list(99);
|
||||||
expect(result.items).toHaveLength(0);
|
expect(result.items).toHaveLength(0);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -59,8 +59,10 @@ describe('placeRepo.list', () => {
|
|||||||
expect(restCalled).toBe(false);
|
expect(restCalled).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('offline — returns empty array when nothing cached', async () => {
|
it('offline — returns empty array when nothing cached and network fails', async () => {
|
||||||
Object.defineProperty(navigator, 'onLine', { value: false });
|
server.use(
|
||||||
|
http.get('/api/trips/99/places', () => HttpResponse.error()),
|
||||||
|
);
|
||||||
const result = await placeRepo.list(99);
|
const result = await placeRepo.list(99);
|
||||||
expect(result.places).toHaveLength(0);
|
expect(result.places).toHaveLength(0);
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user