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.

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

Upgrade versions and fix example #42

Merged
merged 11 commits into from
Jan 21, 2025

Conversation

rezkiy37
Copy link
Contributor

@rezkiy37 rezkiy37 commented Jan 15, 2025

Details

The PR upgrades versions of "react-pdf": "^9.2.1" and "react-window ^1.8.11".

Details

Screenshot 2025-01-16 at 15 41 33 Screenshot 2025-01-16 at 15 41 43

Also, it locks pdfjs-dist to 4.8.69 as react-pdf utilizes.

Related Issues

Expensify/App#55176

Manual Tests

  1. Launch the example app.
  2. Open an example PDF file.
  3. Verify that there are no warnings in the console and that the file is displayed properly.
Screenshots

Screenshot 2025-01-16 at 15 44 47 Screenshot 2025-01-16 at 15 46 21 Screenshot 2025-01-16 at 15 48 04

Linked PRs

@rezkiy37 rezkiy37 changed the title [WIP] Upgrade versions and fix example Upgrade versions and fix example Jan 16, 2025
@rezkiy37 rezkiy37 marked this pull request as ready for review January 16, 2025 15:11
Copy link

@jjcoffee jjcoffee left a comment

Choose a reason for hiding this comment

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

LGTM!

@jjcoffee
Copy link

@rezkiy37 Should I test this against the Expensify app too?

@rezkiy37
Copy link
Contributor Author

@jjcoffee yeah, a good point. I've tried to test both projects locally but unfortunately, react-fast-pdf cannot use a couple of dependencies. I imported /dist, which is not similar to how it works with NPM. So I am going to test it once we deploy it.

@jjcoffee
Copy link

@rezkiy37 Okay that makes sense, thanks!

@rezkiy37
Copy link
Contributor Author

@mountiny, this one it ready to go 🙂

Copy link
Contributor

@mountiny mountiny left a comment

Choose a reason for hiding this comment

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

Please make sure to test this thoroughly in the app

@mountiny mountiny merged commit 4810b10 into Expensify:main Jan 21, 2025
2 checks passed
@os-botify
Copy link
Contributor

os-botify bot commented Jan 21, 2025

🚀 Published to npm in 1.0.23 🎉

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