Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

fix: remove iso-random-stream dependency #262

Merged
merged 1 commit into from
Aug 3, 2022

Conversation

D4nte
Copy link
Contributor

@D4nte D4nte commented Jul 20, 2022

This dependency brings in NodeJS's Buffer which then needs to be polyfilled in the browser.

@noble/* already brings a utility that switch between Node's and the browser's randomBytes interfaces.

One caveat is that iso-random-stream enables generation of arrays greater than 65536 in length in the browser.
It does not strike to me as a feature needed for libp2p-crypto. Let me know your thoughts.

This dependency brings in NodeJS's `Buffer` which then needs to be
polyfilled in the browser.

@noble/secp256k1 already brings a utility that switch between Node's and
the browser's `randomBytes` interfaces.
Copy link
Member

@wemeetagain wemeetagain left a comment

Choose a reason for hiding this comment

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

lgtm

@hugomrdias
Copy link
Member

@achingbrain

@achingbrain
Copy link
Member

This should be fine, I think the only place we need to generate larger chunks of random data is in tests and we can just use iso-random-stream directly where we need that.

@achingbrain achingbrain changed the title chore: remove iso-random-stream dependency fix: remove iso-random-stream dependency Aug 2, 2022
@achingbrain achingbrain merged commit c5cb096 into libp2p:master Aug 3, 2022
github-actions bot pushed a commit that referenced this pull request Aug 3, 2022
## [1.0.2](v1.0.1...v1.0.2) (2022-08-03)

### Bug Fixes

* remove iso-random-stream dependency ([#262](#262)) ([c5cb096](c5cb096))
@github-actions
Copy link

github-actions bot commented Aug 3, 2022

🎉 This PR is included in version 1.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants