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

Replace Buffer with Uint8Array #160

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

0x0ece
Copy link

@0x0ece 0x0ece commented Dec 4, 2023

I don't know why base64 is so hard in js. My recommendation would be to use https://github.com/paulmillr/scure-base, but lmk your thoughts. (btw, if you ever want to add algorithms also for client, there's https://github.com/paulmillr/noble-curves from the same author).

I have node18 on my system, when I added the new dependency it seems it made additional changes to the package.json and lock file. If you can share what version of node you're using, I can probably fix that.

All tests pass on my machine. Interestingly enough, using scure I found some extra spaces in some base64 encodings (scure is more strict).

@dhensby
Copy link
Owner

dhensby commented Dec 4, 2023

Please reword your commit message so that it conforms to the commit linting spec (feat: replace Buffer with Uint8Array)

base64 shouldn't be too hard in browser envs (see https://developer.mozilla.org/en-US/docs/Glossary/Base64#javascript_support). Is an external lib necessary when we only need basic base64 encode/decode, especially when the lib provides so much more?

This removes the use of Buffer, which is nice, but it doesn't actually make it work in browser environments because it still depends on node:crypto package, right?

At the moment minimum support is node 12, so if you install packages using node 12 locally, that should keep the package-lock.json compatible

@0x0ece
Copy link
Author

0x0ece commented Dec 4, 2023

This removes the use of Buffer, which is nice, but it doesn't actually make it work in browser environments because it still depends on node:crypto package, right?

Correct, we'd need a way to conditionally include algorithms for server and not for client. Or, eventually, reimplement all algorithms client-side.

I'll make the changes (but need a bit of time).

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

2 participants