openwallet-foundation / credo-ts

Typescript framework for building decentralized identity and verifiable credential solutions
https://credo.js.org
Apache License 2.0
262 stars 199 forks source link

OID4VC holder module not starting in RN environment #1943

Closed MosCD3 closed 2 months ago

MosCD3 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 2 months ago

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 commented 2 months ago

https://github.com/animo/paradym-wallet

amanji commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

Yes that is correct @MosCD3, but we import express dynamically: https://github.com/openwallet-foundation/credo-ts/blob/808878decbe66909fa9568284ae10cf29201f50d/packages/openid4vc/src/shared/router/express.ts#L3

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

Opened #1946 to fix it

MosCD3 commented 2 months ago

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 commented 2 months ago

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