Skip to content

Commit 0c3b2a4

Browse files
authored
🐛 Prevent stale TDD image loads (#230)
## Why this change In TDD mode, baseline/current/diff images are overwritten in place under stable `/images/*` URLs. That means the reporter can receive fresh comparison data while the browser still shows stale cached image bytes. The result feels like an SSE issue, but the primary problem is image caching. ## What changed - Added a reporter utility that appends a version query param to local `/images/*` URLs. - Added a report-data normalizer so comparisons always have a timestamp fallback for cache-busting (`comparison.timestamp ?? reportData.timestamp`). - Wired cache-busted URLs into comparison image render paths (viewer, fullscreen queue thumbnails, screenshot list, screenshot display). - Added no-cache response headers for image responses in the assets router. - Added focused unit tests for URL rewriting behavior and report-data timestamp normalization. ## Test plan - `node --test tests/server/routers/assets.test.js tests/reporter/utils/image-url.test.js tests/reporter/utils/report-data.test.js` - `npm run build:reporter`
1 parent 568d2a2 commit 0c3b2a4

12 files changed

Lines changed: 265 additions & 21 deletions

File tree

src/reporter/src/api/client.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
* - api.auth.* - Authentication
1010
*/
1111

12+
import { normalizeReportData } from '../utils/report-data.js';
13+
1214
/**
1315
* Make a JSON API request
1416
* @param {string} url - Request URL
@@ -45,7 +47,8 @@ export const tdd = {
4547
* @returns {Promise<Object|null>}
4648
*/
4749
async getReportData() {
48-
return fetchJson('/api/report-data');
50+
let data = await fetchJson('/api/report-data');
51+
return normalizeReportData(data);
4952
},
5053

5154
/**

src/reporter/src/components/comparison/comparison-viewer.jsx

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
} from '@vizzly-testing/observatory';
66
import { useCallback, useMemo, useState } from 'react';
77
import { VIEW_MODES } from '../../utils/constants.js';
8+
import { withImageVersion } from '../../utils/image-url.js';
89

910
/**
1011
* Comparison Viewer for inline card display
@@ -36,12 +37,20 @@ export default function ComparisonViewer({ comparison, viewMode }) {
3637
[comparison]
3738
);
3839

39-
// Build image URLs - no memoization needed, object creation is cheap
40-
const imageUrls = {
41-
current: comparison.current,
42-
baseline: comparison.baseline,
43-
diff: comparison.diff,
44-
};
40+
// Build image URLs once per comparison update.
41+
const imageUrls = useMemo(
42+
() => ({
43+
current: withImageVersion(comparison.current, comparison.timestamp),
44+
baseline: withImageVersion(comparison.baseline, comparison.timestamp),
45+
diff: withImageVersion(comparison.diff, comparison.timestamp),
46+
}),
47+
[
48+
comparison.current,
49+
comparison.baseline,
50+
comparison.diff,
51+
comparison.timestamp,
52+
]
53+
);
4554

4655
// For new screenshots, just show the current image (no baseline exists yet)
4756
if (comparison.status === 'new' || comparison.status === 'baseline-created') {
@@ -52,7 +61,7 @@ export default function ComparisonViewer({ comparison, viewMode }) {
5261
First screenshot - creating new baseline
5362
</p>
5463
<img
55-
src={comparison.current}
64+
src={imageUrls.current}
5665
alt="New baseline screenshot"
5766
className="mx-auto max-w-full block"
5867
/>

src/reporter/src/components/comparison/fullscreen-viewer.jsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import {
4545
} from '@vizzly-testing/observatory';
4646
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
4747
import { VIEW_MODES } from '../../utils/constants.js';
48+
import { withImageVersion } from '../../utils/image-url.js';
4849
import { ScreenshotDisplay } from './screenshot-display.jsx';
4950

5051
/**
@@ -641,7 +642,10 @@ function FullscreenViewerInner({
641642
<QueueItem
642643
item={item}
643644
isActive={isActive}
644-
thumbnailUrl={item.current || item.baseline}
645+
thumbnailUrl={withImageVersion(
646+
item.current || item.baseline,
647+
item.timestamp
648+
)}
645649
onClick={() => onNavigate(item)}
646650
/>
647651
</div>

src/reporter/src/components/comparison/screenshot-display.jsx

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
ToggleMode,
77
} from '@vizzly-testing/observatory';
88
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
9+
import { withImageVersion } from '../../utils/image-url.js';
910

1011
/**
1112
* Unified Screenshot Display Component - matches Observatory architecture
@@ -123,14 +124,21 @@ export function ScreenshotDisplay({
123124
setImageLoadStates(prev => new Map(prev).set(imageKey, 'loaded'));
124125
}, []);
125126

126-
// Build image URLs from comparison object - no memoization needed, object creation is cheap
127-
const imageUrls = comparison
128-
? {
129-
current: comparison.current,
130-
baseline: comparison.baseline,
131-
diff: comparison.diff,
132-
}
133-
: {};
127+
// Build image URLs once per comparison update.
128+
const imageUrls = useMemo(
129+
() =>
130+
comparison
131+
? {
132+
current: withImageVersion(comparison.current, comparison.timestamp),
133+
baseline: withImageVersion(
134+
comparison.baseline,
135+
comparison.timestamp
136+
),
137+
diff: withImageVersion(comparison.diff, comparison.timestamp),
138+
}
139+
: {},
140+
[comparison]
141+
);
134142

135143
// Create a screenshot-like object for the comparison modes
136144
const screenshot = useMemo(() => {
@@ -213,7 +221,7 @@ export function ScreenshotDisplay({
213221
>
214222
{comparison && (
215223
<img
216-
src={comparison.current}
224+
src={imageUrls.current}
217225
alt={comparison.name || 'New screenshot'}
218226
className="block"
219227
onLoad={() => handleImageLoad(`current-${screenshot?.id}`)}

src/reporter/src/components/comparison/screenshot-list.jsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
XCircleIcon,
88
} from '@heroicons/react/24/outline';
99
import { useMemo } from 'react';
10+
import { withImageVersion } from '../../utils/image-url.js';
1011
import { Badge, Button } from '../design-system/index.js';
1112
import SmartImage from '../ui/smart-image.jsx';
1213

@@ -238,7 +239,10 @@ function ScreenshotGroupRow({
238239
}) {
239240
let { primary, hasChanges, hasNew, maxDiff } = group;
240241
let needsAction = hasChanges || hasNew;
241-
let thumbnailSrc = primary.current || primary.baseline;
242+
let thumbnailSrc = withImageVersion(
243+
primary.current || primary.baseline,
244+
primary.timestamp
245+
);
242246

243247
// Generate test ID from primary comparison
244248
let testId = `screenshot-group-${(primary.id || primary.signature || group.name).replace(/[^a-zA-Z0-9-]/g, '-')}`;

src/reporter/src/providers/sse-provider.jsx

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { useQueryClient } from '@tanstack/react-query';
22
import { createContext, useEffect, useRef, useState } from 'react';
33
import { queryKeys } from '../lib/query-keys.js';
4+
import {
5+
normalizeComparisonUpdate,
6+
normalizeReportData,
7+
} from '../utils/report-data.js';
48

59
export let SSE_STATE = {
610
CONNECTING: 'connecting',
@@ -57,7 +61,10 @@ export function SSEProvider({ enabled = true, children }) {
5761
eventSource.addEventListener('reportData', event => {
5862
try {
5963
let data = JSON.parse(event.data);
60-
queryClient.setQueryData(queryKeys.reportData(), data);
64+
queryClient.setQueryData(
65+
queryKeys.reportData(),
66+
normalizeReportData(data)
67+
);
6168
} catch {
6269
// Ignore parse errors
6370
}
@@ -66,9 +73,13 @@ export function SSEProvider({ enabled = true, children }) {
6673
// Incremental: single comparison added or changed
6774
eventSource.addEventListener('comparisonUpdate', event => {
6875
try {
69-
let comparison = JSON.parse(event.data);
76+
let incomingComparison = JSON.parse(event.data);
7077
queryClient.setQueryData(queryKeys.reportData(), old => {
7178
if (!old) return old;
79+
let comparison = normalizeComparisonUpdate(
80+
incomingComparison,
81+
old.timestamp
82+
);
7283
let comparisons = old.comparisons || [];
7384
let idx = comparisons.findIndex(c => c.id === comparison.id);
7485
if (idx >= 0) {
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* Add a version query param for local image URLs so updated screenshots
3+
* are re-fetched when report data changes.
4+
*
5+
* This is a no-op for non-local URLs.
6+
*/
7+
export let LOCAL_IMAGE_PREFIX = '/images/';
8+
9+
export function withImageVersion(url, version) {
10+
if (!url || typeof url !== 'string') {
11+
return url;
12+
}
13+
14+
// Only rewrite local TDD image paths.
15+
if (!url.startsWith(LOCAL_IMAGE_PREFIX)) {
16+
return url;
17+
}
18+
19+
if (version === null || version === undefined) {
20+
return url;
21+
}
22+
23+
let [path, queryString = ''] = url.split('?');
24+
let params = new URLSearchParams(queryString);
25+
params.set('v', String(version));
26+
let query = params.toString();
27+
28+
return query ? `${path}?${query}` : path;
29+
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/**
2+
* Ensure each comparison has a timestamp for image cache-busting.
3+
*/
4+
export function normalizeReportData(reportData) {
5+
if (!reportData || !Array.isArray(reportData.comparisons)) {
6+
return reportData;
7+
}
8+
9+
let needsNormalization = reportData.comparisons.some(
10+
comparison => comparison && comparison.timestamp == null
11+
);
12+
13+
if (!needsNormalization) {
14+
return reportData;
15+
}
16+
17+
let fallbackTimestamp = reportData.timestamp ?? Date.now();
18+
let comparisons = reportData.comparisons.map(comparison =>
19+
normalizeComparisonUpdate(comparison, fallbackTimestamp)
20+
);
21+
22+
return {
23+
...reportData,
24+
comparisons,
25+
};
26+
}
27+
28+
/**
29+
* Ensure a single comparison includes a timestamp.
30+
*/
31+
export function normalizeComparisonUpdate(comparison, fallbackTimestamp) {
32+
if (!comparison || comparison.timestamp != null) {
33+
return comparison;
34+
}
35+
36+
return {
37+
...comparison,
38+
timestamp: fallbackTimestamp ?? Date.now(),
39+
};
40+
}

src/server/routers/assets.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ export function createAssetsRouter() {
8484
if (existsSync(fullImagePath)) {
8585
try {
8686
const imageData = readFileSync(fullImagePath);
87+
// Images are rewritten in place between TDD runs, so disable browser caching.
88+
res.setHeader('Cache-Control', 'no-store, no-cache, must-revalidate');
89+
res.setHeader('Pragma', 'no-cache');
90+
res.setHeader('Expires', '0');
8791
sendFile(res, imageData, 'image/png');
8892
return true;
8993
} catch (error) {
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import assert from 'node:assert';
2+
import { describe, it } from 'node:test';
3+
import { withImageVersion } from '../../../src/reporter/src/utils/image-url.js';
4+
5+
describe('reporter/utils/image-url', () => {
6+
it('returns original value for non-string urls', () => {
7+
assert.strictEqual(withImageVersion(null, 1), null);
8+
assert.strictEqual(withImageVersion(undefined, 1), undefined);
9+
assert.strictEqual(withImageVersion(42, 1), 42);
10+
});
11+
12+
it('returns original url for non-local images', () => {
13+
let url = 'https://cdn.example.com/image.png';
14+
assert.strictEqual(withImageVersion(url, 123), url);
15+
});
16+
17+
it('returns original url when version is missing', () => {
18+
let url = '/images/current/homepage.png';
19+
assert.strictEqual(withImageVersion(url, null), url);
20+
assert.strictEqual(withImageVersion(url, undefined), url);
21+
});
22+
23+
it('appends v query param for local image urls', () => {
24+
assert.strictEqual(
25+
withImageVersion('/images/current/homepage.png', 123),
26+
'/images/current/homepage.png?v=123'
27+
);
28+
});
29+
30+
it('supports zero as a valid cache-busting version', () => {
31+
assert.strictEqual(
32+
withImageVersion('/images/current/homepage.png', 0),
33+
'/images/current/homepage.png?v=0'
34+
);
35+
});
36+
37+
it('appends v query param using ampersand when query already exists', () => {
38+
assert.strictEqual(
39+
withImageVersion('/images/current/homepage.png?mode=thumb', 456),
40+
'/images/current/homepage.png?mode=thumb&v=456'
41+
);
42+
});
43+
44+
it('replaces existing v query param instead of duplicating', () => {
45+
assert.strictEqual(
46+
withImageVersion('/images/current/homepage.png?v=old&mode=thumb', 456),
47+
'/images/current/homepage.png?v=456&mode=thumb'
48+
);
49+
});
50+
51+
it('encodes non-numeric version values', () => {
52+
assert.strictEqual(
53+
withImageVersion('/images/current/homepage.png', 'run 1'),
54+
'/images/current/homepage.png?v=run+1'
55+
);
56+
});
57+
});

0 commit comments

Comments
 (0)