nicolaslopezj / meteor-apollo-accounts

Meteor accounts in GraphQL
MIT License
145 stars 35 forks source link

React Native support #26

Closed dbrrt closed 7 years ago

dbrrt commented 7 years ago

I've splitted the meteor-apollo-accounts lib into another folder, because we can't really import conditionally AsyncStorage

nicolaslopezj commented 7 years ago

I don't like having 2 separate packages..

We could use something like https://github.com/feathersjs/feathers-localstorage/

dbrrt commented 7 years ago

Do you know how to detect if an application is React-Native or not ? I'd suggest using this package to make it "hybrid" : https://www.npmjs.com/package/react-native-storage, but if you want something working with both, we'll need to find a way to adapt the logic depending if it's react or react native.

nicolaslopezj commented 7 years ago

Check here https://github.com/nicolaslopezj/simple-react-form/blob/master/src/Form/index.js#L416

dbrrt commented 7 years ago

So it works great, thanks ! I'm still stuck with the conditional import of AsyncStorage. Do you have any clue for that?

nicolaslopezj commented 7 years ago

Mmm I haven't done that. But we could to use an external library that do that job

dbrrt commented 7 years ago

Do you mind if the react native client is Apollo only (for now)? We could eventually store the local data using Redux and something like that https://github.com/rt2zz/redux-persist ? But we'll face the exact same problem because AsyncStorage is needed and seems to be the only thing to store data onto react-native. And having two libraries for different type of client is ugly tho...

janikvonrotz commented 7 years ago

Have you tried something like:

import AsyncStorage from `react-native`

if(reactNative){
  LocalStorage = AsyncStorage
}
export LocalStorage

Or why not offer the possiblity to inject a local storage?

import AsyncStorage from `react-native`
import initApp from `apollo-accounts`

initApp(AsyncStorage)
dbrrt commented 7 years ago

@janikvonrotz Because the library has to be compatible with React and React-Native, but an exclusive ReactJS will not want to install React-Native to have it working. I like the injection of Storage, might do the job. I'll try it after work, and it might be the only way to have it working on React and React-Native with one library.

nicolaslopezj commented 7 years ago

I had not thought using a react native solution would make this package react only. As @dbrrt this package should not be exclusive for react. I like @janikvonrotz idea

dbrrt commented 7 years ago

@janikvonrotz I tried the following code without any success, it crashes : ReferenceError: navigator is not defined and the require is executed automatically and makes the application crashing on a React Apollo instance without React Native installed. :

// Application crashes
var LocalStorage;

var navigatorUser = navigator;

if(navigatorUser && navigatorUser.product === 'ReactNative') {

    var RNative = require("react-native");

    var {
        AsyncStorage
    } = RNative;

    LocalStorage = AsyncStorage;
}
else {
  LocalStorage = global.localStorage;
}

The injection remains the last possibility to make it hybrid, but it's not super clean. It'd require having LocalStorage available globally and be initiated once by the client. Any help welcomed ;)

nicolaslopezj commented 7 years ago

I think something like this could be the solution

let tokenStore = {
  set: async function ({userId, token, tokenExpires}) {
    global.localStorage['Meteor.userId'] = userId
    global.localStorage['Meteor.loginToken'] = token
    global.localStorage['Meteor.loginTokenExpires'] = tokenExpires
  },
  get: async function ({userId, token, tokenExpires}) {
    return {
      userId: global.localStorage['Meteor.userId'], 
      token: global.localStorage['Meteor.loginToken'], 
      tokenExpires: global.localStorage['Meteor.loginTokenExpires']
    }
  }
}

export const setTokenStore = function (newStore) {
  tokenStore = newStore
}

export const storeLoginToken = function (userId, token, tokenExpires) {
  tokenStore.set({userId, token, tokenExpires})
  tokenDidChange()
}

And in every React Native app people should do:

// Init Meteor Accounts
import {AsyncStorage} from 'react-native'
import {setTokenStore} from 'meteor-apollo-accounts'

setTokenStore({
  set: async function ({userId, token, tokenExpires}) {
    await AsyncStorage.setItem('Meteor.userId', userId)
    await AsyncStorage.setItem('Meteor.loginToken', token)
    await AsyncStorage.setItem('Meteor.loginTokenExpires', tokenExpires)
  },
  get: async function () {
    return {
      userId: await AsyncStorage.getItem('Meteor.userId'), 
      token: await AsyncStorage.getItem('Meteor.loginToken'), 
      tokenExpires: await AsyncStorage.getItem('Meteor.loginTokenExpires')
    }
  }
})
dbrrt commented 7 years ago

It works flawlessly! Bravo @nicolaslopezj ! I updated my repository for the PR. I tried with React Native (0.39.2) onto iPhone simulator (IOS 10.2) on Mac OS Sierra. I'll publish soon my example.

dbrrt commented 7 years ago

@nicolaslopezj I fixed my commit. Let me know if you see things to edit.

nicolaslopezj commented 7 years ago

Please update the changelog too

nicolaslopezj commented 7 years ago

Looks great! did you test it in react native?

dbrrt commented 7 years ago

@nicolaslopezj, As I said yes, I tried it with React Native (0.39.2) onto iPhone simulator (IOS 10.2) on Mac OS Sierra. It seems to work normally, which is good for the future of the package.

nicolaslopezj commented 7 years ago

Please add to the README instructions to use it on RN

ghost commented 7 years ago

I'll add this after work, but except the storing part, it works the same on React and React Native.