fix: ensure worker matches API version#6
Conversation
WalkthroughUpdated pdf.js worker path in pdf2img TypeScript module and bumped pdfjs-dist dependency version. Minor formatting changes; no public API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
package.json (1)
5-10: Automate syncing the worker into public/ to avoid human errorTo keep the worker and library in lockstep, add a postinstall script that copies the worker from node_modules to public/. This avoids drift and removes a manual step.
Example using a small Node one-liner (cross-platform):
"scripts": { "build": "react-router build", "dev": "react-router dev", "start": "react-router-serve ./build/server/index.js", - "typecheck": "react-router typegen && tsc" + "typecheck": "react-router typegen && tsc", + "postinstall": "node -e \"const fs=require('fs');const path=require('path');const src=path.join('node_modules','pdfjs-dist','build','pdf.worker.mjs');const dstDir='public';fs.mkdirSync(dstDir,{recursive:true});fs.copyFileSync(src,path.join(dstDir,'pdf.worker.mjs'));console.log('Synced pdf.worker.mjs to public/.')\"" },Would you like me to generate a separate scripts/sync-pdf-worker.cjs with better error handling and version checks?
app/lib/pdf2img.ts (4)
12-25: Alternative: bind the worker directly from the installed package (no public copy needed)This avoids manual file management entirely and guarantees version alignment with pdfjs-dist.
Replace workerSrc with workerPort:
// In the loader, after importing lib lib.GlobalWorkerOptions.workerPort = new Worker( new URL("pdfjs-dist/build/pdf.worker.mjs", import.meta.url), { type: "module" } ); // No need to set workerSrc or copy files to public/I can provide a full diff if you choose this approach.
36-41: Make rendering scale configurable to avoid large memory usage on big PDFsA fixed scale of 4 can create very large canvases and memory spikes. Consider:
- Using devicePixelRatio to adapt to screens.
- Accepting a scale parameter with a safe default.
Example:
const scale = Math.min(2 * window.devicePixelRatio, 4); const viewport = page.getViewport({ scale });
64-67: Avoid leaking object URLsURL.createObjectURL needs a corresponding URL.revokeObjectURL when the image is no longer needed. At minimum, document this for the caller.
If you want to enforce it here, you could return a small helper:
resolve({ imageUrl: url, file: imageFile, // caller can optionally call this revoke: () => URL.revokeObjectURL(url), });Or ensure the consuming component revokes the URL on unmount/cleanup.
8-9: isLoading is currently unused; consider removing to reduce state churnSince the flag isn’t read anywhere, it can be dropped. If you plan to surface loading state externally later, keep it; otherwise, simplify.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
app/lib/pdf2img.ts(1 hunks)package.json(1 hunks)
🔇 Additional comments (1)
app/lib/pdf2img.ts (1)
21-21: Worker file exists and version matches pdfjs-dist — no change neededpublic/pdf.worker.mjs is present and its header reports "pdfjsVersion = 5.4.54", and package.json lists "pdfjs-dist": "^5.4.54", so the runtime path "/pdf.worker.mjs" used in app/lib/pdf2img.ts is correct.
- Confirmed locations:
- app/lib/pdf2img.ts — lib.GlobalWorkerOptions.workerSrc = "/pdf.worker.mjs"
- public/pdf.worker.mjs — header: pdfjsVersion = 5.4.54
- package.json — "pdfjs-dist": "^5.4.54"
| if (pdfjsLib) return pdfjsLib; | ||
| if (loadPromise) return loadPromise; | ||
|
|
||
| isLoading = true; | ||
| // @ts-expect-error - pdfjs-dist/build/pdf.mjs is not a module | ||
| loadPromise = import("pdfjs-dist/build/pdf.mjs").then((lib) => { | ||
| // Set the worker source to use local file | ||
| lib.GlobalWorkerOptions.workerSrc = "/pdf.worker.min.mjs"; | ||
| pdfjsLib = lib; | ||
| isLoading = false; | ||
| return lib; | ||
| }); | ||
| isLoading = true; | ||
| // @ts-expect-error - pdfjs-dist/build/pdf.mjs is not a module | ||
| loadPromise = import("pdfjs-dist/build/pdf.mjs").then((lib) => { | ||
| // Set the worker source to use local file | ||
| // IMPORTANT: Must match the version of pdfjs-dist installed (e.g., v5.4.54) | ||
| // Get pdf.worker.mjs from https://github.com/mozilla/pdf.js/releases | ||
| lib.GlobalWorkerOptions.workerSrc = "/pdf.worker.mjs"; | ||
| pdfjsLib = lib; | ||
| isLoading = false; | ||
| return lib; | ||
| }); | ||
|
|
||
| return loadPromise; | ||
| return loadPromise; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Make worker path base-aware and harden loader to recover from import failures
Two issues here:
- Using an absolute “/pdf.worker.mjs” breaks when the app is served under a sub-path. Prefer import.meta.env.BASE_URL.
- If the dynamic import fails, isLoading stays true and loadPromise keeps pointing to a rejected promise, preventing retries.
Apply this diff:
async function loadPdfJs(): Promise<any> {
if (pdfjsLib) return pdfjsLib;
if (loadPromise) return loadPromise;
- isLoading = true;
- // @ts-expect-error - pdfjs-dist/build/pdf.mjs is not a module
- loadPromise = import("pdfjs-dist/build/pdf.mjs").then((lib) => {
- // Set the worker source to use local file
- // IMPORTANT: Must match the version of pdfjs-dist installed (e.g., v5.4.54)
- // Get pdf.worker.mjs from https://github.com/mozilla/pdf.js/releases
- lib.GlobalWorkerOptions.workerSrc = "/pdf.worker.mjs";
- pdfjsLib = lib;
- isLoading = false;
- return lib;
- });
-
- return loadPromise;
+ isLoading = true;
+ // @ts-expect-error - pdfjs-dist/build/pdf.mjs is not a module
+ const workerPath = `${import.meta.env.BASE_URL}pdf.worker.mjs`;
+ loadPromise = (async () => {
+ try {
+ const lib = await import("pdfjs-dist/build/pdf.mjs");
+ // Set the worker source to use local file
+ // IMPORTANT: Must match the version of pdfjs-dist installed (e.g., v5.4.54)
+ // Get pdf.worker.mjs from https://github.com/mozilla/pdf.js/releases
+ lib.GlobalWorkerOptions.workerSrc = workerPath;
+ pdfjsLib = lib;
+ return lib;
+ } catch (err) {
+ // Allow subsequent calls to retry loading
+ loadPromise = null;
+ throw err;
+ } finally {
+ isLoading = false;
+ }
+ })();
+
+ return loadPromise!;
}Verification:
- If your deployment uses a non-root base path, confirm that
${import.meta.env.BASE_URL}pdf.worker.mjsresolves correctly at runtime. - Ensure public/pdf.worker.mjs exists in the repo or is copied during postinstall.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Locate app/lib/pdf2img.ts ==="
file=$(find . -path ./node_modules -prune -o -type f -name 'pdf2img.ts' -print -quit || true)
if [ -n "$file" ]; then
echo "Found: $file"
echo
echo "=== Show $file (first 300 lines) ==="
sed -n '1,300p' "$file" || true
else
echo "pdf2img.ts not found"
fi
echo
echo "=== Search for occurrences of 'pdf.worker.mjs' ==="
rg -n --hidden --no-ignore 'pdf.worker.mjs' || true
echo
echo "=== Check if public/pdf.worker.mjs exists ==="
if [ -f "public/pdf.worker.mjs" ]; then
echo "public/pdf.worker.mjs exists"
ls -l public/pdf.worker.mjs
else
echo "public/pdf.worker.mjs NOT found"
fi
echo
echo "=== Search for 'pdfjs-dist' in repo ==="
rg -n --hidden --no-ignore 'pdfjs-dist' || true
echo
echo "=== Search for import.meta.env.BASE_URL usage ==="
rg -n --hidden --no-ignore 'import.meta.env.BASE_URL' || true
echo
echo "=== Find vite config files and check base option ==="
vite_files=$(find . -maxdepth 3 -type f -name 'vite.config.*' -print || true)
if [ -n "$vite_files" ]; then
echo "Found vite config files:"
echo "$vite_files"
echo
for f in $vite_files; do
echo "---- $f ----"
sed -n '1,200p' "$f" || true
echo
done
else
echo "No vite.config.* files found"
fi
echo
echo "=== Show package.json base fields and dependencies for pdfjs-dist (if present) ==="
if [ -f package.json ]; then
jq -r '.name, .version, .dependencies["pdfjs-dist"] // .devDependencies["pdfjs-dist"] // "NO_PDFJS" , .homepage // .base' package.json || true
else
echo "package.json not found"
fi
echo
echo "=== List public/ directory (if exists) ==="
if [ -d public ]; then
ls -la public | sed -n '1,200p' || true
else
echo "public/ directory not found"
fiLength of output: 6171
Make worker path base-aware and allow loader to recover from import failures
Confirmed both issues in app/lib/pdf2img.ts:
- workerSrc currently uses an absolute "/pdf.worker.mjs" (breaks under sub-path deployment).
- loadPromise/isLoading handling leaves the loader stuck after a failed dynamic import (prevents retries).
Repository checks:
- public/pdf.worker.mjs exists at public/pdf.worker.mjs.
- package.json contains "pdfjs-dist": "^5.4.54".
- vite.config.ts exists but does not set a base (import.meta.env.BASE_URL will default to "/").
Files to update:
- app/lib/pdf2img.ts (the loader logic around the dynamic import)
Apply this diff:
async function loadPdfJs(): Promise<any> {
if (pdfjsLib) return pdfjsLib;
if (loadPromise) return loadPromise;
- isLoading = true;
- // @ts-expect-error - pdfjs-dist/build/pdf.mjs is not a module
- loadPromise = import("pdfjs-dist/build/pdf.mjs").then((lib) => {
- // Set the worker source to use local file
- // IMPORTANT: Must match the version of pdfjs-dist installed (e.g., v5.4.54)
- // Get pdf.worker.mjs from https://github.com/mozilla/pdf.js/releases
- lib.GlobalWorkerOptions.workerSrc = "/pdf.worker.mjs";
- pdfjsLib = lib;
- isLoading = false;
- return lib;
- });
-
- return loadPromise;
+ isLoading = true;
+ // @ts-expect-error - pdfjs-dist/build/pdf.mjs is not a module
+ const workerPath = `${import.meta.env.BASE_URL}pdf.worker.mjs`;
+ loadPromise = (async () => {
+ try {
+ const lib = await import("pdfjs-dist/build/pdf.mjs");
+ // Set the worker source to use local file
+ // IMPORTANT: Must match the version of pdfjs-dist installed (e.g., v5.4.54)
+ // Get pdf.worker.mjs from https://github.com/mozilla/pdf.js/releases
+ lib.GlobalWorkerOptions.workerSrc = workerPath;
+ pdfjsLib = lib;
+ return lib;
+ } catch (err) {
+ // Allow subsequent calls to retry loading
+ loadPromise = null;
+ throw err;
+ } finally {
+ isLoading = false;
+ }
+ })();
+
+ return loadPromise!;
}Notes:
- public/pdf.worker.mjs is present so no extra file needed, but ensure it remains in the public folder or is copied during your build/deploy.
- If you deploy under a non-root base, set the Vite base option (vite.config.ts) or otherwise ensure import.meta.env.BASE_URL is configured to the correct sub-path so workerPath resolves correctly.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/lib/pdf2img.ts around lines 12 to 28, the loader uses an absolute
"/pdf.worker.mjs" which breaks under non-root bases and leaves
loadPromise/isLoading stuck after a failed dynamic import; update the worker
path to be base-aware by prepending import.meta.env.BASE_URL (e.g., const
workerPath = `${import.meta.env.BASE_URL}pdf.worker.mjs`) and set
lib.GlobalWorkerOptions.workerSrc = workerPath, and add a .catch handler on the
dynamic import that resets loadPromise = undefined and isLoading = false before
rethrowing the error so subsequent calls can retry (keep returning loadPromise
when present and preserve the existing success path).
| file: File | ||
| ): Promise<PdfConversionResult> { | ||
| try { | ||
| const lib = await loadPdfJs(); | ||
| try { | ||
| const lib = await loadPdfJs(); | ||
|
|
||
| const arrayBuffer = await file.arrayBuffer(); | ||
| const pdf = await lib.getDocument({ data: arrayBuffer }).promise; | ||
| const page = await pdf.getPage(1); | ||
| const arrayBuffer = await file.arrayBuffer(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against SSR/client mismatch
This function uses DOM APIs and must only run in the browser. Add a defensive check to fail fast when accidentally invoked during SSR.
export async function convertPdfToImage(
file: File
): Promise<PdfConversionResult> {
- try {
+ try {
+ if (typeof window === "undefined" || typeof document === "undefined") {
+ return { imageUrl: "", file: null, error: "PDF conversion is only available in the browser." };
+ }
const lib = await loadPdfJs();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| file: File | |
| ): Promise<PdfConversionResult> { | |
| try { | |
| const lib = await loadPdfJs(); | |
| try { | |
| const lib = await loadPdfJs(); | |
| const arrayBuffer = await file.arrayBuffer(); | |
| const pdf = await lib.getDocument({ data: arrayBuffer }).promise; | |
| const page = await pdf.getPage(1); | |
| const arrayBuffer = await file.arrayBuffer(); | |
| file: File | |
| ): Promise<PdfConversionResult> { | |
| try { | |
| if (typeof window === "undefined" || typeof document === "undefined") { | |
| return { imageUrl: "", file: null, error: "PDF conversion is only available in the browser." }; | |
| } | |
| const lib = await loadPdfJs(); | |
| const arrayBuffer = await file.arrayBuffer(); |
🤖 Prompt for AI Agents
In app/lib/pdf2img.ts around lines 31 to 36, the function uses DOM APIs and
needs a runtime guard to prevent SSR/client mismatch; add an early check (e.g.,
if typeof window === "undefined" || typeof document === "undefined") that throws
a clear Error or returns a rejected Promise indicating the function must run in
the browser, so the function fails fast during SSR rather than trying to call
browser-only APIs.
| const canvas = document.createElement("canvas"); | ||
| const context = canvas.getContext("2d"); | ||
|
|
||
| canvas.width = viewport.width; | ||
| canvas.height = viewport.height; | ||
| canvas.width = viewport.width; | ||
| canvas.height = viewport.height; | ||
|
|
||
| if (context) { | ||
| context.imageSmoothingEnabled = true; | ||
| context.imageSmoothingQuality = "high"; | ||
| } | ||
| if (context) { | ||
| context.imageSmoothingEnabled = true; | ||
| context.imageSmoothingQuality = "high"; | ||
| } | ||
|
|
||
| await page.render({ canvasContext: context!, viewport }).promise; | ||
| await page.render({ canvasContext: context!, viewport }).promise; | ||
|
|
There was a problem hiding this comment.
Fix potential null canvas context crash
context! suppresses TS checks but will still pass null at runtime if getContext fails. Return a proper error instead and pass a non-null context to render.
const canvas = document.createElement("canvas");
const context = canvas.getContext("2d");
canvas.width = viewport.width;
canvas.height = viewport.height;
- if (context) {
- context.imageSmoothingEnabled = true;
- context.imageSmoothingQuality = "high";
- }
+ if (!context) {
+ return { imageUrl: "", file: null, error: "Canvas 2D context not available." };
+ }
+ context.imageSmoothingEnabled = true;
+ context.imageSmoothingQuality = "high";
- await page.render({ canvasContext: context!, viewport }).promise;
+ await page.render({ canvasContext: context, viewport }).promise;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const canvas = document.createElement("canvas"); | |
| const context = canvas.getContext("2d"); | |
| canvas.width = viewport.width; | |
| canvas.height = viewport.height; | |
| canvas.width = viewport.width; | |
| canvas.height = viewport.height; | |
| if (context) { | |
| context.imageSmoothingEnabled = true; | |
| context.imageSmoothingQuality = "high"; | |
| } | |
| if (context) { | |
| context.imageSmoothingEnabled = true; | |
| context.imageSmoothingQuality = "high"; | |
| } | |
| await page.render({ canvasContext: context!, viewport }).promise; | |
| await page.render({ canvasContext: context!, viewport }).promise; | |
| const canvas = document.createElement("canvas"); | |
| const context = canvas.getContext("2d"); | |
| canvas.width = viewport.width; | |
| canvas.height = viewport.height; | |
| if (!context) { | |
| return { imageUrl: "", file: null, error: "Canvas 2D context not available." }; | |
| } | |
| context.imageSmoothingEnabled = true; | |
| context.imageSmoothingQuality = "high"; | |
| await page.render({ canvasContext: context, viewport }).promise; |
🤖 Prompt for AI Agents
In app/lib/pdf2img.ts around lines 41 to 53, the code uses context! which can
still be null at runtime; instead check whether canvas.getContext("2d") returned
a non-null value and if it is null return/throw a clear error (or reject) before
calling page.render so you never pass a null context. Set imageSmoothing
properties only when context is present, and pass the validated non-null context
to page.render; remove the non-null assertion and ensure the function’s return
type/signature reflects the early error case.
| } catch (err) { | ||
| return { | ||
| imageUrl: "", | ||
| file: null, | ||
| error: `Failed to convert PDF: ${err}`, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return a cleaner error message
Interpolating an Error object can produce “[object Object]”. Prefer extracting message safely.
} catch (err) {
return {
imageUrl: "",
file: null,
- error: `Failed to convert PDF: ${err}`,
+ error: `Failed to convert PDF: ${
+ err instanceof Error ? err.message : String(err)
+ }`,
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (err) { | |
| return { | |
| imageUrl: "", | |
| file: null, | |
| error: `Failed to convert PDF: ${err}`, | |
| }; | |
| } | |
| } catch (err) { | |
| return { | |
| imageUrl: "", | |
| file: null, | |
| error: `Failed to convert PDF: ${ | |
| err instanceof Error ? err.message : String(err) | |
| }`, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In app/lib/pdf2img.ts around lines 80 to 86, the catch block interpolates the
caught error directly which can yield “[object Object]”; change the returned
error string to extract the message safely (for example use err instanceof Error
? err.message : String(err)) so the error field contains a readable message and
is null/undefined-safe.
| "clsx": "^2.1.1", | ||
| "isbot": "^5.1.27", | ||
| "pdfjs-dist": "^5.3.93", | ||
| "pdfjs-dist": "^5.4.54", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin pdfjs-dist to an exact version to prevent worker/API mismatches
With a manually managed worker file in public/, using a caret range will eventually reintroduce the same error when a minor/patch update is installed. Pin this dependency to the exact version you’ve copied into public/pdf.worker.mjs.
Apply this diff:
- "pdfjs-dist": "^5.4.54",
+ "pdfjs-dist": "5.4.54",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "pdfjs-dist": "^5.4.54", | |
| "pdfjs-dist": "5.4.54", |
🤖 Prompt for AI Agents
In package.json around line 16, the pdfjs-dist dependency is declared with a
caret range ("^5.4.54") which can allow minor/patch updates that will drift from
the manually copied public/pdf.worker.mjs; change the version to the exact
version that matches public/pdf.worker.mjs (e.g., "5.4.54" without the ^), save
the file, then run your package manager (npm install or yarn install) to update
lockfiles so the pinned version is installed.
1 participant
Problem
When using
pdfjs-dist, users get this error:The API version "5.4.54" does not match the Worker version "5.3.93"
This happens because the worker file (
pdf.worker.mjs) must exactly match the version ofpdfjs-distinstalled.Solution
workerSrcto point to/pdf.worker.mjs(standard name)How to Fix in Your Project
npm list pdfjs-distpdfjs-X.X.X-dist.zipbuild/pdf.worker.mjs→ yourpublic/folderSummary by CodeRabbit