fix(P6): AttachmentImage Lightbox blob URL — split unmount-only revoke, lock deps to handle.id

This commit is contained in:
byGalax
2026-05-17 01:09:09 +02:00
parent 58efc66ca7
commit cd7ef8dccc
@@ -3,7 +3,7 @@ import {
downloadAndDecryptAttachment, downloadAndDecryptAttachment,
downloadAndDecryptAttachmentThumb, downloadAndDecryptAttachmentThumb,
} from '@chat-app/shared/chat'; } from '@chat-app/shared/chat';
import { useEffect, useState } from 'react'; import { useEffect, useRef, useState } from 'react';
import { getCachedAttachment, putCachedAttachment } from '../lib/attachmentCache'; import { getCachedAttachment, putCachedAttachment } from '../lib/attachmentCache';
import { supabase } from '../lib/supabase'; import { supabase } from '../lib/supabase';
@@ -166,11 +166,21 @@ export function AttachmentImage({ handle, mine = false }: Props) {
// skipped the eager full-blob download above). Resolves into the same // skipped the eager full-blob download above). Resolves into the same
// `fullUrl` state that the Lightbox consumes; the thumb URL keeps // `fullUrl` state that the Lightbox consumes; the thumb URL keeps
// backing the bubble until the lightbox actually mounts. // backing the bubble until the lightbox actually mounts.
//
// CRITICAL: do NOT revoke the just-created blob URL in this effect's
// cleanup. Setting `fullUrl` re-triggers the effect (state change → re-
// run → previous cleanup fires → URL revoked → Lightbox renders
// referenced-but-revoked URL → "ERR_FILE_NOT_FOUND"). The dedicated
// unmount-only effect below tracks the current URL via ref and revokes
// it once when the component truly leaves the tree.
//
// Deps locked to `handle.id` (not `handle`) — handles are immutable per
// attachment id, so object-identity churn from parent re-renders must
// not re-trigger the fetch.
useEffect(() => { useEffect(() => {
if (!lightboxOpen) return; if (!lightboxOpen) return;
if (fullUrl) return; if (fullUrl) return;
let cancelled = false; let cancelled = false;
const created: string[] = [];
void (async () => { void (async () => {
const cached = await getCachedAttachment(handle.id); const cached = await getCachedAttachment(handle.id);
let blob: Blob; let blob: Blob;
@@ -187,14 +197,27 @@ export function AttachmentImage({ handle, mine = false }: Props) {
} }
if (cancelled) return; if (cancelled) return;
const u = URL.createObjectURL(blob); const u = URL.createObjectURL(blob);
created.push(u);
setFullUrl(u); setFullUrl(u);
})(); })();
return () => { return () => {
cancelled = true; cancelled = true;
for (const u of created) URL.revokeObjectURL(u);
}; };
}, [lightboxOpen, fullUrl, handle]); // eslint-disable-next-line react-hooks/exhaustive-deps
}, [lightboxOpen, handle.id]);
// Track the currently-published fullUrl in a ref so the unmount-only
// cleanup below can revoke whatever URL is live at teardown time
// without subscribing to fullUrl changes (which would re-trigger and
// revoke prematurely — see the comment above the fetch effect).
const fullUrlRef = useRef<string | null>(null);
useEffect(() => {
fullUrlRef.current = fullUrl;
}, [fullUrl]);
useEffect(() => {
return () => {
if (fullUrlRef.current) URL.revokeObjectURL(fullUrlRef.current);
};
}, []);
const blobUrl = thumbUrl ?? fullUrl; const blobUrl = thumbUrl ?? fullUrl;