refactor: remove Buffer dependency from EXIF orientation parser#1717
Open
refactor: remove Buffer dependency from EXIF orientation parser#1717
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The other foliojs modules — png-js, restructure, and fontkit — have all migrated away from Node-specific APIs (notably Buffer) and now operate on plain Uint8Array. PDFKit is the odd one out: it still leans heavily on Buffer, stream, and other Node built-ins throughout the codebase. I think it's time to bring PDFKit in line with the rest of the ecosystem.
Why this matters
Approach
Rather than one mega-PR, I'd like to do this incrementally, one module/area at a time, so each change is small, easy to review, and safe to ship behind the existing test suite.
This PR kicks things off with the EXIF orientation parser in
lib/image/jpeg.jsand introduceslib/binary.jswith the shared Uint8Array helpers that future PRs will build on.Test plan
@blikblum @devongovett would you be onboard of this? I'm trying to start using upstream pdfkit again in react-pdf but things this effort will unblock are mandatory for that to happen. I'll push these changes first on my fork there, but I'd still like to have them merged here for anyone using this lib can benefit from them