-
-
Notifications
You must be signed in to change notification settings - Fork 16
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
Swap out Buffer with browser-compatible Uint8Array #83
Conversation
public readonly chainCodeBuffer: Uint8Array; | ||
|
||
public readonly privateKeyBuffer?: Buffer; | ||
public readonly privateKeyBuffer?: Uint8Array; | ||
|
||
public readonly publicKeyBuffer: Buffer; | ||
public readonly publicKeyBuffer: Uint8Array; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change will be breaking. Should we rename these properties to *Bytes
instead of *Buffer
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're gonna do this in a follow-up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to my inline comment(s), there are some questions by @FrederikBolding that should be resolved before we merge this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Buffer
is a Node.js API and does not work in browsers without polyfills. This swaps out all usage ofBuffer
withUint8Array
, to make this library fully browser compatible without the need for polyfills.