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

Drop node dependencies #48

Merged
merged 5 commits into from
Jun 18, 2022
Merged

Drop node dependencies #48

merged 5 commits into from
Jun 18, 2022

Conversation

devongovett
Copy link
Member

@devongovett devongovett commented Jun 8, 2022

Closes #24

This drops Node Buffer usages, and replaces them with Uint8Array, DataView, and TextDecoder/TextEncoder, which are standard browser APIs also supported by Node.

iconv-lite usages are also dropped. Unfortunately, TextEncoder only supports utf8, while TextDecoder supports many legacy encodings. This means that decoding legacy encodings is supported with no bundle size implications, but encoding is not supported. This should be ok for fontkit but it is a breaking change.

The one remaining node API in use is streams. Web Streams unfortunately are only supported in Node 16, but Node 14 is still LTS. I suppose we could require a polyfill but that's not great...

@benjamind
Copy link

Nice work!

Could the legacy encoding become an optional import under a node-only export map along with the streaming API? Or just provide both through a separate import available through a specific file?

Still a major version breaking change but at least gives folk a fallback option and still keeps the main import clean of node dependencies easily?

Copy link

@carlobeltrame carlobeltrame left a comment

Choose a reason for hiding this comment

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

Thanks, this is definitely a big step forward! I am currently testing it in the context of https://github.com/diegomura/react-pdf, will let you know of any issues I find.

@devongovett
Copy link
Member Author

@carlobeltrame thanks. Please note that this is a breaking change, so will require changes to fontkit as well.

carlobeltrame added a commit to carlobeltrame/react-pdf that referenced this pull request Jun 16, 2022
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.

Dependencies: use node or Buffer shim with browsers
3 participants