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

Fix merge #107

Merged
merged 4 commits into from
Mar 9, 2024
Merged

Fix merge #107

merged 4 commits into from
Mar 9, 2024

Conversation

acestronautical
Copy link
Collaborator

@acestronautical acestronautical commented Mar 8, 2024

Fix merging to use copypages instead of embedPDF. This resolves a margin issue where margins were being used from the source and destination PDFs in a weird way.

Replace hopding/pdf-lib with the maintained cantoo-scribe/pdf-lib to which we have an upstreamed performance fix for copypages cantoo-scribe/pdf-lib#48

Perform an npm audit fix and npm update

improve upload performance by pulling some variables out of the page rotation loop

@momijizukamori
Copy link
Owner

Can you do a quick filesize check? I feel like copyPages had more overhead and that's why I changed it....

@acestronautical
Copy link
Collaborator Author

acestronautical commented Mar 8, 2024

Not sure why the import is failing after switching to the cantoo version? It works fine on my local hosted version?
Edit: import was failing because I had the name wrong lol

File size seems the same for me, but I'm using a mostly text PDF

Hopding has abandoned pdf-lib and cantoo-scribe/pdf-lib
accepted my upstream fix
cantoo-scribe/pdf-lib#48
Copy link

github-actions bot commented Mar 8, 2024

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-03-09 04:55 UTC

Copy link
Owner

@momijizukamori momijizukamori left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you for the notes on some of the changes :) I bet jsdom was a dep of the original library version and isn't on the new one or something, heh.

@sithel sithel merged commit 4c0ffc8 into main Mar 9, 2024
3 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