Closed Meligy closed 6 years ago
I think rather than alias require
, the best idea is to remove node_support
modules from the barrel - until we have a better solution in place.
That will work well yes.
We'll still need to do something about crypto_utils.ts
. Easiest way is to alias require
only in this file. Another way is to split it into a browser and a node version.
Which option would make more sense to you?
Yes, let's split it into 2 files, and add the node
specific version to node_support
. We can add an interface for it and then provide 2 different implementations. Make both versions of the AuthorizationRequestHandler use the appropriate versions.
so it supports React Native now? Is there any sample code to do that?
No official support, no.
It should work though, with some hacks in the way you use it.
I wanted to use it in a project that was swapping ID providers and considered moving away from a utility library that came from the old provider (sorry I cannot provide more details).
After this issue was solved, I was able to get a PoC working by using the browser redirect request handler in the library. Since everything is pluggable in the library (great work!), I was able to create an object tha looked like window.location and in fact synced url to the it'll of a Text Native web view.
It looked like it was working, but I couldn't confirm much as my project plan changed to something else instead of this work. That's why I cannot confirm, and that's what I mean by some hacks.
One thing I haven't had time to confirm for example is that the data in the JWT token comes correctly.
As I said I'm not actively working on this, and also my React Native skills are near none. I might try to get a sample end of next week, but no promises.
I promise to reply to you and have a look at / try to help with any issue you may find though if you want to give it a spin. Just tag me.
Hi @Meligy, did you end up getting a POC for RN? I'm working on one now. I run into the 'Unable to resolve crypto from crypto_utils' red screen. Will continue to work on it.
Also, a sample React Native app would be very useful, so would any documentation for implementing the AppAuth for a React Native app.
I wrote an article for scotch.io last week that shows you how to use React Native App Auth to add authentication to a React Native application. Hope this helps! https://scotch.io/tutorials/build-a-react-native-app-and-authenticate-with-oauth-20
@paulbrittain I haven't got to it yet and won't be very soon sorry. I think the problem has been resolved by replacing the random generator function in every class that required it with something similar to this https://github.com/okta/okta-auth-js/blob/master/lib/util.js#L108
A friend shared @mraible article with me this week though and thi is going to be more interesting option to try for mobile IMO, but I haven't checked out the details yet. Give it try and share the experience with us.
@mraible Interesting article, unfortunate that the iOS setup is in Objective-C, why not Swift? It makes it very difficult to setup in a modern xcode project
@paulbrittain That's a good question for Formidable, the folks that run React Native App Auth.
@paulbrittain Did you get this working with react-native? I'm going through the same process as I intend to use it in a react-native windows application, and was wondering if you had the POC I could compare to
Sorry mate @afonsopbarros, this was year ago, and I don't have access to the project code or recollection of what we ended up doing for it.
Hello, this is a new feature request!
I'm trying to get this library to work with React Native, and having a few issues. I'd love to work with the team on improving this if allowed so they can keep focusing on other priorities.
After implementing barrels, I discovered a couple of issues caused by a (IMHO very silly) React Native behaviour: if it sees any
require('...')
call, it tries to load the module in thisrequire
. It doesn't matter whether this code is even insideif(false)
ortry { ... } catch { ...}
. Since most of theserequire()
s call built in Node stuff, and React Native runtime is not Node, it fails. See https://github.com/facebook/react-native/issues/5079 for reference.So, it sees stuff from
node_support/
andcrypto_utils.ts
, and shows an error for loading them, even if I'm not using these.Each of these 2 can have different ways to handle.
crypto_utils.ts
In
crypto_utils.ts
, the line that breaks it islet crypto = require('crypto');
. We can workaround the issue by doing something like:This is sweet and easy, and if you approve, I'll create a quick PR for it.
node_support
The contents of
node_support
is meant to be for Node. We can use the same workaround as above, but it's not clean and quite silly actually.If you approve, again, it might be a temporary solution.
Another temporary solution is to remove
node_support
from the main barrel. This way Node users will have to import from@openid/appauth/built/node_support
(which has its own barrel), but having to import frombuilt
is quite silly.The reason we have this issue is that TypeScript cannot have a base folder for looking for submodules except the package root - see https://github.com/Microsoft/TypeScript/issues/8305#issuecomment-343003095 as mentioned in https://github.com/openid/AppAuth-JS/pull/34#issuecomment-343003850
we can solve the
built
issue by compiling in-place instead of havingsrc
andbuilt
folders. Or, when publishing the package, implement a workflow where we copy thepackage.json
file tobuilt
folder and publishing from inside that folder.I can have a spin at this workflow (publishing from
built
but I cannot test it as I don't have publish permissions. So, we either:require
calls innnode_support
for nownnode_support
from the root barrel for nowbuilt
Happy to do pull request for any option you choose, or provide better clarification if this issue is a bit overwhelming.