-
Notifications
You must be signed in to change notification settings - Fork 30
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
[React Native] Not working #9
Comments
@stereodenis not sure where your error came from, but this library uses localstorage and is therefore currently incompatible with react native You could make a PR or fork this library. Then use AsyncStorage for storing the tokens and this react-native compatible library to decode them. |
We're not married to localStorage |
There’s no cross-platform (web and React native) solution for local storage. In order to make this library RN-compatible, we’d have to implement a check to see what environment the code is running on and use React Native’s AsyncStorage instead of window.localStorage if the answer is react-native. I think this can be done with https://stackoverflow.com/a/39473604/946872 Additionally we may have to switch to the jwt-decode library to decode the tokens. |
I've forked this library and plan to publish it as react-native-axios-jwt: https://github.com/mvanroon/react-native-axios-jwt/ The reason why I went with a fork instead of a pull request:
|
I don't see anything wrong with making everything an async function. Just call it v2.0.0 |
I think recombining both libraries would quickly result in messy code (if-elsing between both storage solutions). But feel free to take a stab at it. |
I took a stab at combining the LocalStorage and AsyncStorage codebases using React's built-in platform extensions (https://reactnative.dev/docs/platform-specific-code#platform-specific-extensions). The example is in my forked repo. Realistically, all it does currently is import either LocalStorage or AsyncStorage based on platform, and alias' both with the same name to make the actual index.ts import a single line. Unfortunately, this would also mean that anything depending on LocalStorage currently (which would be around 3/4's of the code) would need to be async. In my linked repo I've taken care of this, so that you can see what exactly would have to be updated. That being said, I'm significantly more in favor of this approach as opposed to a completely separate fork that shares most of its code with the (still maintained) upstream repo. EDIT: Editting to address the JWT library. I completely missed that the current library isn't RN compatible. I think that the library @mvanroon suggested is a good drop-in replacement; I'll add that to what I have when I have the time. |
Just DI the storage instead of hard coding it. Provide an interface for tokens storage. |
that's a great idea! - maybe have it default to localstorage so it can be omitted |
I went with a slightly different approach: #29 |
The text was updated successfully, but these errors were encountered: