Refactor AddEventModal state and improve useRouteQuery hook
Initialize AddEventModal state from editEvent props directly, removing the useEffect that synchronized state. Add AbortController to useRouteQuery to cancel in-flight requests when dependencies change or component unmounts. Add @testing-library/user-event and fix related tests to handle async user events. Polyfill localStorage in Vitest setup for jsdom environment.
This commit is contained in:
@@ -20,7 +20,7 @@
|
||||
"@react-navigation/native-stack": "^7.14.14",
|
||||
"@timetoleave/api-client": "*",
|
||||
"@timetoleave/core": "*",
|
||||
"expo": "~54.0.33",
|
||||
"expo": "~54.0.34",
|
||||
"expo-calendar": "~15.0.8",
|
||||
"expo-dev-client": "~6.0.21",
|
||||
"expo-location": "~19.0.8",
|
||||
|
||||
@@ -26,6 +26,7 @@
|
||||
"@testing-library/dom": "^10.4.1",
|
||||
"@testing-library/jest-dom": "^6.9.1",
|
||||
"@testing-library/react": "^16.3.2",
|
||||
"@testing-library/user-event": "^14.6.1",
|
||||
"@types/node": "^20",
|
||||
"@types/react": "~19.1.10",
|
||||
"@types/react-dom": "~19.1.10",
|
||||
|
||||
@@ -24,31 +24,19 @@ type AddEventModalProps = {
|
||||
|
||||
const AddEventModal: React.FC<AddEventModalProps> = ({ isOpen, onClose, editEvent, className = "" }) => {
|
||||
const isEditing = !!editEvent;
|
||||
const [title, setTitle] = useState("");
|
||||
const [destination, setDestination] = useState("");
|
||||
const [eventTime, setEventTime] = useState("");
|
||||
const [eventDate, setEventDate] = useState("");
|
||||
const [title, setTitle] = useState(() => editEvent?.title ?? "");
|
||||
const [destination, setDestination] = useState(() => editEvent?.destination ?? "");
|
||||
const [eventTime, setEventTime] = useState(
|
||||
() => editEvent ? format(editEvent.eventTime, "HH:mm") : "",
|
||||
);
|
||||
const [eventDate, setEventDate] = useState(
|
||||
() => editEvent ? format(editEvent.eventTime, "yyyy-MM-dd") : "",
|
||||
);
|
||||
const [loading, setLoading] = useState(false);
|
||||
const [success, setSuccess] = useState(false);
|
||||
|
||||
const { addEvent, updateEvent } = useEventsStore();
|
||||
|
||||
useEffect(() => {
|
||||
if (!isOpen) return;
|
||||
if (editEvent) {
|
||||
setTitle(editEvent.title);
|
||||
setDestination(editEvent.destination);
|
||||
setEventDate(format(editEvent.eventTime, "yyyy-MM-dd"));
|
||||
setEventTime(format(editEvent.eventTime, "HH:mm"));
|
||||
} else {
|
||||
setTitle("");
|
||||
setDestination("");
|
||||
setEventDate("");
|
||||
setEventTime("");
|
||||
}
|
||||
setSuccess(false);
|
||||
}, [editEvent, isOpen]);
|
||||
|
||||
const handleSubmit = async (e: React.FormEvent) => {
|
||||
e.preventDefault();
|
||||
if (!title.trim() || !destination.trim() || !eventTime.trim() || !eventDate.trim()) return;
|
||||
|
||||
@@ -166,7 +166,8 @@ describe("EventCard", () => {
|
||||
|
||||
// useJourneys should be called with origin extId as first arg
|
||||
// and the destination station's extId as second arg (NOT null)
|
||||
expect(useJourneys).toHaveBeenCalledWith("0WB0F0000600", "0WB0F0001500", new Date("2025-12-01T12:00:00"), 0);
|
||||
// trainStationArrivalTarget = eventTime - arrivalBufferMinutes (5 min)
|
||||
expect(useJourneys).toHaveBeenCalledWith("0WB0F0000600", "0WB0F0001500", new Date("2025-12-01T11:55:00"), 0, true);
|
||||
});
|
||||
|
||||
it("passes null as originStation extId to useJourneys when originStation is null", () => {
|
||||
@@ -181,6 +182,7 @@ describe("EventCard", () => {
|
||||
render(<EventCard event={mockEvent} originStation={null} />);
|
||||
|
||||
// When originStation is null, the first arg to useJourneys should be null
|
||||
expect(useJourneys).toHaveBeenCalledWith(null, "0WB0F0001500", new Date("2025-12-01T10:00:00"), 0);
|
||||
// trainStationArrivalTarget = eventTime - arrivalBufferMinutes (5 min)
|
||||
expect(useJourneys).toHaveBeenCalledWith(null, "0WB0F0001500", new Date("2025-12-01T09:55:00"), 0, true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
import "@testing-library/jest-dom";
|
||||
import React from "react";
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import JourneyList from "@/app/event/JourneyList";
|
||||
import { Journey } from "@timetoleave/core";
|
||||
|
||||
@@ -66,7 +67,7 @@ describe("JourneyList component", () => {
|
||||
expect(screen.getByText("R1")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should dim late journeys based on arrivalBufferMinutes", () => {
|
||||
it("should dim late journeys based on arrivalBufferMinutes", async () => {
|
||||
const lateJourney: Journey = {
|
||||
id: "1",
|
||||
sD: new Date("2025-01-01T14:00:00Z"),
|
||||
@@ -103,13 +104,16 @@ describe("JourneyList component", () => {
|
||||
/>
|
||||
);
|
||||
|
||||
// Expand to show all journeys
|
||||
await userEvent.click(screen.getByRole("button", { name: /show all/i }));
|
||||
|
||||
// Late journey should be dimmed and have line-through
|
||||
const lateJourneyElement = screen.getByText("R1").closest("li");
|
||||
const lateJourneyElement = screen.getByText("R1").closest("div");
|
||||
expect(lateJourneyElement).toHaveClass("opacity-40");
|
||||
expect(lateJourneyElement).toHaveClass("line-through");
|
||||
|
||||
// On-time journey should not be dimmed
|
||||
const onTimeJourneyElement = screen.getByText("R2").closest("li");
|
||||
const onTimeJourneyElement = screen.getByText("R2").closest("div");
|
||||
expect(onTimeJourneyElement).not.toHaveClass("opacity-40");
|
||||
expect(onTimeJourneyElement).not.toHaveClass("line-through");
|
||||
});
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useState, useEffect, useRef } from "react";
|
||||
import { useState, useEffect, useRef, useCallback } from "react";
|
||||
|
||||
type RouteFetcher<T> = (fromLat: number, fromLng: number, toLat: number, toLng: number) => Promise<T>;
|
||||
|
||||
@@ -22,11 +22,14 @@ export function useRouteQuery<T>(
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
|
||||
const fetcherRef = useRef(fetcher);
|
||||
fetcherRef.current = fetcher;
|
||||
|
||||
useEffect(() => {
|
||||
let isMounted = true;
|
||||
fetcherRef.current = fetcher;
|
||||
}, [fetcher]);
|
||||
|
||||
const abortControllerRef = useRef<AbortController | null>(null);
|
||||
|
||||
const fetchRoute = useCallback(async () => {
|
||||
if (fromLat == null || fromLng == null || toLat == null || toLng == null) {
|
||||
if (clearOnMissing) {
|
||||
setData(null);
|
||||
@@ -36,28 +39,33 @@ export function useRouteQuery<T>(
|
||||
return;
|
||||
}
|
||||
|
||||
setLoading(true);
|
||||
setError(null);
|
||||
// Cancel any in-flight request
|
||||
abortControllerRef.current?.abort();
|
||||
const controller = new AbortController();
|
||||
abortControllerRef.current = controller;
|
||||
|
||||
fetcherRef.current(fromLat, fromLng, toLat, toLng)
|
||||
.then((result) => {
|
||||
if (isMounted) {
|
||||
setData(result);
|
||||
setLoading(false);
|
||||
}
|
||||
})
|
||||
.catch((err: unknown) => {
|
||||
if (isMounted) {
|
||||
setError(err instanceof Error ? err.message : "Failed to fetch route");
|
||||
setLoading(false);
|
||||
}
|
||||
});
|
||||
|
||||
return () => {
|
||||
isMounted = false;
|
||||
};
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
try {
|
||||
setLoading(true);
|
||||
setError(null);
|
||||
const result = await fetcherRef.current(fromLat, fromLng, toLat, toLng);
|
||||
if (!controller.signal.aborted) {
|
||||
setData(result);
|
||||
setLoading(false);
|
||||
}
|
||||
} catch (err: unknown) {
|
||||
if (!controller.signal.aborted) {
|
||||
setError(err instanceof Error ? err.message : "Failed to fetch route");
|
||||
setLoading(false);
|
||||
}
|
||||
}
|
||||
}, [fromLat, fromLng, toLat, toLng, clearOnMissing]);
|
||||
|
||||
useEffect(() => {
|
||||
// Data fetching is the canonical effect use case per React docs
|
||||
// eslint-disable-next-line react-hooks/set-state-in-effect
|
||||
fetchRoute();
|
||||
return () => abortControllerRef.current?.abort();
|
||||
}, [fetchRoute]);
|
||||
|
||||
return { data, loading, error };
|
||||
}
|
||||
|
||||
@@ -1 +1,15 @@
|
||||
import "@testing-library/jest-dom/vitest";
|
||||
|
||||
// jsdom does not define localStorage by default in all Vitest configurations.
|
||||
Object.defineProperty(globalThis, "localStorage", {
|
||||
value: (() => {
|
||||
let store: Record<string, string> = {};
|
||||
return {
|
||||
getItem: (key: string) => store[key] ?? null,
|
||||
setItem: (key: string, value: string) => { store[key] = String(value); },
|
||||
removeItem: (key: string) => { delete store[key]; },
|
||||
clear: () => { store = {}; },
|
||||
};
|
||||
})(),
|
||||
writable: true,
|
||||
});
|
||||
|
||||
@@ -6,6 +6,11 @@ const config = {
|
||||
plugins: [react()],
|
||||
test: {
|
||||
environment: "jsdom",
|
||||
environmentOptions: {
|
||||
jsdom: {
|
||||
url: "http://localhost/",
|
||||
},
|
||||
},
|
||||
globals: true,
|
||||
setupFiles: ["./src/test/setup.ts"],
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user