Skip to content

Commit 2c8a1f6

Browse files
committed
Fix bug caused by the @pierre/diffs update.
1 parent 75c1615 commit 2c8a1f6

4 files changed

Lines changed: 80 additions & 28 deletions

File tree

core/__tests__/ReviewCodeView-scroll.test.tsx

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* @vitest-environment jsdom
33
*/
44

5+
import { CodeView, preloadHighlighter } from '@pierre/diffs';
56
import { act, useCallback, useState } from 'react';
67
import { createRoot, type Root } from 'react-dom/client';
78
import { beforeEach, expect, test, vi } from 'vite-plus/test';
@@ -1282,6 +1283,50 @@ test('hunk navigation skips pull request description source context', async () =
12821283
);
12831284
});
12841285

1286+
test('markdown preview files survive layout updates in the real virtualized renderer', async () => {
1287+
await preloadHighlighter({ langs: ['text'], themes: ['github-dark'] });
1288+
const file = createLoadedMarkdownFile('# Readme\n', 'markdown-layout');
1289+
await using app = await renderReact(<ReviewCodeViewHarness files={[file]} />);
1290+
const root = document.createElement('div');
1291+
root.className = 'code-view';
1292+
document.body.append(root);
1293+
const viewer = new CodeView<unknown>({
1294+
disableErrorHandling: true,
1295+
theme: 'github-dark',
1296+
themeType: 'dark',
1297+
});
1298+
try {
1299+
viewer.setup(root);
1300+
viewer.setItems(codeViewMock.lastItems);
1301+
viewer.render(true);
1302+
expect(viewer.getRenderedItems()).toHaveLength(1);
1303+
await app.rerender(<ReviewCodeViewHarness collapsed={new Set([file.path])} files={[file]} />);
1304+
expect(codeViewMock.lastItems[0]?.collapsed).toBe(true);
1305+
viewer.setItems(codeViewMock.lastItems);
1306+
expect(() => viewer.render(true)).not.toThrow();
1307+
expect(viewer.getRenderedItems()).toHaveLength(1);
1308+
await app.rerender(<ReviewCodeViewHarness files={[file]} />);
1309+
viewer.setItems(codeViewMock.lastItems);
1310+
expect(() => viewer.render(true)).not.toThrow();
1311+
1312+
const updatedFile = createLoadedMarkdownFile('# Updated readme\n', 'markdown-updated');
1313+
await app.rerender(<ReviewCodeViewHarness files={[updatedFile]} />);
1314+
expect(codeViewMock.lastItems[0]?.annotations).toEqual(
1315+
expect.arrayContaining([
1316+
expect.objectContaining({
1317+
metadata: expect.objectContaining({ contents: '# Updated readme\n' }),
1318+
}),
1319+
]),
1320+
);
1321+
viewer.setItems(codeViewMock.lastItems);
1322+
expect(() => viewer.render(true)).not.toThrow();
1323+
expect(viewer.getRenderedItems()).toHaveLength(1);
1324+
} finally {
1325+
viewer.cleanUp();
1326+
root.remove();
1327+
}
1328+
});
1329+
12851330
test('read-only markdown previews trigger CodeView layout remeasurement after height change', async () => {
12861331
const markdownFile = createLoadedMarkdownFile(
12871332
'![diagram](https://example.com/diagram.png)\n',

core/__tests__/helpers/review-code-view.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import type { ChangedFile, ReviewSource } from '../../types.ts';
88
export type { ReviewDiffBlock } from '../../app/components/ReviewCodeView.tsx';
99

1010
const codeViewMockState = vi.hoisted(() => ({
11-
lastItems: [] as ReadonlyArray<{ id: string; type: string; version?: unknown }>,
11+
lastItems: [] as ReadonlyArray<CodeViewItem<unknown>>,
1212
lastOptions: null as Record<string, unknown> | null,
1313
postRenderNodes: [] as Array<HTMLElement>,
1414
renderCount: 0,

core/app/components/ReviewCodeView.tsx

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ import type {
120120
ReviewAuthor,
121121
ReviewSource,
122122
} from '../../types.ts';
123+
import { useCodeViewPlaceholderFile } from '../hooks/useCodeViewPlaceholderFile.ts';
123124
import { Avatar } from './Avatar.tsx';
124125
import { Button } from './Button.tsx';
125126
import { DefinitionPopover } from './DefinitionPopover.tsx';
@@ -2616,6 +2617,7 @@ export function ReviewCodeView({
26162617
wordWrap: boolean;
26172618
}) {
26182619
const codeViewRef = useRef<CodeViewHandle<ReviewAnnotationMetadata, undefined>>(null);
2620+
const getPlaceholderFile = useCodeViewPlaceholderFile();
26192621
const markdownEditorRefs = useRef(new Map<string, MarkdownDocumentEditorHandle>());
26202622
const refreshingMarkdownSectionsRef = useRef(new Set<string>());
26212623
const deferredTimersRef = useRef<Set<number>>(new Set());
@@ -2812,12 +2814,7 @@ export function ReviewCodeView({
28122814
} satisfies LineAnnotation<ReviewAnnotationMetadata>,
28132815
],
28142816
collapsed: false,
2815-
file: {
2816-
cacheKey: `walkthrough-header:${block.id}`,
2817-
contents: ' ',
2818-
lang: 'text',
2819-
name: headerId,
2820-
},
2817+
file: getPlaceholderFile(headerId),
28212818
id: headerId,
28222819
type: 'file',
28232820
// Selection stays out of the version: a scroll-driven current-stop
@@ -2974,12 +2971,7 @@ export function ReviewCodeView({
29742971
} satisfies LineAnnotation<ReviewAnnotationMetadata>,
29752972
],
29762973
collapsed: isCollapsed,
2977-
file: {
2978-
cacheKey: `image-preview:${file.fingerprint}:${section.id}`,
2979-
contents: ' ',
2980-
lang: 'text',
2981-
name: file.path,
2982-
},
2974+
file: getPlaceholderFile(file.path),
29832975
id,
29842976
type: 'file',
29852977
version: getItemVersion(
@@ -3010,14 +3002,7 @@ export function ReviewCodeView({
30103002
} satisfies LineAnnotation<ReviewAnnotationMetadata>,
30113003
],
30123004
collapsed: isCollapsed,
3013-
file: {
3014-
cacheKey: `markdown-preview:${section.newFile?.cacheKey ?? file.fingerprint}:${
3015-
markdownPreview.contents.length
3016-
}:${markdownPreviewAddedLinesDigest}`,
3017-
contents: ' ',
3018-
lang: 'text',
3019-
name: file.path,
3020-
},
3005+
file: getPlaceholderFile(file.path),
30213006
id,
30223007
type: 'file',
30233008
version: getItemVersion(
@@ -3068,6 +3053,7 @@ export function ReviewCodeView({
30683053
diffStyle,
30693054
expandedReviewKeys,
30703055
forceExpandedPaths,
3056+
getPlaceholderFile,
30713057
imagePreviewLayoutPassBySection,
30723058
isReadOnly,
30733059
itemVersionByKey,
@@ -3091,17 +3077,15 @@ export function ReviewCodeView({
30913077
return [
30923078
{
30933079
collapsed: true,
3094-
file: {
3095-
cacheKey: sourceDescriptionItemId,
3096-
contents: '',
3097-
lang: 'text',
3098-
name: shouldShowCommitMessage ? 'commit-message.md' : 'source-description.md',
3099-
},
3080+
file: getPlaceholderFile(
3081+
shouldShowCommitMessage ? 'commit-message.md' : 'source-description.md',
3082+
'',
3083+
),
31003084
id: sourceDescriptionItemId,
31013085
type: 'file',
31023086
},
31033087
];
3104-
}, [items, shouldShowCommitMessage, sourceDescriptionItemId]);
3088+
}, [getPlaceholderFile, items, shouldShowCommitMessage, sourceDescriptionItemId]);
31053089

31063090
const clearCommentLineHighlight = useCallback(() => {
31073091
codeViewRef.current?.clearSelectedLines();
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import type { FileContents } from '@pierre/diffs';
2+
import { useState } from 'react';
3+
4+
export function useCodeViewPlaceholderFile() {
5+
const [getPlaceholderFile] = useState(() => {
6+
const files = new Map<string, FileContents>();
7+
8+
return (name: string, contents: '' | ' ' = ' '): FileContents => {
9+
const cacheKey = `placeholder:${contents.length}:${name}`;
10+
let file = files.get(cacheKey);
11+
if (!file) {
12+
file = { cacheKey, contents, lang: 'text', name };
13+
files.set(cacheKey, file);
14+
}
15+
return file;
16+
};
17+
});
18+
19+
// VirtualizedFile prepares layout using the cached file object, then checks
20+
// its identity against the rendered file. Keep these objects for the viewer's
21+
// lifetime; preview content and layout changes belong to annotations/version.
22+
return getPlaceholderFile;
23+
}

0 commit comments

Comments
 (0)