Open dmitrizagidulin opened 3 years ago
Thanks for the callout @dmitrizagidulin! I have come across this issue as well. Any workarounds in the meantime?
I'll note that DigitialCredential's fork released under @digitalcredentials/rdf-canonize
provides RN support--although I've ran into some dependency issues regarding crypto.subtle
, but at least that might be a starting point for you @kezike.
All of DigitalCredential's forks provide React Native functionality AFAIK, which is described here.
Thanks for your response @JamesKEbert! Yes, I was able to come to the same conclusion a couple weeks ago.
Regarding your issue with crypto.subtle
. I have found the following updates to package.json
to resolve this issue:
scripts
: "postinstall": "npx rn-nodeify --install crypto,process,stream,events,module,util --hack && npx patch-package"
"react-native": {
"crypto": "react-native-crypto",
"_stream_transform": "readable-stream/transform",
"_stream_readable": "readable-stream/readable",
"_stream_writable": "readable-stream/writable",
"_stream_duplex": "readable-stream/duplex",
"_stream_passthrough": "readable-stream/passthrough",
"stream": "stream-browserify"
}
Hope this helps!
We don't use currently use react native at Digital Bazaar, so figuring out these support issues and testing is a bit difficult. The mass forking seems a bit unfortunate. I hope there's a better solution here. We've been in the process of converting all our other packages to ESM, and are trying to make everything run in modern browsers and recent Node.js versions. We'll get to jsonld, jsigs, and this package soon. And I hope that addresses some of the problems. It would be great if someone could help with figuring out what a minimal react native support patch would be. And perhaps also let us know how to test it.
We don't use currently use react native at Digital Bazaar, so figuring out these support issues and testing is a bit difficult. The mass forking seems a bit unfortunate. I hope there's a better solution here. We've been in the process of converting all our other packages to ESM, and are trying to make everything run in modern browsers and recent Node.js versions. We'll get to jsonld, jsigs, and this package soon. And I hope that addresses some of the problems. It would be great if someone could help with figuring out what a minimal react native support patch would be. And perhaps also let us know how to test it.
@davidlehn completely understand your points on supporting and testing react-native specific issues! But the fix is relatively simple and it was tested by at least our team and @kezike as well who proposed the root of the fix.
Using rn-nodeify
is not an option for everybody, especially for people creating a library for react-native users (which uses rdf-canonize
).
The actual fix is just a single line of code change in package.json
which is just metadata to make things to work (I can create a PR for that).
@pleerock It's never "just a single line". :-) Please do propose a PR if the changes are not invasive! If special RN support is needed, we also need a "npm run test-rn", or similar script, that tests RN support is working.
As some time has passed, what are the current issues using this package as-is? We really would prefer a straightforward approach of writing code that uses standard web APIs as much as possible with minimal platform specific support and dependencies. Applications should polyfill standard APIs as needed on their platforms. In this package we do have Node.js crypto and Web Crypto API support. We would only use only the Web Crypto API, but it turns out the Node.js crypto API performs better for our lots-of-hashing-calls use case.
I see your comment saying that a Web Crypto API polyfill is not possible in React Native? What is stopping them from fixing that? I had been hoping they would update their system such that RN support here would be only adding a RN override to the webcrypto code paths. In that case, it might only be a single line fix, excluding testing cruft.
In general, we don't have the resources to spend much time on this issue. I still don't know how to even test this in RN. Please help. A good start would be a PR that adds a RN test so we see the issues that need to be fixed.
About my "ESM soon" comments... ahem... maybe not so soon! It's on our roadmap though.
One small correction: there is actually a polyfill for React Native to replace node's crypto
, but not WebCrypto
. It's usable.
Obviously react-native
environment is NOT a browser and NOT node.js. Environment is different, and it's more like a node
than browser
.
There are polyfills which we can use, and usually rn-nodeify is used to resolve issues which can be resolved by exist polyfills.
The way rn-nodeify
works is a bit tricky and it works assumption-based. Also it's used after we install the package (e.g. rdf-canonize
in our case) and it modifies the original package.json
(e.g. changes installed node_modules/rdf-canonize/package.json
) the way it needs. E.g. works like post-installation script.
Particularly in this case it adds the following lines to package.json
:
"react-native": {
"./lib/MessageDigest.js": "./lib/MessageDigest-browser.js",
// ...
}
It happens because rn-nodeify
internally makes some assumptions on the browser
section defined in rdf-canonize
:
"browser": {
"./lib/MessageDigest.js": "./lib/MessageDigest-browser.js",
"rdf-canonize-native": false
}
But it cannot really understand that in this particular case browser-based code won't work in react-native becausern-nodify
uses node crypto shim for the crypto
capabilities. There is no way we can use browser version of crypto
in react-native.
Solution is simple as if we add a directive to original rdf-canonize's package.json to use MessageDigest.js
in react-native
environment, e.g.:
"react-native": {
"./lib/MessageDigest.js": "./lib/MessageDigest.js"
}
We really would prefer a straightforward approach of writing code that uses standard web APIs as much as possible with minimal platform specific support and dependencies.
Of course that's what we always want to have. But reality is rough. Especially with cryptography.
in React Native? What is stopping them from fixing that?
React Native is a big and very very slow project. I guess it will take another 10 years for them to ship spec-complaint crypto-implementation out of the box.
I had been hoping they would update their system such that RN support here would be only adding a RN override to the webcrypto code paths. In that case, it might only be a single line fix, excluding testing cruft.
Suggested solution is exactly what you've asked. Single line fix in paths.
In general, we don't have the resources to spend much time on this issue. I still don't know how to even test this in RN. Please help. A good start would be a PR that adds a RN test so we see the issues that need to be fixed.
Completely understand. It's another big environment to test and maintain... But since project is open-sourced and community contribute, sometimes we have to trust the community to add the changes and trust them to cover some of the manual testing we have to do in environments, such as react native. To make the project better. You just need to be sure it's not something over-complicated or breaking some of the other stuff. And the changes I propose exactly meet that criteria.
About my "ESM soon" comments... ahem... maybe not so soon! It's on our roadmap though.
I would put ESM as a top-level priority in your case. Non-ESM libraries these days bring too many problems to their users.
@davidlehn I can create a PR if you are okay with the proposed changes in package.json
. It's basically metadata which has zero breaking change issues.
Update - @pleerock proposedchange to package.json
has been made, is in latest npm v4.0.1. I think this issue can be closed.
Currently fails on React Native (since that environment has neither node's crypto nor the browser's
crypto.subtle.
.Instead, convert to use
isomorphic-webcrypto
lib, which also has React Native support - https://github.com/kevlened/isomorphic-webcrypto#react-native.