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

Update pdfjs-dist to 2.6.347 #746

Merged
merged 1 commit into from
Mar 19, 2021
Merged

Update pdfjs-dist to 2.6.347 #746

merged 1 commit into from
Mar 19, 2021

Conversation

andi-dev
Copy link
Contributor

Hey guys, I don't know what you think about keeping the pdfjs version up-to-date, and I honestly can't overlook if this could break anything in react-pdf. However I need to run react-pdf with a newer pdfjs version, and I would prefer not to use a fork...

If there is anything I can further do, to make sure this could make it into upstream I'd be happy to help.

@andi-dev andi-dev force-pushed the master branch 2 times, most recently from bee168f to a8b73f2 Compare March 17, 2021 20:27
@wojtekmaj
Copy link
Owner

I honestly can't overlook if this could break anything in react-pdf

Unfortunately Mozilla is yet to discover the wonders of semver. It was pretty common to introduce breaking changes in previous "minor" releases.

That's why pdfjs-dist dependency version is strictly set. That being said, every pdfjs-dist update needs to be thoroughly tested with React-PDF.

@wojtekmaj
Copy link
Owner

This update looks clean though!

One more request. Could you also run yarn in test directory so we don't end up with outdated lockfiles? Thanks!

@wojtekmaj wojtekmaj added the evergreening Keeps the project up to date label Mar 18, 2021
@andi-dev
Copy link
Contributor Author

@wojtekmaj hey, thanks for the feedback, and yes I will quickly run yarn in test.

Btw. you mention in #748 that the pdfjs update is included in react-pdf-5.3.0, but this PR is not yet merged. So it is not included in the beta right? Once it is included in the beta I will install it an help testing.

@andi-dev
Copy link
Contributor Author

andi-dev commented Mar 18, 2021

@wojtekmaj I updated this PR with the updated test/yarn.lock

@wojtekmaj
Copy link
Owner

wojtekmaj commented Mar 18, 2021

Btw. you mention in #748 that the pdfjs update is included in react-pdf-5.3.0, but this PR is not yet merged. So it is not included in the beta right? Once it is included in the beta I will install it an help testing.

Actually it is included in the beta. Just because something isn't on master doesn't mean it's not somewhere else (in this case, next branch) :)

@wojtekmaj
Copy link
Owner

Looks like we're good here. Thanks!

@wojtekmaj wojtekmaj merged commit 0953e70 into wojtekmaj:master Mar 19, 2021
@wojtekmaj wojtekmaj added this to the 5.3.0 milestone Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evergreening Keeps the project up to date
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants