-
Notifications
You must be signed in to change notification settings - Fork 10
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
Refactor #34
base: master
Are you sure you want to change the base?
Conversation
You don’t like classes? 😜 |
Haha it's not that. Maybe I shouldn't have pushed this yet. It's a WIP, but that thrown very good results so far. Let me explain a bit what this is 😄 The original cause of this refactor are some issues I was having with the original solution:
Don't get me wrong. The current solution is awesome and has proven to have value on react-pdf. However, I wanted to give it a shot on a refactor that follows two main heuristics:
I didn't want to change the API of the lib (which is not immutable), so there are still some things that can be confusing, but based on some tests (with some other enhancements on the react-pdf side) I observed a performance boost of ~25%. It's pretty visible on large documents. Anyways, would love to hear your thoughts! There's still some work to do on this repo, but I plan to release a react-pdf with this solution hopefully soon, after testing it a bit further. I'm now in a point in which 100% of the documents I tried (some of them very complex), passed successfully. |
@devongovett would yo be ok to merge this? It's being tested on |
No description provided.