foliojs / textkit

Text layout framework
41 stars 10 forks source link

Refactor #34

Open diegomura opened 5 years ago

devongovett commented 5 years ago

You don’t like classes? 😜

diegomura commented 5 years ago

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.

diegomura commented 5 years ago

@devongovett would yo be ok to merge this? It's being tested on react-pdf for awhile now and it's working very good. Having all functional like this really makes the entire problem more easy to maintain and adding new features. For me, having this merged would make the development process (both for react-pdf and textkit) way more easy πŸ˜„