bunqCommunity / bunqJSClient

A javascript SDK for the bunq API
MIT License
58 stars 23 forks source link

React-native: Summary for complete support #40

Closed DannyvanderJagt closed 5 years ago

DannyvanderJagt commented 5 years ago

I've got this lib working in React-Native, but in the process I ran into a few issues. Not sure if you are planning to support React-Native, in case you do:

What the issues were:

1) Haste modules: "Unable to resolve 'url' from /Users....'

It looks like this is actually a react-native metro bug, (stale react-native issue). Workaround: yarn add url to your react-native project.

2) store node_module doesn't support React-Native, gives error at import

Acutal error: "Undefined is not an object (Global.navigator ? Global.navigator.userAgent....)

When compiled to js, we are importing store in dist/BunqJSClient.js but the store module doens't support React-Native.

Workaround: Instead of using import you can use require and set the navigator before you do

global.navigator = { userAgent: '' }; 
const BunqClient = require('@bunq-community/bunq-js-client').default

3) node-forge generateKeyPair is very slow as react-native doesn’t have support for native window.crypto**

It is now using the default javascript version, which is very slow causing your app to freeze for a couple of seconds (5-10s).

Possible solution: For the React-Native developer using this lib a good alternative can be a native crypto lib, (like this). I tried to use your process.env.CI_PUBLIC_KEY_PEM and process.env.CI_PRIVATE_KEY_PEM (here) but kept getting 400 response from bunq. Not sure why.

Edit: Got a little bit close to solving the last one, I now get that you can't re-use public/private keys that is providing the 400 response. Also react-native-rsa-native is providing different length public and private keys, not sure why (both use 2048 bit). Do you think this is related?

Crecket commented 5 years ago

The store lib is a pretty simple and small library so I'll likely just recreate it to avoid this issue and to get rid of one more dependency.

It should be possible to add a method to set a custom RSA key generation callback. So you can use a different lib when required.

DannyvanderJagt commented 5 years ago

Solved to last one.

I used react-native-rsa-native which has good performance. It provides an RSA Public key which you need to convert to PUBLIC KEY. When you have the correct key you can set the process.env.CI_PUBLIC_KEY_PEM and process.env.CI_PRIVATE_KEY_PEM before using this lib and everything works great. It indeed would be cleaner if we can provide the RSA keys by callback instead of through process.

I think the current store is great for web and node. I guess if we import it in the constructor of BunqJSClient instead of where it is now, we keep the same functionality and allow others to provide their own store without a need for the re-write. What do you think?

I will create a quick summary of how to use BunqJSClient in react-native to add to the docs if that's ok with you.

Crecket commented 5 years ago

Sorry for the late response, I've been busy with my internship and other projects. I rewrote the constructor to only import the store library when no default is given as you suggested. And feel free to add a guide for react-native to the docs :+1:

The CI_PUBLIC_KEY_PEM environment variables are used for testing to prevent the 100+ tests from each creating their own private key which takes too long. It might be better if I change how that is done more properly but that'd be a breaking change so I'll wait with that for now.

Crecket commented 5 years ago

To add to this, I added two store examples to the src/Stores/* directory for both localstorage and json-file storage to be used in the code so users don't have to write them themselves in most cases.

Feel free to add a react-native compatible version there

DannyvanderJagt commented 5 years ago

No worries, same here. Thanks for the changes!

I will add it this weekend, already got it written down. I will also add the react-native version of the store.

rauldeheer commented 5 years ago

@Crecket @DannyvanderJagt Has this been added yet? I'm trying to get this library working in a React Native app.

DannyvanderJagt commented 5 years ago

I'm sorry, I completely forgot about this one. I just found the project on my computer, will test it to make sure it still works and then propose a PR and share a repo with the project.

DannyvanderJagt commented 5 years ago

@rauldeheer I created an example project and included a readme with steps to add BunqJSClient to your own project.

Note: For now use the BunqJSClient version 0.40.2, the newer versions are having an issue with a missing dns module. I will see if/how we can fix this and report back.

Crecket commented 5 years ago

Currently away until Monday so I can check it out when I get back 👍

DannyvanderJagt commented 5 years ago

No hurry, have a nice weekend! 👍

Found the problem. We are using the module socks-proxy-agent here in RequestLimitFactory.ts. This one is using a few node-js modules which we can't use in react-native.

@Crecket Looking at the history you just added it, is it a requirement for bunq or a (soon-to-be) core part of this module? Otherwise, maybe we can make it optional (and lazy require the socket-proxy-agent) to allow for RN support. What do you think?

Crecket commented 5 years ago

I'll look into making it a peer dependency or only loading it when a custom proxy is given.

Crecket commented 5 years ago

The dependency is now only required when setting a custom proxy service so it should work again for your use-case

rauldeheer commented 5 years ago

@DannyvanderJagt Thank you for providing me with the example! I'll take a look at it.

DannyvanderJagt commented 5 years ago

@Crecket Thanks for the adjustment! @rauldeheer You're welcome. If you have any questions, let me know.

I guess we can close this issue now. Glad it all worked out.