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

Eliminate Buffer dependency [WIP] #925

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft

Eliminate Buffer dependency [WIP] #925

wants to merge 14 commits into from

Conversation

billiegoose
Copy link
Member

@billiegoose billiegoose commented Nov 11, 2019

BREAKING CHANGE: methods will return Uint8Arrays instead of Buffers.

Reasoning:

  • Buffer is not isomorphic, and using it breaks Angular. (You can polyfill it manually but it doesn't work out-of-the-box.)
  • Why should we be using Buffer when Uint8Array has been around forever and is part of the language.
  • This lets us remove the buffer module (which gets automatically inserted by Webpack) shaving off a whopping 5.71 KB.

I've confirmed that sha.js is the only module using Buffer (via safe-buffer). Since we recently enabled WebCrypto SHA1 as an optional optimization - and the "new" MS Edge based on Chromium actually supports SHA-1 digest (at least in the beta version currently installed) - and in Node we could always use the native SHA1... I definitely wonder if we could get away with dropping the sha.js dependency. That would save an additional 1 KB, and when I compile that with Webpack, we now are at 64 KB instead of 71 KB gzipped, or a 10% reduction, which is pretty phenomenal. The only bigger improvement would be if we could drop packo.

@isomorphic-git-bot
Copy link

Thank you wmhilton! I auto-formatted the code using prettier-standard. 🤖

@billiegoose billiegoose changed the title No Buffer dependency [WIP] Eliminate Buffer dependency [WIP] Nov 11, 2019
Base automatically changed from master to main December 17, 2020 03:53
@nadilas
Copy link

nadilas commented Aug 27, 2022

Hi @wmhilton,

This lets us remove the buffer module (which gets automatically inserted by Webpack) shaving off a whopping 5.71 KB.

this would also enable vercel's edge runtime to work:

error - Error: The edge runtime does not support Node.js 'buffer' module.
Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime

@jcubic
Copy link
Contributor

jcubic commented Aug 27, 2022

@nadilas I'm afraid this will never be finished. OP is not working on the project anymore.

@nadilas
Copy link

nadilas commented Aug 27, 2022

thanks @jcubic, does that mean it is in general not recommended to use the library at all?

@jcubic
Copy link
Contributor

jcubic commented Aug 27, 2022

@nadilas No, it means that the project is driven only by contributors, it's maintained but there is no single person that does all the hard work of adding new features and fixing bugs. If you want to help and remove Buffer dependency you are more than welcome to do this.

@nadilas
Copy link

nadilas commented Aug 27, 2022

@jcubic as it should be. I'll take a crack at it

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.

None yet

4 participants