Skip to content

Move responsibility for configuring PDF worker to the consumer#52

Merged
AndrewGable merged 1 commit intoExpensify:mainfrom
andriivitiv:76303-Move-responsibility-for-configuring-PDF-worker-to-the-consumer
Mar 23, 2026
Merged

Move responsibility for configuring PDF worker to the consumer#52
AndrewGable merged 1 commit intoExpensify:mainfrom
andriivitiv:76303-Move-responsibility-for-configuring-PDF-worker-to-the-consumer

Conversation

@andriivitiv
Copy link
Copy Markdown
Contributor

Details

We have switched between the modern and legacy PDF worker multiple times in the past. Each time we do this, we must update both E/App and this repo. PR #45 and Expensify/App#56473 are good examples of how we had to implement the same code twice, only to remove it later (again in both repos).

Furthermore, setting pdfjs.GlobalWorkerOptions.workerSrc in E/App has no effect because it would get overridden by this line:

pdfjs.GlobalWorkerOptions.workerSrc = URL.createObjectURL(new Blob([pdfWorkerSource], {type: 'text/javascript'}));

Even with the correct import order, both worker builds would be included in the bundle, increasing its size.

This PR removes the default worker configuration and instead expects applications using this library to configure it, which is the same approach used by react-pdf. This makes it more reliable and easier to maintain (we will only need to update the worker once in E/App).

Related Issues

Expensify/App#76303

Manual Tests

  1. Build with npm run build && npm pack.
  2. Install it in the example app or E/App with npm install ../react-fast-pdf-1.0.31.tgz.
  3. Open a PDF file.
  4. Verify that the file is displayed properly.

Linked PRs

@QichenZhu
Copy link
Copy Markdown

Looks good overall.

Screenshots/Videos

Android: HybridApp Screenshot_1773137891
Android: mWeb Chrome Screenshot_1773137695
iOS: HybridApp Simulator Screenshot - iPhone 17 - 2026-03-10 at 23 16 49
iOS: mWeb Safari Simulator Screenshot - iPhone 17 - 2026-03-10 at 23 17 39
MacOS: Chrome / Safari Screenshot 2026-03-10 at 11 13 57 PM

@QichenZhu
Copy link
Copy Markdown

@flodnv could you or someone on the team give this a second review? Thanks!

@flodnv
Copy link
Copy Markdown
Contributor

flodnv commented Mar 23, 2026

@AndrewGable in @roryabraham's absence, could you please review this PR?

@flodnv flodnv requested a review from AndrewGable March 23, 2026 16:14
@AndrewGable AndrewGable merged commit 26e2e64 into Expensify:main Mar 23, 2026
5 checks passed
@os-botify
Copy link
Copy Markdown
Contributor

os-botify bot commented Mar 23, 2026

🚀 Published to npm in 1.0.32 🎉

@flodnv
Copy link
Copy Markdown
Contributor

flodnv commented Mar 23, 2026

Thank you @AndrewGable 🙇

@andriivitiv andriivitiv deleted the 76303-Move-responsibility-for-configuring-PDF-worker-to-the-consumer branch March 25, 2026 09:29
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.

4 participants