Skip to content

Hotfix: fix infinite update loop crashing the map - #33

Closed
TomProkop wants to merge 5 commits into
mainfrom
users/tomas.prokop/turbo-potato
Closed

TomProkop wants to merge 5 commits into
mainfrom
users/tomas.prokop/turbo-potato

Conversation

@TomProkop

Copy link
Copy Markdown
Member

Root cause

The previous PR (#32) introduced an inline arrow function for markerRef in MediaMarkers.tsx, which gets a new identity every render. useClusterer relies on ref-callback identity staying stable per marker (its own docstring calls this out explicitly) — a fresh function each render makes React detach/reattach every AdvancedMarker ref on every render, which triggers the setMarkers state update in a loop and exceeds React's max update depth → crash (React error #185, "Maximum update depth exceeded") on any page rendering the map with geotagged media.

Fix

Caches one stable ref-callback function per post id (same pattern useClusterer uses internally), reading the post to tag onto the marker from a ref instead of capturing it in a new closure each render.

Verification

  • npm run build — clean
  • npm run lint — clean

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

TomProkop and others added 5 commits July 23, 2026 17:15
Three follow-ups from live use on the trip:

1. The shareable link required ?token=... - now the shared secret is
   embedded server-side into the rendered page (same pattern as the
   Maps API key) instead of being read from the URL, so crews can just
   be sent cicerallye.com/photos. Server-side validation is unchanged,
   so this is the same protection level as before.

2. Uploads were getting stuck forever on "Preparing upload for..."
   with no way to recover except reloading. Plain fetch() has no
   timeout, so a stalled mobile connection just hung indefinitely.
   Added an AbortController-based timeout (25s for the small JSON
   calls, 60s for the actual file PUT) with one automatic retry before
   surfacing a clear failure message. Confirmed via Azure CLI that the
   app settings and storage account config are otherwise healthy, so
   this was a client-side gap rather than a server regression.

3. Researched why the iOS gallery Location toggle still doesn't always
   work: confirmed this is a known WebKit/iOS inconsistency specific to
   HEIC (JPEG is far more reliable), not fixable client-side. Expanded
   the in-modal tip to mention switching Camera format to "Most
   Compatible" for anyone who wants reliable auto-geotagging.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- api/src/tripId.ts: getCurrentTripId() as the API-side source of truth
  for the active trip id, mirroring src/data/editions.ts (must be kept
  in sync manually - documented in the file).
- mediaTable.ts/mediaBlob.ts: partition key and blob paths are now
  trip-scoped ({tripId}/{day}/...); mediaComplete.ts's path regexes
  updated to match.
- Client-side thumbnail generation (photos.ts): generates a small
  (~200px, JPEG q0.7) thumbnail in-browser before upload (canvas draw
  from an img element for photos, a seeked video frame for videos) so
  map pins/clusters never download full-size originals. Best-effort -
  on any decode failure the upload proceeds without a thumbnail.
- mediaSas.ts now issues a second SAS URL for the thumbnail blob
  alongside the original; mediaComplete.ts stores thumbBlobPath/thumbUrl
  when present; media.ts/mediaMine.ts DTOs expose thumbUrl.
- MediaMarker.tsx: individual map pins now render the real photo/video
  thumbnail (circular, thumbUrl falling back to blobUrl) instead of a
  generic icon.
- MediaMarkers.tsx/mediaClusterRenderer.ts: markers are tagged with
  their post data on ref-set so cluster bubbles can show a random
  photo from the cluster (Apple Photos map-view style) plus the count
  badge, falling back to the amber camera badge if no thumbnail is
  available.
- MediaLightbox.tsx: close button switched to a solid bg-black/70 chip
  with a white ring and a larger (44px) tap target for visibility
  against bright photos.
- Also includes the previously-drafted "My uploads" ownership work
  (uploadedBy tracking, mediaMine.ts, mediaItem.ts delete/patch,
  email-capture + uploads list UI in photos.ts).

Verified: npx tsc --noEmit (api), npm run build, npm run lint, and a
syntax check of the embedded photos.ts script. Not merged/shipped per
instructions - holding on this branch pending further feature work.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- MediaMarker.tsx: only render a <video> element when no thumbUrl is
  present; thumbUrl is always a JPEG frame, so it must go through <img>.
- mediaItem.ts: DELETE now also removes the thumbnail blob
  (entity.thumbBlobPath) when present, instead of leaving it orphaned.
- mediaClusterRenderer.ts: pick the cluster thumbnail deterministically
  (newest post by id) instead of Math.random(), so it no longer
  flickers between different photos on every pan/zoom re-render. Also
  stopped falling back to blobUrl for videos without a generated
  thumbnail (that would point an <img> at a video file).
- photos.ts "Your uploads" list: prefer thumbUrl for the preview
  thumbnail, only falling back to a <video>/full-size <img> for older
  posts without one.
- mediaComplete.ts: blobPath/thumbBlobPath validation is now pinned to
  the current trip id (same value mediaPartitionKey() uses) instead of
  accepting any trip prefix, closing a cross-trip tampering gap.

Verified: npx tsc --noEmit (api), npm run build, npm run lint, and the
embedded photos.ts script syntax check.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
MediaMarkers.tsx was passing an inline arrow function as markerRef,
giving it a new identity on every render. useClusterer relies on ref
callback identity staying stable per marker (documented in its own
comment) - a fresh function each render makes React detach/reattach
every AdvancedMarker's ref on every render, which triggers the
setMarkers state update in a loop and blows past React's max update
depth, crashing the whole page with any geotagged media on the map.

Fixed by caching one stable ref-callback function per post id (same
pattern useClusterer itself uses internally), reading the post to tag
onto the marker from a ref instead of capturing it in a new closure.

Verified: npm run build, npm run lint.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@TomProkop

Copy link
Copy Markdown
Member Author

Superseded - branch history diverged after the #32 squash-merge. Reopened as a clean hotfix branch off main.

@TomProkop TomProkop closed this Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant