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

Remove direct crypto module dependency to support React Native #19

Closed
kamescg opened this issue Nov 2, 2019 · 10 comments
Closed

Remove direct crypto module dependency to support React Native #19

kamescg opened this issue Nov 2, 2019 · 10 comments
Assignees
Milestone

Comments

@kamescg
Copy link

kamescg commented Nov 2, 2019

Describe the bug
The crypto module breaks React Native.

Would be great to use the shim provided by ethers to add the essential crypto methods.

@kamescg kamescg changed the title Remove crypto module to support React Native Remove direct crypto module dependency to support React Native Nov 2, 2019
@kamescg
Copy link
Author

kamescg commented Nov 2, 2019

Something odd is happening, because I have ethers already installed too.

When I include identity-wallet it breaks the crypto require for ethers too. However, without identity wallet, the ethers modules works as expected.

So, maybe because identity-wallet doesn't require the shim it appears to rewrite stuff and break ethers too?

@kamescg
Copy link
Author

kamescg commented Nov 2, 2019

Removing crypto to create the sha256 hash will fix the problem. Got ethers and identity-wallet working in tandem now in a React Native env.

https://github.com/3box/identity-wallet-js/blob/11d7fdb7590c1e7fd63490ad02e81bebab25c87c/src/utils.js#L13

@kamescg
Copy link
Author

kamescg commented Nov 3, 2019

I realize now it's a futile effort, because the HDNode module use random-bytes which I thought ethers it was shimming, but it's not.

So probably not worth the effort to remove crypto because still need the random-bytes in RN.

Maybe WASM modules to replace all the node/crypto stuff is a smart long-term move for a React Native environment?

@oed
Copy link
Member

oed commented Nov 4, 2019

Thanks for reporting. You're right, we should probably just use js-sha256 instead. As for problems with random-bytes, I've had success with rn-nodeify before.

@oed oed added this to the Sprint 31 milestone Nov 5, 2019
@kamescg
Copy link
Author

kamescg commented Nov 7, 2019

Quick heads up that using the dist version fixed all the React Native Metro crypto bundler problems for me. I don't if you guys want to keep non-dist version (for tree shaking) or not, but maybe we can get a React Native fix inlcuded somehow?

import {utils} from 'ethers/dist/ethers';
...
this._baseNode = utils.HDNode.fromSeed(this._seed).derivePath(BASE_PATH);

@oed
Copy link
Member

oed commented Nov 8, 2019

@kamescg we are no longer importing the entire ethers library for size reasons. Did you try using rn-nodeify as I mentioned above?

@kamescg
Copy link
Author

kamescg commented Nov 8, 2019

Yes. My whole app is nodified/cryptoified. I'm running ethers nicely.

It's only this library throwing import errors and I believe it's because the Metro compile is not catching the crypto module in 3Box and re-setting to react-native-crypto.

@kamescg
Copy link
Author

kamescg commented Nov 8, 2019

It's an old fix from awhile back => ethers-io/ethers.js#268 (comment)

I'm gonna make sure I'm running the latest and see if I still have any issues.. I think my identity-wallet version might be behind an update or too.

@oed
Copy link
Member

oed commented Nov 11, 2019

catching the crypto module in 3Box

Wait are you running 3box, or IdentityWallet in RN? The former will likely not work at all.

@kamescg
Copy link
Author

kamescg commented Nov 11, 2019

IdentityWallet. That was a typo.

I'm aware of all the 3Box React Native issues and I'm not going paid enough to want to deal with that headache :)

@oed oed assigned oed and unassigned ghiliweld Nov 26, 2019
@oed oed closed this as completed Dec 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants