Fixing Path Between Destinations#202
Conversation
|
Great job! All CI checks are green and the logic looks solid. Same situation as #201 — there's a merge conflict, so could you rebase onto main and force-push? Happy to merge once that's resolved. |
Remove duplicate translate on Advanced Marker pin content (default anchor is already bottom-center) and always set scaledSize plus anchor on legacy Marker icons so stop pins stay pinned to their geographic coordinates when zooming. Co-authored-by: Cursor <cursoragent@cursor.com>
Load routes only from sessionStorage after optimize; show an error overlay when no stored data or corrupt JSON instead of silently rendering mock routes. Co-authored-by: Cursor <cursoragent@cursor.com>
Use callback-based directions requests with status checks, extract road paths from overview or leg steps, fetch routes sequentially to avoid rate limits, stop overwriting cached road paths with straight lines, and document Directions API requirement in .env.example. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
d574153 to
6d1615f
Compare
markboenigk
left a comment
There was a problem hiding this comment.
Good refactor overall — the sequential fetch, extractRoadPath fallback, and marker anchor fix are all solid improvements. A few issues worth addressing before merge:
1. Cache-miss after pin-commit leaves polyline stale (regression)
The PR removes the else { poly.setPath(committed); } branch from the pin-move sync effect. When a user commits a pin drag and the Directions cache is cold for the new coordinates, nothing sets the polyline path — it stays at whatever shape it had from the drag preview until the async fetch finishes. The straight-line snap was the intended intermediate state. Suggest keeping it:
if (cached && cached.path.length >= 2) {
poly.setPath(cached.path);
} else {
poly.setPath(committed); // straight-line until async fetch fills the cache
}2. Hydration mismatch: server renders no error modal, client does
readInitialRoutes now returns {error: "No optimized routes found…"} when sessionStorage is empty. On the server the typeof window === "undefined" guard fires first and returns {error: null}. Next.js hydrates the server HTML (no modal) against the client's initial state (modal present), producing a hydration mismatch. One fix is to show the error only after a useEffect confirms we're on the client with no data — same pattern as the ?mock=1 branch.
3. Blank-map flash when savePendingPinMove fires
savePendingPinMove batches setRoutes + setPendingPinMove(null) into one render, so both effects fire together. Effect 1 (directions) clears polylinesByVehicleRef.current = {} synchronously before launching its async IIFE. Effect 2 (pin-move sync) then reads the now-empty ref, finds no polylines, and silently skips the snap-back. The map shows nothing until the async fetch resolves. This was probably unintentional — the effect ordering interplay here is subtle.
4. Minor: extractRoadPath step fallback produces duplicate vertices
Adjacent steps share an endpoint (step[N].path[-1] === step[N+1].path[0]), so concatenating them raw duplicates the boundary point at every step join. Easy fix: skip the first point of each subsequent step's path.
for (const [i, step] of (leg.steps ?? []).entries()) {
if (step.path?.length) {
path.push(...(i === 0 ? step.path : step.path.slice(1)));
}
}…smatch Restore straight-line polyline fallback on directions cache miss after pin commit, recreate polylines when the directions effect clears refs first, dedupe step-path vertices in extractRoadPath, and defer missing-data errors to a client-only useEffect to prevent SSR hydration mismatch. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the client-only error useEffect with useSyncExternalStore so validation runs after hydration without triggering react-hooks/set-state-in-effect. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks for the sequential-fetch refactor and the [Bug — high] Polyline ref leak from effect ordering Effect 1 clears Fix: in effect 2's [Bug — medium] The fallback loop uses Fix: track a [Nit — low] O(n²)
|
Summary
/results.Motivation
/resultslooked like direct stop-to-stop segments, not drivable paths — misleading for drivers/dispatchers even though lat/lng data was correct.RoutePolylinesOverlayalready called Google DirectionsService, but in practice most routes hit the straight-line fallback (drawFallback).Changes
Frontend (
app/ui)Map.tsx— road-following polylinesrequestDrivingDirections()with explicitDirectionsStatus.OKcheck (callback-based, not fire-and-forget await).extractRoadPath()— usesoverview_path, falls back to leg step paths when overview is empty.OVER_QUERY_LIMITfailures.setPath(committed)on cache miss.routesPathKeyso note/distance edits don’t tear down and redraw directions.Map.tsx— marker anchoring (prior commit on branch)scaledSize+ bottom-center anchor.page.tsx— data loading (prior commit on branch)?mock=1and automatic mock fallback; show error overlay when no/corruptsessionStoragedata..env.exampleNEXT_PUBLIC_GOOGLE_MAPS_KEY.Backend / mobile app / infra
Validation
Frontend
npm --prefix app/ui run lintnpm --prefix app/ui run format:checknpm --prefix app/ui run typechecknpm --prefix app/ui run testnpm --prefix app/ui run buildnpm --prefix app/ui run test:e2enpm --prefix app/mobile run lintnpm --prefix app/mobile run typecheckBackend
cmake --preset dev.github/scripts/check-backend-static.sh build/devcmake --build --preset dev --parallelctest --preset dev --output-on-failure --no-tests=error -LE 'e2e|docker'docker compose -f deploy/compose/docker-compose.arm64.yml --env-file deploy/env/http-server.arm64.env configManual checks
/edit→ open/results: polylines follow roads/turns between depot and stops.DirectionsService failed./resultsvisit without optimize: error overlay (no mock routes).Risk
/resultsrequires prior optimize or shows error overlay.Rollout and Recovery
app/ui; no migrations or feature flags.Map.tsxRoutePolylinesOverlaychanges while keeping marker/data-loading fixes.High-Signal PR Checklist
Closes #195