search

hyperledger / aries-askar

Secure storage designed for Hyperledger Aries agents.
Apache License 2.0
59 stars 45 forks source link

feat(js): use buffer instead of text-decoder #170

Closed berendsliedrecht closed 1 year ago

berendsliedrecht commented 1 year ago

This is by no means a perfect app for testing and if we would rather wait for a better testing app I can remove it from this PR and create it later.

Ideally I would like to move to use pnpm as it makes dealing with workspaces a whole lot easier. And create some way to "quickly" (still need to compile askar like 6 times...) use custom askar builds.

andrewwhitehead commented 1 year ago

Looks like the workflow needs to switch to node 18

andrewwhitehead commented 1 year ago

I'm trying to test this out but haven't gotten very far yet. A few things I've noticed (trying the ios build):

At the moment I'm stuck on this error. I think I need to build the iOS framework and put it somewhere, maybe under /ios?

Screenshot 2023-08-23 at 10 15 42

Edit: It looks like I can bypass this issue by running yarn ios instead of yarn expo > open iOS simulator, and then I was able to run the application.

berendsliedrecht commented 1 year ago

I'm trying to test this out but haven't gotten very far yet. A few things I've noticed (trying the ios build):

  • Commands like yarn build from the root directory produce an error, as they aren't defined by the app package
  • I had to add "uuid": "3.4.0" to the package dependencies to work around this issue: SDK45: Unable to resolve module uuid/v5 in expo go expo/expo#17261
  • I had to remove "@types/react" from aries-askar-react-native as it was producing duplicate type definitions

At the moment I'm stuck on this error. I think I need to build the iOS framework and put it somewhere, maybe under /ios?

Screenshot 2023-08-23 at 10 15 42

Edit: It looks like I can bypass this issue by running yarn ios instead of yarn expo > open iOS simulator, and then I was able to run the application.

Hmmm, maybe I should not have added the example app just yet. Setting it up where it is easy to change the askar libraries and build it will require some more effort.

I think for now the best we can do is just to remove the example app and create a PR later with it which will include a simple script which can build the libraries and place them in the correct location.

berendsliedrecht commented 1 year ago

I removed the example application as it was not ready to be merged. Will need to do some research for how we can properly set this up.

berendsliedrecht commented 1 year ago

@genaris any idea what is going on here? https://github.com/hyperledger/aries-askar/actions/runs/6089025819/job/16534903694 seems to refer to your patched version of the library.

genaris commented 1 year ago

@genaris any idea what is going on here? https://github.com/hyperledger/aries-askar/actions/runs/6089025819/job/16534903694 seems to refer to your patched version of the library.

I think there is an issue when yarn tries to resolve types for @2060.io/ref-napi (direct dependency) and types for ref-napi (it seems that @types/ref-array-di is requiring it). Both link to the same (as we use "@types/2060.io__ref-napi": "npm:@types/ref-napi") so yarn complains with ... is trying to unpack in the same destination "xxx" as pattern ["xxx"]. This could result in a non deterministic behavior, skipping..

In this PR, the entry for @types/ref-napi in yarn.lock was removed. If I manually re-add it, it works. Not sure yet what's the right approach here to force these types before this breaks again. Maybe we'll need to patch @types/ref-array-di as well or create an actual @types/2060.io__ref-napi in DefinitelyTyped repo?

genaris commented 1 year ago

@genaris any idea what is going on here? https://github.com/hyperledger/aries-askar/actions/runs/6089025819/job/16534903694 seems to refer to your patched version of the library.

Update on this: I've added type definitions to the packages in order to attempt types clashing. Changes in nodejs' package.json would look like this:

image

I've tested it locally (build, check-types, test, etc.) from a clean checkout and seems to work fine. Hope this can help to unblock this PR!