Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/desktop/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useEffect, useState } from 'react';
import { tauriGateway } from './lib/tauriGateway';
import { PdfViewer } from './components/PdfViewer';
import { ErrorBoundary } from './components/ErrorBoundary';
import { createViewerRenderer } from './lib/viewerRenderer';
import type { ExtractedPageTextRecord, RecentDocumentView } from '@gitplant/shared-types';

export function App() {
Expand Down Expand Up @@ -90,6 +91,7 @@ export function App() {
bytes={bytes}
overlayBytes={overlayBytes}
title={title}
rendererFactory={createViewerRenderer}
onPageCount={(n) => {
if (currentRevisionId) void tauriGateway.updatePageCount(currentRevisionId, n);
}}
Expand Down
3 changes: 3 additions & 0 deletions apps/desktop/src/components/PdfViewer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ describe('PdfViewer states', () => {

it('ui-level code avoids direct pdfjs imports', () => {
const appSource = fs.readFileSync(path.resolve(__dirname, '../App.tsx'), 'utf8');
const viewerSource = fs.readFileSync(path.resolve(__dirname, './PdfViewer.tsx'), 'utf8');
expect(appSource).not.toContain('pdfjs-dist');
expect(appSource).not.toContain('@gitplant/viewer-pdfjs');
expect(viewerSource).not.toContain('pdfjs-dist');
expect(viewerSource).not.toContain('@gitplant/viewer-pdfjs');
});
});
17 changes: 9 additions & 8 deletions apps/desktop/src/components/PdfViewer.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { useEffect, useMemo, useState } from 'react';
import { createBaseRenderScene, withOverlayPdfLayer } from '@gitplant/viewer-core';
import { PdfjsRendererAdapter } from '@gitplant/viewer-pdfjs';
import { createBaseRenderScene, withOverlayPdfLayer, type ViewerRenderer } from '@gitplant/viewer-core';

type Props = {
bytes: Uint8Array | null;
title: string;
overlayBytes?: Uint8Array | null;
onPageCount?: (n: number) => void;
rendererFactory?: () => ViewerRenderer;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Make renderer factory mandatory or supply a default

rendererFactory is declared optional, but when it is omitted and bytes is non-null the component now renders "Viewer renderer unavailable." instead of displaying the PDF, which is a runtime regression from the previous behavior and easy to trigger in any caller that forgets the new prop. Because the prop is optional, TypeScript will not catch those call sites; either require the prop in Props or provide an internal fallback renderer.

Useful? React with 👍 / 👎.

};

export function PdfViewer({ bytes, title, overlayBytes, onPageCount }: Props) {
const renderer = useMemo(() => new PdfjsRendererAdapter(), []);
export function PdfViewer({ bytes, title, overlayBytes, onPageCount, rendererFactory }: Props) {
const renderer = useMemo(() => (rendererFactory ? rendererFactory() : null), [rendererFactory]);
const [page, setPage] = useState(1);
const [pageCount, setPageCount] = useState(0);
const [zoom, setZoom] = useState(1);
Expand All @@ -20,7 +20,7 @@ export function PdfViewer({ bytes, title, overlayBytes, onPageCount }: Props) {

useEffect(() => {
void (async () => {
if (!bytes) return;
if (!bytes || !renderer) return;
setState('loading');
try {
await renderer.openDocument(bytes);
Expand All @@ -38,13 +38,13 @@ export function PdfViewer({ bytes, title, overlayBytes, onPageCount }: Props) {
}
})();
return () => {
void renderer.closeDocument();
if (renderer) void renderer.closeDocument();
};
}, [bytes, overlayBytes]);
}, [bytes, overlayBytes, renderer]);

useEffect(() => {
void (async () => {
if (!bytes || state === 'error' || pageCount === 0) return;
if (!bytes || !renderer || state === 'error' || pageCount === 0) return;
setState('loading');
try {
const nextScene = {
Expand All @@ -63,6 +63,7 @@ export function PdfViewer({ bytes, title, overlayBytes, onPageCount }: Props) {
}, [page, zoom]);

if (!bytes) return <div>Select a PDF to view.</div>;
if (!renderer) return <div>Viewer renderer unavailable.</div>;
if (state === 'error') return <div role="alert">Failed to render document.</div>;
return (
<div>
Expand Down
6 changes: 6 additions & 0 deletions apps/desktop/src/lib/viewerRenderer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import type { ViewerRenderer } from '@gitplant/viewer-core';
import { PdfjsRendererAdapter } from '@gitplant/viewer-pdfjs';

export function createViewerRenderer(): ViewerRenderer {
return new PdfjsRendererAdapter();
}
21 changes: 18 additions & 3 deletions apps/desktop/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,24 @@
"strict": true,
"baseUrl": ".",
"paths": {
"@gitplant/*": ["../../packages/*/src"]
"@gitplant/*": [
"../../packages/*/src"
]
},
"types": ["vitest/globals", "@testing-library/jest-dom"]
"types": [
"vite/client"
],
"skipLibCheck": true,
"lib": [
"ES2023",
"DOM"
]
},
"include": ["src", "vite.config.ts", "vitest.setup.ts"]
"include": [
"src"
],
"exclude": [
"src/**/*.test.ts",
"src/**/*.test.tsx"
]
}
7 changes: 5 additions & 2 deletions apps/desktop/vite.config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { defineConfig } from 'vite';
import { defineConfig } from 'vitest/config';
import react from '@vitejs/plugin-react';

export default defineConfig({ plugins: [react()], test: { environment: 'jsdom', setupFiles: './vitest.setup.ts' } });
export default defineConfig({
plugins: [react()],
test: { environment: 'jsdom', setupFiles: './vitest.setup.ts', globals: true }
});
2 changes: 1 addition & 1 deletion apps/desktop/vitest.setup.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
import '@testing-library/jest-dom';
import '@testing-library/jest-dom/vitest';