Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update pdfWorkerSource to fix console warnings #39

Merged
merged 1 commit into from
Nov 27, 2024
Merged

Update pdfWorkerSource to fix console warnings #39

merged 1 commit into from
Nov 27, 2024

Conversation

VickyStash
Copy link
Contributor

@VickyStash VickyStash commented Nov 21, 2024 •
edited
Loading

Details

This PR fixes two things:

  1. Updates the worker path inside the webpack config of the example app, cause it used the non-existing file. Due to this the fake worker always was used.

image

  1. Uses a pdf.worker.min.mjs instead of pdf.worker.js to fix console warnings during PDF file preview.

Related Issues

Expensify/App#51313

Manual Tests

  1. Launch the example app.
  2. Open example PDF file.
  3. Make sure there are no warnings in the console and file is displayed as expected.

image

Linked PRs

Expensify/App#53080

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 21, 2024 •
edited
Loading

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@VickyStash VickyStash mentioned this pull request Nov 21, 2024
Closed
8 tasks
@VickyStash
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

CLABotify added a commit to Expensify/CLA that referenced this pull request Nov 21, 2024
@CLABotify
@VickyStash has signed the CLA in Expensify/react-fast-pdf#39
d9ba2ee
@VickyStash
Copy link
Contributor Author

@allgandalf Will you be able to take a look at this PR?

@VickyStash VickyStash mentioned this pull request Nov 25, 2024
Merged
49 tasks
@allgandalf
Copy link

Looking

allgandalf
allgandalf approved these changes Nov 26, 2024
View reviewed changes
Copy link

@allgandalf allgandalf left a comment

Choose a reason for hiding this comment

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

LGTM!

@tylerkaraszewski tylerkaraszewski merged commit e8366dc into Expensify:main Nov 27, 2024
2 checks passed
@os-botify
Copy link
Contributor

os-botify bot commented Nov 27, 2024

🚀 Published to npm in 1.0.20 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@allgandalf allgandalf allgandalf approved these changes

@tylerkaraszewski tylerkaraszewski tylerkaraszewski approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@VickyStash @allgandalf @tylerkaraszewski