Skip to content

Commit 54bc2c0

Browse files
OXeuOXeu
authored
fix: resolve blurhash image loading race (#466)
* fix: resolve blurhash image loading race * test: stabilize image load state assertions * test: support bun coverage for client hook tests
1 parent 59ec12c commit 54bc2c0

6 files changed

Lines changed: 131 additions & 34 deletions

File tree

‎client/src/components/feed_card.tsx‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,17 @@ import { Link } from "wouter";
22
import { useTranslation } from "react-i18next";
33
import { timeago } from "../utils/timeago";
44
import { HashTag } from "./hashtag";
5-
import { useEffect, useMemo, useRef, useState } from "react";
5+
import { useEffect, useMemo, useRef } from "react";
66
import { drawBlurhashToCanvas } from "../utils/blurhash";
77
import { parseImageUrlMetadata } from "../utils/image-upload";
8+
import { useImageLoadState } from "../utils/use-image-load-state";
89

910
function FeedCardImage({ src }: { src: string }) {
1011
const canvasRef = useRef<HTMLCanvasElement>(null);
11-
const [loaded, setLoaded] = useState(false);
12-
const [failed, setFailed] = useState(false);
1312
const { src: cleanSrc, blurhash, width, height } = parseImageUrlMetadata(src);
13+
const { failed, imageRef, loaded, onError, onLoad } = useImageLoadState(cleanSrc);
1414
const aspectRatio = width && height ? `${width} / ${height}` : undefined;
1515

16-
useEffect(() => {
17-
setLoaded(false);
18-
setFailed(false);
19-
}, [src]);
20-
2116
useEffect(() => {
2217
if (!blurhash || !canvasRef.current) {
2318
return;
@@ -30,7 +25,10 @@ function FeedCardImage({ src }: { src: string }) {
3025
}, [blurhash]);
3126

3227
return (
33-
<div className="relative flex flex-row items-center mb-2 overflow-hidden rounded-xl" style={{ aspectRatio }}>
28+
<div
29+
className="relative mb-2 flex max-h-80 w-full flex-row items-center overflow-hidden rounded-xl"
30+
style={{ aspectRatio }}
31+
>
3432
{blurhash && !loaded ? (
3533
<canvas
3634
ref={canvasRef}
@@ -39,18 +37,13 @@ function FeedCardImage({ src }: { src: string }) {
3937
/>
4038
) : null}
4139
<img
40+
ref={imageRef}
4241
src={cleanSrc}
4342
alt=""
4443
width={width}
4544
height={height}
46-
onLoad={() => {
47-
setLoaded(true);
48-
setFailed(false);
49-
}}
50-
onError={() => {
51-
setLoaded(false);
52-
setFailed(true);
53-
}}
45+
onLoad={onLoad}
46+
onError={onError}
5447
className={`absolute inset-0 h-full w-full object-cover object-center hover:scale-105 translation duration-300 ${blurhash && (!loaded || failed) ? "opacity-0" : "opacity-100"
5548
}`}
5649
/>

‎client/src/components/markdown.tsx‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import "katex/dist/katex.min.css";
2-
import React, { cloneElement, isValidElement, useEffect, useMemo, useRef, useState } from "react";
2+
import React, { cloneElement, isValidElement, useEffect, useMemo, useRef } from "react";
33
import ReactMarkdown from "react-markdown";
44
import { Prism as SyntaxHighlighter } from "react-syntax-highlighter";
55
import {
@@ -20,6 +20,7 @@ import "yet-another-react-lightbox/styles.css";
2020
import { drawBlurhashToCanvas } from "../utils/blurhash";
2121
import { useColorMode } from "../utils/darkModeUtils";
2222
import { parseImageUrlMetadata } from "../utils/image-upload";
23+
import { useImageLoadState } from "../utils/use-image-load-state";
2324

2425

2526
const countNewlinesBeforeNode = (text: string, offset: number) => {
@@ -64,17 +65,11 @@ function MarkdownImage({
6465
className?: string;
6566
}) {
6667
const canvasRef = useRef<HTMLCanvasElement>(null);
67-
const [loaded, setLoaded] = useState(false);
68-
const [failed, setFailed] = useState(false);
6968
const { src: cleanSrc, blurhash, width, height } = parseImageUrlMetadata(src);
69+
const { failed, imageRef, loaded, onError, onLoad } = useImageLoadState(cleanSrc);
7070
const roundedClass = rounded ? "rounded-xl" : "";
7171
const aspectRatio = width && height ? `${width} / ${height}` : undefined;
7272

73-
useEffect(() => {
74-
setLoaded(false);
75-
setFailed(false);
76-
}, [src]);
77-
7873
useEffect(() => {
7974
if (!blurhash || !canvasRef.current) {
8075
return;
@@ -99,21 +94,16 @@ function MarkdownImage({
9994
/>
10095
) : null}
10196
<img
97+
ref={imageRef}
10298
src={cleanSrc}
10399
alt={alt}
104100
width={width}
105101
height={height}
106102
onClick={() => {
107103
show(cleanSrc);
108104
}}
109-
onLoad={() => {
110-
setLoaded(true);
111-
setFailed(false);
112-
}}
113-
onError={() => {
114-
setLoaded(false);
115-
setFailed(true);
116-
}}
105+
onLoad={onLoad}
106+
onError={onError}
117107
className={`mx-auto max-w-full cursor-zoom-in transition-opacity ${roundedClass} ${className || ""} ${
118108
blurhash && (!loaded || failed) ? "opacity-0" : "opacity-100"
119109
}`}

‎client/src/test/setup.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,14 @@
1-
import '@testing-library/jest-dom'
1+
import { JSDOM } from "jsdom";
2+
import '@testing-library/jest-dom'
3+
4+
if (typeof document === "undefined") {
5+
const { window } = new JSDOM("<!doctype html><html><body></body></html>");
6+
7+
Object.assign(globalThis, {
8+
document: window.document,
9+
HTMLElement: window.HTMLElement,
10+
HTMLImageElement: window.HTMLImageElement,
11+
navigator: window.navigator,
12+
window,
13+
});
14+
}

‎client/src/types/jsdom.d.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
declare module "jsdom" {
2+
export class JSDOM {
3+
constructor(html?: string, options?: unknown);
4+
window: Window & typeof globalThis;
5+
}
6+
}

‎client/src/utils/__tests__/use-image-load-state.test.tsx‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import "../../test/setup";
2+
import { render, waitFor } from "@testing-library/react";
3+
import type { MutableRefObject } from "react";
4+
import { describe, expect, it } from "vitest";
5+
import { useImageLoadState } from "../use-image-load-state";
6+
7+
function TestImage({ src, complete, naturalWidth }: { src: string; complete: boolean; naturalWidth: number }) {
8+
const { imageRef, loaded, failed, onLoad, onError } = useImageLoadState(src);
9+
10+
return (
11+
<>
12+
<div data-testid="status">{JSON.stringify({ failed, loaded })}</div>
13+
<img
14+
ref={(node) => {
15+
(imageRef as MutableRefObject<HTMLImageElement | null>).current = node;
16+
if (!node) {
17+
return;
18+
}
19+
Object.defineProperty(node, "complete", {
20+
configurable: true,
21+
get: () => complete,
22+
});
23+
Object.defineProperty(node, "naturalWidth", {
24+
configurable: true,
25+
get: () => naturalWidth,
26+
});
27+
}}
28+
src={src}
29+
alt=""
30+
onLoad={onLoad}
31+
onError={onError}
32+
/>
33+
</>
34+
);
35+
}
36+
37+
describe("useImageLoadState", () => {
38+
it("marks a cached image as loaded after src changes", async () => {
39+
const { getByTestId, rerender } = render(<TestImage src="https://example.com/a.png" complete={false} naturalWidth={0} />);
40+
41+
expect(getByTestId("status")).toHaveTextContent('{"failed":false,"loaded":false}');
42+
43+
rerender(<TestImage src="https://example.com/b.png" complete={true} naturalWidth={640} />);
44+
45+
await waitFor(() => {
46+
expect(getByTestId("status")).toHaveTextContent('{"failed":false,"loaded":true}');
47+
});
48+
});
49+
50+
it("marks a completed broken image as failed", async () => {
51+
const { getByTestId } = render(<TestImage src="https://example.com/broken.png" complete={true} naturalWidth={0} />);
52+
53+
await waitFor(() => {
54+
expect(getByTestId("status")).toHaveTextContent('{"failed":true,"loaded":false}');
55+
});
56+
});
57+
});

‎client/src/utils/use-image-load-state.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { useEffect, useRef, useState } from "react";
2+
3+
export function useImageLoadState(src?: string) {
4+
const imageRef = useRef<HTMLImageElement>(null);
5+
const [loaded, setLoaded] = useState(false);
6+
const [failed, setFailed] = useState(false);
7+
8+
useEffect(() => {
9+
setLoaded(false);
10+
setFailed(false);
11+
12+
const image = imageRef.current;
13+
if (!src || !image || !image.complete) {
14+
return;
15+
}
16+
17+
if (image.naturalWidth > 0) {
18+
setLoaded(true);
19+
return;
20+
}
21+
22+
setFailed(true);
23+
}, [src]);
24+
25+
return {
26+
failed,
27+
imageRef,
28+
loaded,
29+
onError: () => {
30+
setLoaded(false);
31+
setFailed(true);
32+
},
33+
onLoad: () => {
34+
setLoaded(true);
35+
setFailed(false);
36+
},
37+
};
38+
}

0 commit comments

Comments
 (0)