Skip to content
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

OID4VC holder module not starting in RN environment #1943

Closed
MosCD3 opened this issue Jul 7, 2024 · 13 comments · Fixed by #1946
Closed

OID4VC holder module not starting in RN environment #1943

MosCD3 opened this issue Jul 7, 2024 · 13 comments · Fixed by #1946

Comments

@MosCD3
Copy link
Contributor

MosCD3 commented Jul 7, 2024

Hi Team
I am trying to add support for OID4VC to Bifold which runs on React Native. As the docs said, out of the box, the holder module is supported on RN. Installing deps should be as straight forward as adding
"@credo-ts/openid4vc": "0.5.3",
to the dependencies. That was not the case, at first I got an error
serve-static couldn't be found in node modules.
I then installed serve-static. Now am getting this error when initializing the module

error: Error: Unable to resolve module http from ***/aries-mobile-agent-react-native/packages/legacy/app/node_modules/express/lib/request.js: http could not be found within the project or in these directories:
  node_modules/express/node_modules
  node_modules
  ../../../node_modules
  ../../../../../../../node_modules
  18 | var isIP = require('net').isIP;
  19 | var typeis = require('type-is');
> 20 | var http = require('http');

any thoughts?

@TimoGlastra
Copy link
Contributor

Thanks for reporting! I've had some other reports on similar issue. I think we have to handle imports for Node.JS specific modules a bit differently. It's weird that it somehow works in Expo, but not in bare React Native.

BTW -- I do recommend to upgrade the Bifold app to Expo, as it's now also the recommended way to manage a project (you don't have to use Expo cloud services to use the open source expo framework): https://reactnative.dev/blog/2024/06/25/use-a-framework-to-build-react-native-apps. We will update the documentation to reflect this as well (however we will keep supporting bare React Native applications of course)

@MosCD3
Copy link
Contributor Author

MosCD3 commented Jul 7, 2024

Upgrading Bifold to Expo is a whole community dilemma since Bifold now is officially the underlying engine for some publicly used wallets in many different orgs and different countries. The other issue is that we do utilize bridges in android/iOS to harness some core system features that are not available out of the box in RN packages such as biometry security policies to detect changes .. etc

@TimoGlastra
Copy link
Contributor

All these things are possible with Expo, expo is a layer on top of React Native that makes it simpler to use. It's like using Next.JS for a React project, instead of React.

I think in the long run, expo will make it a lot easier for those orgs, as expo provides easy APIs to generate and modify any part of the application using Expo Modules and Config Plugins

@amanji
Copy link
Contributor

amanji commented Jul 9, 2024

I had issues with getting credo-ts to work in Expo. I kept getting errors trying to resolve @digitalcredentials. Do we have some examples of working Expo applications?

@MosCD3
Copy link
Contributor Author

MosCD3 commented Jul 9, 2024

@amanji
Copy link
Contributor

amanji commented Jul 9, 2024

Perhaps just a problem when running in web mode?

Web Bundling failed 621ms node_modules/expo/AppEntry.js (2510 modules)
Unable to resolve "@digitalcredentials/open-badges-context" from "node_modules/@digitalcredentials/vc/lib/legacyDocumentLoader.js"

@MosCD3
Copy link
Contributor Author

MosCD3 commented Jul 11, 2024

I can see that the package has the following dependencies

`"express": "^4.18.2"`
 "nock": "^13.3.0",
    "rimraf": "^4.4.0",

Those are Node packages and I see the reason since the holder which Is meant to run on a device (React Native) is packaged along with the issuer and verifier which is meant to run on Node enviroment
and it has been introduced to the package in #1708

@TimoGlastra
Copy link
Contributor

Yes that is correct @MosCD3, but we import express dynamically:

export function importExpress() {

But appereantly the bundler for React Native doesn't play well with this.

I think what we have to do is to have a express.native.ts and the importExpress will throw an error when called, as it should never called in react native

@TimoGlastra
Copy link
Contributor

Could you try in your node_modules copy node_modules/@credo-ts/openid4vc/build/shared/router/express.js to node_modules/@credo-ts/openid4vc/build/shared/router/express.native.js and update the implementation of importExpress to throw an error, but not require express package? If that works we can create a PR

@TimoGlastra
Copy link
Contributor

I was able to reproduce this in our Expo app as well now. So I'm not sure what changed as we've been using this for quite a while, but something about the loader / bundler must have changed causing this.

@TimoGlastra
Copy link
Contributor

Opened #1946 to fix it

@MosCD3
Copy link
Contributor Author

MosCD3 commented Jul 15, 2024

Could you try in your node_modules copy node_modules/@credo-ts/openid4vc/build/shared/router/express.js to node_modules/@credo-ts/openid4vc/build/shared/router/express.native.js and update the implementation of importExpress to throw an error, but not require express package? If that works we can create a PR

this solution worked

@TimoGlastra
Copy link
Contributor

Okay great to hear @MosCD3 . Then once #1946 has been merged and released it'll be fixed.

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

Successfully merging a pull request may close this issue.

3 participants