From 259972e2c41c39b8e9dd88f97f95ffa4ee5768a1 Mon Sep 17 00:00:00 2001 From: mrsibe Date: Sat, 26 Sep 2026 00:52:14 +0800 Subject: [PATCH] fix(reader): render the PDF reader with the pdfjs legacy build `pdfjs-dist` 6.3.289 calls `Map.prototype.getOrInsertComputed` (and `getOrInsert`) throughout the API build and the worker, but those shipped in Chromium 145 and Electron 39 is Chromium 142. The renderer imported the modern build, so the first `page.render()` threw TypeError: this[#methodPromises].getOrInsertComputed is not a function from `WorkerTransport.getOptionalContentConfig`, and every page painted blank. The main-process loader already imports `pdfjs-dist/legacy/build/pdf.mjs`, whose core-js polyfills install those methods, so text extraction was unaffected while the reader was broken. Point the reader and its worker at the same legacy build. Switch back to `pdfjs-dist/build/*` once the app runs on Chromium >= 145. The reader path slipped through #124 (the upgrade for #121), whose own message records the rendering path as unverified manual QA. `test/pdfjsScriptingBoundary.test.ts` now fails if any source file imports the modern build, and its self-check accepts the legacy subpath. Verified: `npm test` (271 pass), `npm run typecheck`, `npm run build`; the built `out/renderer/assets/pdf.worker.min-*.mjs` carries the polyfill (`target:"Map",proto:!0`) and the renderer bundle references that legacy worker. The new guard was falsified by temporarily restoring the modern import, which fails it as intended. Not verified: canvas render, text-layer selection, bbox highlighting and zoom under a real window - that needs manual QA, and is #125's job. --- .../source/reader/PdfSourceReader.tsx | 11 +++++- test/pdfjsScriptingBoundary.test.ts | 37 ++++++++++++++++++- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/components/notebook/source/reader/PdfSourceReader.tsx b/src/renderer/src/components/notebook/source/reader/PdfSourceReader.tsx index c9f9162..f56fc07 100644 --- a/src/renderer/src/components/notebook/source/reader/PdfSourceReader.tsx +++ b/src/renderer/src/components/notebook/source/reader/PdfSourceReader.tsx @@ -9,9 +9,16 @@ import { type CSSProperties, type ReactElement } from 'react' -import * as pdfjs from 'pdfjs-dist' +// The legacy build is required, not a size trade-off: pdfjs-dist 6.x calls +// `Map.prototype.getOrInsertComputed` throughout the API and the worker, and that +// proposal only shipped in Chromium 145. Electron 39 is Chromium 142, so the modern +// build throws `getOrInsertComputed is not a function` on the first `page.render()` +// and every page paints blank (#125). The legacy build bundles the core-js polyfills +// for it (`Map`/`WeakMap`, `getOrInsert`/`getOrInsertComputed`), which the main-process +// loader already relies on. Move back to `pdfjs-dist/build/*` once Chromium >= 145. +import * as pdfjs from 'pdfjs-dist/legacy/build/pdf.mjs' import type { PDFDocumentLoadingTask, PDFDocumentProxy, RenderTask } from 'pdfjs-dist' -import workerUrl from 'pdfjs-dist/build/pdf.worker.min.mjs?url' +import workerUrl from 'pdfjs-dist/legacy/build/pdf.worker.min.mjs?url' import { useTranslation } from 'react-i18next' import { Minus, Plus, Loader2 } from 'lucide-react' import { documentUrl } from '../../../../../../shared/utils/documentUrl' diff --git a/test/pdfjsScriptingBoundary.test.ts b/test/pdfjsScriptingBoundary.test.ts index dabbceb..277da19 100644 --- a/test/pdfjsScriptingBoundary.test.ts +++ b/test/pdfjsScriptingBoundary.test.ts @@ -109,6 +109,39 @@ test('`enableScripting` is never left on', () => { ) }) +test('no source file imports the modern pdfjs build, which requires Chromium 145', () => { + // pdfjs-dist 6.x calls `Map.prototype.getOrInsertComputed` (and `getOrInsert`) in the + // main-thread API and in the worker. That proposal shipped in Chromium 145; Electron 39 + // is Chromium 142, so the modern build dies at the first `page.render()` with + // `getOrInsertComputed is not a function` and every page paints blank (#125). The legacy + // build bundles the core-js polyfills for it, which is why every runtime import must go + // through `pdfjs-dist/legacy/build/*`. Lift this guard once the app runs on Chromium >= 145. + const offenders: string[] = [] + + for (const file of sourceFiles()) { + const source = readFileSync(file, 'utf8') + for (const line of source.split('\n')) { + // Type-only imports emit nothing, so `import type ... from 'pdfjs-dist'` is fine. + // It is the value import (`import * as pdfjs from 'pdfjs-dist'`) and the + // `pdfjs-dist/build/*` subpaths that pull the modern runtime in. + if (/^\s*import\s+type\b/.test(line)) continue + for (const specifier of importSpecifiers(line)) { + if (specifier === 'pdfjs-dist' || specifier.startsWith('pdfjs-dist/build/')) { + offenders.push(`${file} imports ${specifier}`) + } + } + } + } + + assert.deepEqual( + offenders, + [], + 'The modern pdfjs build needs `Map.prototype.getOrInsertComputed`, which Electron 39 ' + + '(Chromium 142) does not implement, so it throws on the first render (#125). Use ' + + "'pdfjs-dist/legacy/build/*' instead; the legacy build ships the polyfill." + ) +}) + test('the guard actually scans the source tree', () => { // A boundary test that silently scans nothing is worse than no test: it would report the // invariant as enforced while checking no files at all. @@ -130,7 +163,9 @@ test('the guard actually scans the source tree', () => { ) assert.ok(reader) assert.ok( - importSpecifiers(readFileSync(reader, 'utf8')).includes('pdfjs-dist'), + importSpecifiers(readFileSync(reader, 'utf8')).some((specifier) => + specifier.startsWith('pdfjs-dist') + ), 'import extraction found no pdfjs specifier in the reader' ) })