Skip to content

Release/2.5.3: fix PDF uploads in common-file-uploader - #19

Merged
istevens507 merged 3 commits into
mainfrom
release/2.5.3
Oct 13, 2026
Merged

istevens507 merged 3 commits into
mainfrom
release/2.5.3

Conversation

@istevens507

Copy link
Copy Markdown
Contributor

Problem

Every PDF upload failed with CannotOpenPDF ("That PDF cannot be opened, please upload a different attachment." in MSP) in apps that never set PdfService.workerSrc. That includes MSP DEAM. pdfjs-dist 4.x has no main-thread fallback when no worker is configured, so getDocument() threw No "GlobalWorkerOptions.workerSrc" specified.. Affects 2.1.0 through 2.5.2. Image uploads were not affected.

Fix

  • PdfService.load(): when workerSrc is unset, import pdfjs-dist/legacy/build/pdf.worker.mjs and register it as globalThis.pdfjsWorker, so pdfjs runs the worker on the main thread.
  • The import keeps its .catch(), so pdfjs-dist stays an optional peer.
  • pdfjs is cached only after the worker is registered, so selecting several PDFs at once, or retrying after a failed load, works.
  • Apps that set workerSrc are unchanged. No public API or peer-range change.

Release

  • Version bumped to 2.5.3 in all four places (build_utils/release-set-version.sh), with a CHANGELOG entry.
  • release-preflight.sh 2.5.3 passes.
  • Merging to main runs release.yml and publishes 2.5.3.

pdfjs-dist 4.x has no main-thread fallback without a worker: getDocument()
throws 'No "GlobalWorkerOptions.workerSrc" specified.' unless
globalThis.pdfjsWorker is set, so every PDF upload failed with CannotOpenPDF
in apps that never set PdfService.workerSrc. Import the legacy worker module
and register it in that case. Keep both imports behind .catch() so pdfjs-dist
stays an optional peer, and cache pdfjs only after the worker is registered
so concurrent and retried calls work.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The release’s central pdfjs integration is tested only through virtual mocks that reproduce the production assumption.

1 open finding
What changed in this PR

Fixes PDF uploads when consumers do not configure PdfService.workerSrc.

Changes:

  • Registers the bundled pdfjs worker as a main-thread fallback.
  • Adds worker-loading, concurrency, retry, and error tests.
  • Bumps the release to 2.5.3 with release notes.
File Description
src/​lib/​components/​file-uploader/​pdf.service.ts Adds bundled worker fallback.
src/​lib/​components/​file-uploader/​pdf.service.spec.ts Tests worker loading behavior.
projects/​common-lib/​package.json Bumps library version.
package.json Bumps root version.
package-lock.json Synchronizes locked version metadata.
CHANGELOG.md Documents the 2.5.3 fix.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/lib/components/file-uploader/pdf.service.spec.ts

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused fallback fix is supported by comprehensive unit and real-library regression coverage.

1 open finding

🧠 Review effort: Balanced

@istevens507
istevens507 enabled auto-merge (squash) October 13, 2026 21:57

@cyrilbravo cyrilbravo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes looks good to me.

@istevens507
istevens507 merged commit cd56807 into main Oct 13, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants