search

bitcoinjs / tiny-secp256k1

A tiny secp256k1 native/JS wrapper
MIT License
92 stars 54 forks source link

fix: removed dep from `window` object #112

Closed shamoilarsi closed 1 year ago

shamoilarsi commented 1 year ago

closes #111

landabaso commented 1 year ago

Why not consider an approach like this instead?

if (typeof crypto === 'undefined') {
  throw new Error('The crypto object is unavailable. This may occur if your environment does not support the Web Cryptography API.');
}
crypto.getRandomValues(bytes);

By directly referencing crypto, the correct scope (window or self) will be selected based on the environment. This could remove the necessity of using a separate crypto package, while still ensuring we have the required functionality available.

junderw commented 1 year ago

NACK on adding crypto-js as a dep.

Since it's the entire PR, basically, I'm tempted to close unless you can come up with something better.

shamoilarsi commented 1 year ago

@junderw will try a different approach. converting the PR to draft till then

shamoilarsi commented 1 year ago

@landabaso yep that makes sense. I wasn't aware web workers have a crypto package as well which can be called with self. thanks for the help :D

junderw commented 1 year ago

Please rebase. It will fix the broken CI test.

junderw commented 1 year ago

Published as v2.2.2