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
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
37 changes: 36 additions & 1 deletion test/pdfjsScriptingBoundary.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand 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'
)
})
Loading