Merge remote-tracking branch 'origin/main' into codex/usage-v4-client
This commit is contained in:
@@ -1,3 +1,3 @@
|
||||
{
|
||||
".": "3.126.1-beta.0"
|
||||
".": "3.126.2-beta.0"
|
||||
}
|
||||
|
||||
@@ -5,6 +5,15 @@ All notable changes to PicPeak will be documented in this file.
|
||||
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
|
||||
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
## [3.126.2-beta.0](https://github.com/PicPeak/picpeak/compare/v3.126.1-beta.0...v3.126.2-beta.0) (2026-09-06)
|
||||
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
* **gallery:** release grid tiles once they are far enough out of view ([b3937d0](https://github.com/PicPeak/picpeak/commit/b3937d0b8c54c8ddf8a428be88aa444ab2b4f2d2))
|
||||
* **gallery:** retry a failed image fetch once the tile is back on screen ([c4b03a8](https://github.com/PicPeak/picpeak/commit/c4b03a831f843ec447a54d712aefa89c9761e8e8))
|
||||
* **gallery:** retry a failed image fetch once the tile is back on screen ([77ae94e](https://github.com/PicPeak/picpeak/commit/77ae94e649f367bb0a44166d3215ce1884c660d2))
|
||||
|
||||
## [3.126.1-beta.0](https://github.com/PicPeak/picpeak/compare/v3.126.0-beta.0...v3.126.1-beta.0) (2026-09-06)
|
||||
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "picpeak-backend",
|
||||
"version": "3.126.1-beta.0",
|
||||
"version": "3.126.2-beta.0",
|
||||
"description": "Backend for PicPeak event photo sharing platform",
|
||||
"main": "server.js",
|
||||
"engines": {
|
||||
|
||||
+6
-1
@@ -239,7 +239,12 @@ const corsOptions = {
|
||||
// deployments can read the server's chosen download filename. Used
|
||||
// by the gallery/admin download flows to honour the #493 "original
|
||||
// camera filename" toggle on individual photo downloads (#507).
|
||||
exposedHeaders: ['Content-Disposition'],
|
||||
//
|
||||
// Retry-After is not CORS-safelisted either. AuthenticatedImage reads it
|
||||
// off a 429 to wait out the rate-limit window before retrying a thumbnail
|
||||
// fetch; without it a split-origin deployment would spend its retry budget
|
||||
// inside the window and leave the tile blank after the limit had lifted.
|
||||
exposedHeaders: ['Content-Disposition', 'Retry-After'],
|
||||
};
|
||||
|
||||
// Only attach CORS to API endpoints, not static assets
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
{
|
||||
"name": "picpeak-frontend",
|
||||
"private": true,
|
||||
"version": "3.126.1-beta.0",
|
||||
"version": "3.126.2-beta.0",
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
"dev": "vite",
|
||||
|
||||
@@ -30,6 +30,44 @@ interface AuthenticatedImageProps extends Omit<React.ImgHTMLAttributes<HTMLImage
|
||||
queuePriority?: 'high' | 'prefetch' | 'normal';
|
||||
}
|
||||
|
||||
/**
|
||||
* Retry budget for a fetch that rejects (#1287). Three attempts with a
|
||||
* doubling delay — 2 s, 4 s, 8 s — and each one waits until the placeholder
|
||||
* is actually on screen and the document is visible before it fires.
|
||||
*/
|
||||
const MAX_RETRIES = 3;
|
||||
const RETRY_BASE_DELAY_MS = 2000;
|
||||
|
||||
/** A non-OK response, with its status so the retry can tell transient from final. */
|
||||
class HttpError extends Error {
|
||||
status: number;
|
||||
/** Server-stated cooldown from `Retry-After`, in ms; 0 when absent. */
|
||||
retryAfterMs: number;
|
||||
constructor(status: number, statusText: string, retryAfter: string | null) {
|
||||
super(`Failed to fetch image: ${status} ${statusText}`);
|
||||
this.status = status;
|
||||
this.retryAfterMs = parseRetryAfter(retryAfter);
|
||||
}
|
||||
}
|
||||
|
||||
/** `Retry-After` is either delay-seconds or an HTTP date. */
|
||||
function parseRetryAfter(value: string | null): number {
|
||||
if (!value) return 0;
|
||||
const seconds = Number(value);
|
||||
if (Number.isFinite(seconds)) return Math.max(0, seconds * 1000);
|
||||
const at = Date.parse(value);
|
||||
return Number.isFinite(at) ? Math.max(0, at - Date.now()) : 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* A 4xx other than 408 (timeout) and 429 (rate limited) is the server's final
|
||||
* answer for this URL — an expired gallery token, a missing photo — and asking
|
||||
* again cannot change it. Everything else (network failure, 5xx, aborts that
|
||||
* were not ours) may.
|
||||
*/
|
||||
const isFinalStatus = (status: number) =>
|
||||
status >= 400 && status < 500 && status !== 408 && status !== 429;
|
||||
|
||||
/**
|
||||
* Fetches an image with the gallery's bearer token and renders it.
|
||||
*
|
||||
@@ -76,6 +114,17 @@ export const AuthenticatedImage: React.FC<AuthenticatedImageProps> = ({
|
||||
const [canvasFailed, setCanvasFailed] = useState(false);
|
||||
const canvasRef = useRef<HTMLCanvasElement>(null);
|
||||
const imageRef = useRef<HTMLImageElement | null>(null);
|
||||
// Retry state (#1287). The nonce is a dependency of the fetch effect, so
|
||||
// bumping it is the retry; the counter is per src, so a new image gets a
|
||||
// fresh budget without an extra effect run to reset it.
|
||||
const [retryNonce, setRetryNonce] = useState(0);
|
||||
const attemptsRef = useRef(0);
|
||||
// Cooldown the server asked for on the last failure (#1287). The backoff
|
||||
// alone would spend all three retries inside a 15-minute rate-limit window
|
||||
// and leave the tile blank after the limit had actually lifted.
|
||||
const retryAfterRef = useRef(0);
|
||||
const lastSrcRef = useRef<string | undefined>(undefined);
|
||||
const retryRef = useRef<HTMLDivElement | null>(null);
|
||||
|
||||
// Draw image to canvas when canvas rendering is enabled
|
||||
// Returns whether the pixels actually made it onto the canvas, so the
|
||||
@@ -108,9 +157,15 @@ export const AuthenticatedImage: React.FC<AuthenticatedImageProps> = ({
|
||||
// screen, which on a several-hundred-photo gallery is most of them.
|
||||
const controller = new AbortController();
|
||||
|
||||
if (lastSrcRef.current !== src) {
|
||||
lastSrcRef.current = src;
|
||||
attemptsRef.current = 0;
|
||||
}
|
||||
|
||||
// Determine which token to use based on context
|
||||
if (!src) {
|
||||
setImageSrc(fallbackSrc || '');
|
||||
setError(false);
|
||||
setIsLoading(false);
|
||||
return;
|
||||
}
|
||||
@@ -175,7 +230,11 @@ export const AuthenticatedImage: React.FC<AuthenticatedImageProps> = ({
|
||||
});
|
||||
|
||||
if (!response.ok) {
|
||||
throw new Error(`Failed to fetch image: ${response.status} ${response.statusText}`);
|
||||
throw new HttpError(
|
||||
response.status,
|
||||
response.statusText,
|
||||
response.headers?.get?.('Retry-After') ?? null,
|
||||
);
|
||||
}
|
||||
|
||||
return await response.blob();
|
||||
@@ -203,6 +262,13 @@ export const AuthenticatedImage: React.FC<AuthenticatedImageProps> = ({
|
||||
// Returning here also stops the fallback below from firing a second
|
||||
// request against an already-aborted signal.
|
||||
if (aborted) return;
|
||||
// A final 4xx exhausts the retry budget: on a 68-tile viewport, three
|
||||
// retries per tile against an expired token would be ~200 requests
|
||||
// that cannot succeed.
|
||||
if (err instanceof HttpError && isFinalStatus(err.status)) {
|
||||
attemptsRef.current = MAX_RETRIES;
|
||||
}
|
||||
retryAfterRef.current = err instanceof HttpError ? err.retryAfterMs : 0;
|
||||
setIsLoading(false);
|
||||
if (fallbackSrc && fallbackSrc !== src) {
|
||||
try {
|
||||
@@ -238,7 +304,65 @@ export const AuthenticatedImage: React.FC<AuthenticatedImageProps> = ({
|
||||
objectUrls.forEach((url) => URL.revokeObjectURL(url));
|
||||
};
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [src, fallbackSrc, slug, queuePriority]);
|
||||
}, [src, fallbackSrc, slug, queuePriority, retryNonce]);
|
||||
|
||||
// Retry a failed fetch once the tile is back on screen (#1287).
|
||||
//
|
||||
// Before this, a rejected fetch set `error` and nothing ever asked again:
|
||||
// the effect above only re-runs when its inputs change, and for a grid
|
||||
// tile they never do. On the original reporter's install that was the
|
||||
// difference between a transient failure and a permanently blank tile —
|
||||
// a hiccup on cellular, or Safari cancelling loads when the tab goes to
|
||||
// the background, left a tile with no image, no request in flight and
|
||||
// nothing in any log, for as long as the gallery stayed open. That is the
|
||||
// retry-on-scrolling-back-into-view the reporter asked for in the issue.
|
||||
//
|
||||
// Bounded, and gated on visibility. The delay doubles per attempt so a
|
||||
// server that is actually down is not hammered, and an attempt does not
|
||||
// fire until the placeholder intersects the viewport and the document is
|
||||
// visible — a tile that failed while backgrounded retries when the user
|
||||
// comes back, not while they are still away. Without IntersectionObserver
|
||||
// the placeholder counts as visible.
|
||||
const retryable = error && !fallbackSrc && attemptsRef.current < MAX_RETRIES;
|
||||
useEffect(() => {
|
||||
if (!retryable) return;
|
||||
const el = retryRef.current;
|
||||
if (!el) return;
|
||||
|
||||
let cancelled = false;
|
||||
let delayElapsed = false;
|
||||
let onScreen = typeof IntersectionObserver === 'undefined';
|
||||
|
||||
const retry = () => {
|
||||
if (cancelled || !delayElapsed || !onScreen) return;
|
||||
if (typeof document !== 'undefined' && document.visibilityState === 'hidden') return;
|
||||
cancelled = true;
|
||||
attemptsRef.current += 1;
|
||||
setRetryNonce((n) => n + 1);
|
||||
};
|
||||
|
||||
const timer = setTimeout(() => {
|
||||
delayElapsed = true;
|
||||
retry();
|
||||
}, Math.max(RETRY_BASE_DELAY_MS * 2 ** attemptsRef.current, retryAfterRef.current));
|
||||
|
||||
let observer: IntersectionObserver | null = null;
|
||||
if (typeof IntersectionObserver !== 'undefined') {
|
||||
observer = new IntersectionObserver((entries) => {
|
||||
onScreen = entries.some((entry) => entry.isIntersecting);
|
||||
retry();
|
||||
});
|
||||
observer.observe(el);
|
||||
}
|
||||
document.addEventListener('visibilitychange', retry);
|
||||
|
||||
return () => {
|
||||
cancelled = true;
|
||||
clearTimeout(timer);
|
||||
observer?.disconnect();
|
||||
document.removeEventListener('visibilitychange', retry);
|
||||
};
|
||||
}, [retryable, retryNonce]);
|
||||
|
||||
// Effect to draw to canvas when image is loaded and canvas rendering is enabled
|
||||
useEffect(() => {
|
||||
@@ -314,6 +438,15 @@ export const AuthenticatedImage: React.FC<AuthenticatedImageProps> = ({
|
||||
return <img src={fallbackSrc} alt={alt} {...props} />;
|
||||
}
|
||||
|
||||
if (error) {
|
||||
// Same box as the loading placeholder, and the element the retry effect
|
||||
// observes. Returning null here (as this used to) left nothing to watch
|
||||
// and nothing for the user to see either.
|
||||
return (
|
||||
<div ref={retryRef} className={props.className} style={{ backgroundColor: '#f3f4f6', ...props.style }} />
|
||||
);
|
||||
}
|
||||
|
||||
if (!imageSrc) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,216 @@
|
||||
/**
|
||||
* Retry after a failed fetch (#1287).
|
||||
*
|
||||
* A rejected fetch used to set `error`, render nothing, and never ask again:
|
||||
* the fetch effect only re-runs when its inputs change, and for a grid tile
|
||||
* they never do. A transient failure — a hiccup on cellular, Safari
|
||||
* cancelling loads when the tab is backgrounded — was therefore a
|
||||
* permanently blank tile with nothing in any log.
|
||||
*
|
||||
* The retry is bounded (three attempts, doubling delay) and gated on the
|
||||
* placeholder being on screen and the document being visible.
|
||||
*/
|
||||
import { act, render } from '@testing-library/react';
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
|
||||
vi.mock('../../../utils/galleryAuthStorage', () => ({
|
||||
getActiveGallerySlug: () => 'demo',
|
||||
getGalleryToken: () => 'token',
|
||||
inferGallerySlugFromLocation: () => 'demo',
|
||||
resolveSlugFromRequestUrl: () => 'demo',
|
||||
}));
|
||||
vi.mock('../../../utils/url', () => ({ buildResourceUrl: (u: string) => `http://localhost${u}` }));
|
||||
|
||||
import { AuthenticatedImage } from '../AuthenticatedImage';
|
||||
|
||||
const ok = () => Promise.resolve({
|
||||
ok: true,
|
||||
blob: async () => new Blob(['x'], { type: 'image/png' }),
|
||||
});
|
||||
const loadFailed = () => Promise.reject(new TypeError('Load failed'));
|
||||
const status = (code: number, headers: Record<string, string> = {}) => () => Promise.resolve({
|
||||
ok: false,
|
||||
status: code,
|
||||
statusText: 'x',
|
||||
headers: { get: (name: string) => headers[name] ?? null },
|
||||
});
|
||||
|
||||
/** The observer callbacks registered by the component, so a test can drive them. */
|
||||
type IOCallback = (entries: Array<{ isIntersecting: boolean }>) => void;
|
||||
let observers: IOCallback[];
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
observers = [];
|
||||
URL.createObjectURL = vi.fn(() => 'blob:mock') as unknown as typeof URL.createObjectURL;
|
||||
URL.revokeObjectURL = vi.fn() as unknown as typeof URL.revokeObjectURL;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
/** Let every pending promise in the fetch/queue chain settle. */
|
||||
const flush = () => act(async () => { await vi.advanceTimersByTimeAsync(0); });
|
||||
const advance = (ms: number) => act(async () => { await vi.advanceTimersByTimeAsync(ms); });
|
||||
|
||||
/** IntersectionObserver stub that never fires on its own; tests drive it. */
|
||||
function stubIntersectionObserver() {
|
||||
vi.stubGlobal('IntersectionObserver', class {
|
||||
constructor(cb: IOCallback) { observers.push(cb); }
|
||||
observe() {}
|
||||
disconnect() {}
|
||||
unobserve() {}
|
||||
});
|
||||
}
|
||||
|
||||
describe('AuthenticatedImage retry', () => {
|
||||
it('retries a failed fetch after the delay and renders the image', async () => {
|
||||
const fetchMock = vi.fn().mockImplementationOnce(loadFailed).mockImplementation(ok);
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
const { container } = render(<AuthenticatedImage src="/api/gallery/demo/thumbnail/1" alt="t" />);
|
||||
await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
expect(container.querySelector('img')).toBeNull();
|
||||
|
||||
await advance(1999);
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
|
||||
await advance(1);
|
||||
await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
expect(container.querySelector('img')?.getAttribute('src')).toBe('blob:mock');
|
||||
});
|
||||
|
||||
it('gives up after three retries with a doubling delay', async () => {
|
||||
const fetchMock = vi.fn().mockImplementation(loadFailed);
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
render(<AuthenticatedImage src="/api/gallery/demo/thumbnail/1" alt="t" />);
|
||||
await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
|
||||
await advance(2000); await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
await advance(4000); await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(3);
|
||||
await advance(8000); await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(4);
|
||||
|
||||
await advance(60_000); await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(4);
|
||||
});
|
||||
|
||||
it('waits until the placeholder is on screen before retrying', async () => {
|
||||
stubIntersectionObserver();
|
||||
const fetchMock = vi.fn().mockImplementationOnce(loadFailed).mockImplementation(ok);
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
const { container } = render(<AuthenticatedImage src="/api/gallery/demo/thumbnail/1" alt="t" />);
|
||||
await flush();
|
||||
expect(observers).toHaveLength(1);
|
||||
|
||||
// Delay elapsed, but the tile is not on screen: no retry.
|
||||
await advance(2000); await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Scrolled back into view: retry fires now, without another delay.
|
||||
act(() => observers[0]([{ isIntersecting: true }]));
|
||||
await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
expect(container.querySelector('img')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('does not retry while the document is hidden, and does when it comes back', async () => {
|
||||
const fetchMock = vi.fn().mockImplementationOnce(loadFailed).mockImplementation(ok);
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
const visibility = vi.spyOn(document, 'visibilityState', 'get').mockReturnValue('hidden');
|
||||
|
||||
render(<AuthenticatedImage src="/api/gallery/demo/thumbnail/1" alt="t" />);
|
||||
await flush();
|
||||
await advance(2000); await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
|
||||
visibility.mockReturnValue('visible');
|
||||
act(() => { document.dispatchEvent(new Event('visibilitychange')); });
|
||||
await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
visibility.mockRestore();
|
||||
});
|
||||
|
||||
it('does not retry a final 4xx such as an expired token', async () => {
|
||||
const fetchMock = vi.fn().mockImplementation(status(401));
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
render(<AuthenticatedImage src="/api/gallery/demo/thumbnail/1" alt="t" />);
|
||||
await flush();
|
||||
await advance(60_000); await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('retries a 5xx and a 429', async () => {
|
||||
const fetchMock = vi.fn()
|
||||
.mockImplementationOnce(status(503))
|
||||
.mockImplementationOnce(status(429))
|
||||
.mockImplementation(ok);
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
const { container } = render(<AuthenticatedImage src="/api/gallery/demo/thumbnail/1" alt="t" />);
|
||||
await flush();
|
||||
await advance(2000); await flush();
|
||||
await advance(4000); await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(3);
|
||||
expect(container.querySelector('img')?.getAttribute('src')).toBe('blob:mock');
|
||||
});
|
||||
|
||||
it('waits out a Retry-After longer than the backoff before spending a retry', async () => {
|
||||
const fetchMock = vi.fn()
|
||||
.mockImplementationOnce(status(429, { 'Retry-After': '30' }))
|
||||
.mockImplementation(ok);
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
const { container } = render(<AuthenticatedImage src="/api/gallery/demo/thumbnail/1" alt="t" />);
|
||||
await flush();
|
||||
// The 2 s backoff alone would have fired here.
|
||||
await advance(29_999); await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
await advance(1); await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
expect(container.querySelector('img')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('leaves the fallbackSrc path alone', async () => {
|
||||
const fetchMock = vi.fn().mockImplementation(loadFailed);
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
const { container } = render(
|
||||
<AuthenticatedImage src="/api/gallery/demo/thumbnail/1" fallbackSrc="/static/fallback.png" alt="t" />,
|
||||
);
|
||||
await flush();
|
||||
// primary + fallback, then the plain <img> takes over — no retry loop.
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
expect(container.querySelector('img')?.getAttribute('src')).toBe('/static/fallback.png');
|
||||
await advance(60_000); await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('a new src gets a fresh retry budget', async () => {
|
||||
const fetchMock = vi.fn().mockImplementation(loadFailed);
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
const { rerender } = render(<AuthenticatedImage src="/api/gallery/demo/thumbnail/1" alt="t" />);
|
||||
await flush();
|
||||
await advance(2000); await flush();
|
||||
await advance(4000); await flush();
|
||||
await advance(8000); await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(4);
|
||||
|
||||
rerender(<AuthenticatedImage src="/api/gallery/demo/thumbnail/2" alt="t" />);
|
||||
await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(5);
|
||||
await advance(2000); await flush();
|
||||
expect(fetchMock).toHaveBeenCalledTimes(6);
|
||||
});
|
||||
});
|
||||
@@ -35,6 +35,13 @@ export interface PhotoCardProps {
|
||||
/** Lazy-render via IntersectionObserver with a skeleton placeholder. */
|
||||
lazy?: boolean;
|
||||
inViewRootMargin?: string;
|
||||
/**
|
||||
* Outer band, in `rootMargin` form. When set, a tile that leaves it is
|
||||
* unmounted again rather than kept for the life of the page (#1287). Opt-in
|
||||
* per layout: only a layout whose skeleton holds the tile's box can release
|
||||
* without reflowing, which today is Grid (`aspect-square`).
|
||||
*/
|
||||
releaseRootMargin?: string;
|
||||
skeletonClassName?: string;
|
||||
/** Keep container at opacity 0 until in view (only meaningful with `lazy`). */
|
||||
fadeInWhenVisible?: boolean;
|
||||
@@ -85,6 +92,7 @@ export const PhotoCard: React.FC<PhotoCardProps> = ({
|
||||
imageProps,
|
||||
lazy = false,
|
||||
inViewRootMargin,
|
||||
releaseRootMargin,
|
||||
skeletonClassName = 'skeleton w-full h-full rounded-lg',
|
||||
fadeInWhenVisible = false,
|
||||
overlayBaseClassName,
|
||||
@@ -157,13 +165,40 @@ export const PhotoCard: React.FC<PhotoCardProps> = ({
|
||||
}
|
||||
}, [isSelectionMode, hideOverlay]);
|
||||
|
||||
// Lazy loading with intersection observer
|
||||
const { ref, inView: observedInView } = useInView({
|
||||
triggerOnce: true,
|
||||
// Lazy loading with intersection observer.
|
||||
//
|
||||
// Two bands with a deliberate gap between them (#1287). The inner one, from
|
||||
// `inViewRootMargin`, decides when a tile starts loading. The outer one,
|
||||
// from `releaseRootMargin`, decides when it is far enough away to unmount —
|
||||
// and unmounting is the part that frees anything, because AuthenticatedImage
|
||||
// revokes its object URL and drops any protection canvas in its cleanup, and
|
||||
// neither is reclaimable while the tile stays mounted. On a 546-photo grid
|
||||
// the old latch meant every tile scrolled past was retained for the life of
|
||||
// the page, which is the memory profile iOS Safari discards a tab over.
|
||||
//
|
||||
// The gap between the bands is the hysteresis: a tile is not released until
|
||||
// it is well outside the band that would immediately reload it, so scrolling
|
||||
// back and forth across one edge cannot thrash. Without a release band the
|
||||
// observer keeps its original `triggerOnce` latch, so every other layout
|
||||
// behaves exactly as before.
|
||||
const releases = Boolean(lazy && releaseRootMargin);
|
||||
const { ref: loadBandRef, inView: withinLoadBand } = useInView({
|
||||
triggerOnce: !releases,
|
||||
threshold: 0.1,
|
||||
rootMargin: inViewRootMargin,
|
||||
});
|
||||
const inView = !lazy || observedInView;
|
||||
const { ref: keepBandRef, inView: withinKeepBand } = useInView({
|
||||
skip: !releases,
|
||||
threshold: 0,
|
||||
rootMargin: releaseRootMargin,
|
||||
});
|
||||
const [rendered, setRendered] = useState(false);
|
||||
useEffect(() => {
|
||||
if (!releases) return;
|
||||
if (withinLoadBand) setRendered(true);
|
||||
else if (!withinKeepBand) setRendered(false);
|
||||
}, [releases, withinLoadBand, withinKeepBand]);
|
||||
const inView = !lazy || (releases ? rendered : withinLoadBand);
|
||||
|
||||
// Tile width for the responsive tier (#1095), measured rather than inferred.
|
||||
// The observer entry only exists for `lazy` cards, and Mosaic, Masonry and
|
||||
@@ -183,8 +218,10 @@ export const PhotoCard: React.FC<PhotoCardProps> = ({
|
||||
const [tile, setTile] = useState<{ width: number | null } | null>(null);
|
||||
const setContainerRef = useCallback((node: HTMLDivElement | null) => {
|
||||
containerRef.current = node;
|
||||
if (lazy) ref(node);
|
||||
}, [ref, lazy]);
|
||||
if (!lazy) return;
|
||||
loadBandRef(node);
|
||||
if (releases) keepBandRef(node);
|
||||
}, [loadBandRef, keepBandRef, lazy, releases]);
|
||||
|
||||
useLayoutEffect(() => {
|
||||
if (!inView || tile) return;
|
||||
|
||||
@@ -0,0 +1,183 @@
|
||||
/**
|
||||
* Releasing offscreen grid tiles (#1287).
|
||||
*
|
||||
* The pre-load band made tiles arrive in time; it did nothing about them never
|
||||
* leaving. A tile that has been scrolled past keeps its object URL, and where
|
||||
* image protection is on a full-resolution canvas the browser may not evict,
|
||||
* for the life of the page — so a several-hundred-photo grid grows monotonically
|
||||
* until the browser discards the tab.
|
||||
*
|
||||
* Unmounting is what frees those, so that is what these assert: the tile's
|
||||
* subtree really goes away when it is far enough out, really comes back, and
|
||||
* never flickers on ordinary scrolling. The `releaseRootMargin`-less path is
|
||||
* pinned too, because every other layout still depends on the old latch.
|
||||
*/
|
||||
import React from 'react';
|
||||
import { describe, it, expect, beforeEach, vi } from 'vitest';
|
||||
import { render, screen } from '@testing-library/react';
|
||||
|
||||
import { PhotoCard } from '../PhotoCard';
|
||||
import type { Photo } from '../../../types';
|
||||
|
||||
const LOAD_BAND = '100% 0px';
|
||||
const KEEP_BAND = '300% 0px';
|
||||
|
||||
/** Which bands the tile is currently inside; the test drives these directly. */
|
||||
let bands: Record<string, boolean> = {};
|
||||
// `triggerOnce` has to be modelled, not ignored: it is the whole difference
|
||||
// between a layout that opts in to releasing and one that does not, so a mock
|
||||
// that always reports live visibility would quietly turn the latch test into a
|
||||
// test of the mock.
|
||||
vi.mock('react-intersection-observer', () => ({
|
||||
useInView: (
|
||||
{ rootMargin, skip, triggerOnce }:
|
||||
{ rootMargin?: string; skip?: boolean; triggerOnce?: boolean },
|
||||
) => {
|
||||
const latched = React.useRef(false);
|
||||
const live = skip ? false : Boolean(bands[rootMargin ?? '']);
|
||||
if (live) latched.current = true;
|
||||
return { ref: () => {}, inView: triggerOnce ? latched.current : live };
|
||||
},
|
||||
}));
|
||||
|
||||
// Counting mounts and unmounts is the whole point: it is the unmount that
|
||||
// revokes the object URL and drops the canvas.
|
||||
const lifecycle = { mounted: 0, unmounted: 0 };
|
||||
vi.mock('../../common', () => ({
|
||||
AuthenticatedImage: ({ src }: { src: string }) => {
|
||||
React.useEffect(() => {
|
||||
lifecycle.mounted += 1;
|
||||
return () => { lifecycle.unmounted += 1; };
|
||||
}, []);
|
||||
return <img data-testid="tile" src={src} alt="" />;
|
||||
},
|
||||
}));
|
||||
vi.mock('../../../contexts/GuestIdentityContext', () => ({
|
||||
useGuestIdentityOptional: () => null,
|
||||
}));
|
||||
|
||||
const PHOTO = {
|
||||
id: 7,
|
||||
filename: 'IMG_0001.jpg',
|
||||
url: '/api/gallery/x/photo/7',
|
||||
thumbnail_url: '/api/gallery/x/thumbnail/7',
|
||||
type: 'individual',
|
||||
size: 1,
|
||||
uploaded_at: '2026-01-01T00:00:00Z',
|
||||
} as Photo;
|
||||
|
||||
function renderCard(props: Partial<React.ComponentProps<typeof PhotoCard>> = {}) {
|
||||
return render(
|
||||
<PhotoCard
|
||||
photo={PHOTO}
|
||||
isSelected={false}
|
||||
isSelectionMode={false}
|
||||
onClick={() => {}}
|
||||
onDownload={() => {}}
|
||||
onToggleSelect={() => {}}
|
||||
className="tile aspect-square"
|
||||
skeletonClassName="skeleton aspect-square"
|
||||
overlayBaseClassName="overlay"
|
||||
imageProps={{ src: PHOTO.thumbnail_url!, alt: PHOTO.filename }}
|
||||
lazy
|
||||
inViewRootMargin={LOAD_BAND}
|
||||
releaseRootMargin={KEEP_BAND}
|
||||
{...props}
|
||||
/>,
|
||||
);
|
||||
}
|
||||
|
||||
const scrollTo = (
|
||||
rerender: (ui: React.ReactElement) => void,
|
||||
next: Record<string, boolean>,
|
||||
props: Partial<React.ComponentProps<typeof PhotoCard>> = {},
|
||||
) => {
|
||||
bands = next;
|
||||
rerender(
|
||||
<PhotoCard
|
||||
photo={PHOTO}
|
||||
isSelected={false}
|
||||
isSelectionMode={false}
|
||||
onClick={() => {}}
|
||||
onDownload={() => {}}
|
||||
onToggleSelect={() => {}}
|
||||
className="tile aspect-square"
|
||||
skeletonClassName="skeleton aspect-square"
|
||||
overlayBaseClassName="overlay"
|
||||
imageProps={{ src: PHOTO.thumbnail_url!, alt: PHOTO.filename }}
|
||||
lazy
|
||||
inViewRootMargin={LOAD_BAND}
|
||||
releaseRootMargin={KEEP_BAND}
|
||||
{...props}
|
||||
/>,
|
||||
);
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
bands = {};
|
||||
lifecycle.mounted = 0;
|
||||
lifecycle.unmounted = 0;
|
||||
Object.defineProperty(HTMLElement.prototype, 'offsetWidth', {
|
||||
configurable: true,
|
||||
get() { return 200; },
|
||||
});
|
||||
});
|
||||
|
||||
describe('grid tiles release when they are far enough out of view', () => {
|
||||
it('does not load a tile that is only inside the outer band', () => {
|
||||
bands = { [KEEP_BAND]: true };
|
||||
renderCard();
|
||||
expect(screen.queryByTestId('tile')).toBeNull();
|
||||
expect(lifecycle.mounted).toBe(0);
|
||||
});
|
||||
|
||||
it('loads inside the pre-load band and releases past the outer band', () => {
|
||||
bands = { [LOAD_BAND]: true, [KEEP_BAND]: true };
|
||||
const { rerender } = renderCard();
|
||||
expect(screen.getByTestId('tile')).toBeTruthy();
|
||||
expect(lifecycle.mounted).toBe(1);
|
||||
|
||||
// Scrolled well past: outside both bands.
|
||||
scrollTo(rerender, {});
|
||||
expect(screen.queryByTestId('tile')).toBeNull();
|
||||
// The unmount is the release — object URL revoked, canvas dropped.
|
||||
expect(lifecycle.unmounted).toBe(1);
|
||||
});
|
||||
|
||||
it('holds the tile in the gap between the bands, so scrolling cannot thrash', () => {
|
||||
bands = { [LOAD_BAND]: true, [KEEP_BAND]: true };
|
||||
const { rerender } = renderCard();
|
||||
|
||||
// Past the load band but still within the keep band — the hysteresis gap.
|
||||
scrollTo(rerender, { [KEEP_BAND]: true });
|
||||
expect(screen.getByTestId('tile')).toBeTruthy();
|
||||
expect(lifecycle.unmounted).toBe(0);
|
||||
|
||||
// Back towards the viewport without ever having been released.
|
||||
scrollTo(rerender, { [LOAD_BAND]: true, [KEEP_BAND]: true });
|
||||
expect(lifecycle.mounted).toBe(1);
|
||||
});
|
||||
|
||||
it('brings a released tile back when it returns', () => {
|
||||
bands = { [LOAD_BAND]: true, [KEEP_BAND]: true };
|
||||
const { rerender } = renderCard();
|
||||
scrollTo(rerender, {});
|
||||
expect(screen.queryByTestId('tile')).toBeNull();
|
||||
|
||||
scrollTo(rerender, { [LOAD_BAND]: true, [KEEP_BAND]: true });
|
||||
expect(screen.getByTestId('tile')).toBeTruthy();
|
||||
expect(lifecycle.mounted).toBe(2);
|
||||
});
|
||||
|
||||
it('keeps the old latch for layouts that do not opt in', () => {
|
||||
bands = { [LOAD_BAND]: true };
|
||||
const { rerender } = renderCard({ releaseRootMargin: undefined });
|
||||
expect(screen.getByTestId('tile')).toBeTruthy();
|
||||
|
||||
// Far outside everything. A measured layout has no skeleton that holds the
|
||||
// box, so releasing there would reflow — it must stay mounted.
|
||||
scrollTo(rerender, {}, { releaseRootMargin: undefined });
|
||||
expect(screen.getByTestId('tile')).toBeTruthy();
|
||||
expect(lifecycle.unmounted).toBe(0);
|
||||
});
|
||||
});
|
||||
@@ -101,6 +101,26 @@ const GridPhoto: React.FC<GridPhotoProps> = ({
|
||||
* of lead in each direction, which is what vh would have meant.
|
||||
*/
|
||||
inViewRootMargin="100% 0px"
|
||||
/*
|
||||
* Release band (#1287). The pre-load band above fixed tiles arriving
|
||||
* late; it did nothing about tiles never leaving. Every tile scrolled
|
||||
* past kept its object URL — and, where image protection is on, a
|
||||
* full-resolution canvas that the browser is not allowed to evict — for
|
||||
* the life of the page. On a several-hundred-photo gallery that grows
|
||||
* monotonically, which is the shape a memory-constrained browser
|
||||
* discards the tab over.
|
||||
*
|
||||
* Three viewport heights, against a one-viewport load band: a tile has
|
||||
* to travel two further viewport heights after it stops loading before
|
||||
* it is released, so ordinary scrolling never crosses both edges.
|
||||
* Thumbnails are served `private, max-age=1800`, so coming back costs a
|
||||
* cache hit rather than a round trip.
|
||||
*
|
||||
* Grid only, and deliberately so: the skeleton here is `aspect-square`
|
||||
* and holds the tile's box exactly, so releasing shifts nothing. The
|
||||
* measured layouts have no such guarantee.
|
||||
*/
|
||||
releaseRootMargin="300% 0px"
|
||||
fadeInWhenVisible={animationType === 'fade'}
|
||||
skeletonClassName="skeleton aspect-square w-full rounded-lg"
|
||||
imageProps={{
|
||||
|
||||
@@ -45,6 +45,26 @@ describe('grid lazy pre-load band', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('Grid releases what it loaded, and the outer band is legal and wider', () => {
|
||||
// The pre-load band fixed tiles arriving late; it did nothing about them
|
||||
// never leaving. Measured in Chrome on a seeded 546-photo grid: without a
|
||||
// release band the mounted count climbs 24 → 100 → 212 → 364 → 546 and
|
||||
// never falls, because a tile that has been scrolled past keeps its object
|
||||
// URL and any protection canvas for the life of the page. With it the peak
|
||||
// is 68.
|
||||
const src = read('GridGalleryLayout.tsx');
|
||||
const release = src.match(/releaseRootMargin="([^"]+)"/);
|
||||
expect(release, 'Grid declares no releaseRootMargin').toBeTruthy();
|
||||
expect(release![1]).toMatch(LEGAL_ROOT_MARGIN);
|
||||
|
||||
// The gap between the bands is the hysteresis. If the outer band were not
|
||||
// strictly wider, a tile would be released and immediately reloaded on
|
||||
// every scroll across the edge.
|
||||
const load = src.match(/inViewRootMargin="([^"]+)"/);
|
||||
const percent = (value: string) => Number(value.split(/\s+/)[0].replace('%', ''));
|
||||
expect(percent(release![1])).toBeGreaterThan(percent(load![1]));
|
||||
});
|
||||
|
||||
it('every layout that lazy-renders also declares a pre-load band', () => {
|
||||
// The defect was Grid being lazy with no margin. Any future layout that
|
||||
// opts into `lazy` and forgets the margin reintroduces it.
|
||||
|
||||
Reference in New Issue
Block a user