fullstackreact / react-native-firestack

A firestack v3 react-native implementation
MIT License
715 stars 132 forks source link

persisting auth state in salakar's branch - refresh necessary to get correct auth state #153

Closed florianbepunkt closed 7 years ago

florianbepunkt commented 7 years ago

With Salakar's fork I noticed following behaviour which might be a bug or something intentional:

I have a fresh react-native project on iOS (simulator). in my index.ios I have a login function and an auth state listener (see below).

Login works fine and auth state listener triggers. When I now exit the app in simulator and reopen it again user is not loggedin (state.user is null). However when I hit Command + R to refresh, user is logged in.

Exspected behaviour: User would be loggedin when reopening the app without the need of additional refresh.

componentDidMount() {
    this.unsubscribe = firestack.auth().onAuthStateChanged((user) => {
      if (user) {
        Alert.alert('loggedin');
        // User is signed in.
        this.setState({user});
      }
      else {
        // use is not signed in
        this.setState({user: ''});
      }
    });
  }
login() {
    console.log('login func called');
    firestack.auth().signInWithEmailAndPassword('mail@florian-bischoff.de', '123456')
      .then((user) => {
        console.log('logged in user. ', user);
        Alert.alert('logged in user');
      })
      .catch((ex) => {
        console.error('error during login', ex.message);
        Alert.alert('Error during login', ex.message);
      })
  }
render() {
    return (
      <View style={styles.container}>
        <Text style={styles.welcome}>
          Welcome to React Native {this.state.user.uid}!
        </Text>
        <Button title="Login" onPress={this.login}></Button>
      </View>
    );
  }
Salakar commented 7 years ago

@chrisbianca this is why i was calling getUser in the auth constructor ;p think we'll need to add that logic back in, otherwise the user isn't there on boot. Thoughts?

florianbepunkt commented 7 years ago

@Salakar What was the reason why it was removed? If you point me to the file I can try iny my project if this fixes it. Thank you.

Salakar commented 7 years ago

@florianbepunkt this was the commit that changed it: https://github.com/Salakar/react-native-firestack/commit/91ace7a974b9ea2147e595647d111db4db8dd5e9#diff-2e22bff7808c41e1f24596e25ab3bfb9 - if you could change auth.js back on your local node modules version and let me know if that fixes it?

chrisbianca commented 7 years ago

@florianbepunkt the reason I removed it is that it means two auth requests are sent unnecessarily. Could you try moving your auth listener to componentWillMount and see if this fixes it?

chrisbianca commented 7 years ago

@florianbepunkt I've just double checked on my iOS build and I am not seeing this problem. The only difference between your code and mine is that I start listening in the componentWillMount method rather than componentDidMount.

Ehesp commented 7 years ago

@florianbepunkt what's in your constructor/componentWillMount()?

Salakar commented 7 years ago

@florianbepunkt if doing it in willMount works then I'd say go for that, i'll update the example in auth then =]

Ehesp commented 7 years ago

Try like so:

constructor() {
    super();
    const state = {
        user: null,
    }
    this.unsubscribe = firestack.auth().onAuthStateChanged((user) => {
      if (user) {
        state.user = user;
      }
      else {
        // use is not signed in
        state.user = null;
      }
    });

    this.state = state;
  }
florianbepunkt commented 7 years ago

First of all, thank you all for your help.

Using componentWillMount() putting the state listener in componentWillMount() does not work. If I (completely) exit the app and start it again, I'm not logged in until I refresh (command + r). My testfile is here:

http://pastebin.com/Upt99qkw

@Salakar I added the following back in constructor of auth.js in firestack lib but to avail, same behaviour


this.getCurrentUser().then((u: Object) => {
      this._onAuthStateChanged({ authenticated: !!u, user: u || null });
    }).catch(() => {
      // todo check if error contains user disabled message maybe and add a disabled flag?
      this._onAuthStateChanged({ authenticated: false, user: null });
    });
chrisbianca commented 7 years ago

Ok, so one thing I am doing which may hide this in my app is that I display an "initialising" screen until I receive my first response from Firestack/Firebase. Essentially, until I receive an event in onAuthStateChanged, the user is in an unknown state;

There's invariably some initialisation time in the native code whilst it validates the saved token, so this needs to be taken into account.

Try something like this:

  constructor() {
    super();
    const state = {
        user: null,
        initialised: false
    }
  }
  componentWillMount() {
    this.unsubscribe = firestack.auth().onAuthStateChanged((user) => {
      if (user) {
        Alert.alert('loggedin');
        // User is signed in.
        this.setState({user, initialised: true});
      }
      else {
        // user is not signed in
        this.setState({user: null, initialised: true});
      }
    });
  }
 render() {
    const { initialised, user } = this.state;

    if (initialised)
      return (
        <View style={styles.container}>
          <Text style={styles.welcome}>
            Welcome to React Native {user ? user.uid : ''}!
          </Text>
          <Button title="Login" onPress={this.login}></Button>
        </View>
      );
    } else {
        <View style={styles.container}>
            <Text>Initialising</Text>
        </View>
    }
  }
Salakar commented 7 years ago

@chrisbianca not sure if this is a stupid idea but could we not just ignore the first auth event JS side and never send to user's listeners? The first event is always going to be before initialised anyway?

In your example above there's no way to tell if the user is genuinely not signed in at app boot - as the auth listener sends null if no authenticated user, as per web api?

florianbepunkt commented 7 years ago

@chrisbianca Thanks. I did this, but the underlying problem remmains: Using your approach I still need to do a rfresh after reopening the app...otherwise the "initialising" message will stay on forever. I had to modify your code to get rid of some red death screens..

What I do is: run the app with react-native run-ios, then login. the in simulator i hit the home button twice, swipe the app up to close it. then reopen the app.


constructor() {
     super();

     this.state = {
        user: null,
        initialised: false
     };
   }

   componentWillMount() {
     this.unsubscribe = firestack.auth().onAuthStateChanged((user) => {
       if (user) {
         Alert.alert('loggedin', user.uid);
         // User is signed in.
         this.setState({user, initialised: true});
       }
       else {
         // user is not signed in
         this.setState({user: null, initialised: false});
       }
     });
   }

  render() {
     // const { initialised, user } = this.state;

     if (this.state.initialised) {
       return (
         <View style={styles.container}>
           <Text style={styles.welcome}>
             Welcome to React Native {this.state.user ? this.state.user.uid : ''}
           </Text>
           <Button title="Login" onPress={this.login}></Button>
         </View>
       )
     } else {
        return (
         <View style={styles.container}>
             <Text>Initialising</Text>
         </View>
         )
     }
   }
chrisbianca commented 7 years ago

@florianbepunkt Just noticed there was a typo in my example, the onAuthStateChanged should set initialised to true, regardless of the response. I have updated my comment above, but for clarity:

  componentWillMount() {
     this.unsubscribe = firestack.auth().onAuthStateChanged((user) => {
       if (user) {
         Alert.alert('loggedin', user.uid);
         // User is signed in.
         this.setState({user, initialised: true});
       }
       else {
         // user is not signed in
         this.setState({user: null, initialised: true});
       }
     });
   }

@Salakar the first event isn't sent by the native code until Firebase has initialised, therefore it is the source of truth for whether the user is signed in at application boot. If you were to ignore the first event, how would you be able to establish the state of the user?

chrisbianca commented 7 years ago

@Salakar to follow this up, the Firebase web documentation makes it clear that listening to the authStateChanged event is the recommended way to get the user to ensure that it's not in an intermediary state: https://firebase.google.com/docs/auth/web/manage-users#get_the_currently_signed-in_user

Equally, the same is true for iOS and Android

Salakar commented 7 years ago

@chrisbianca you're right, it's one of those days sorry ;p I clearly need more caffeine

florianbepunkt commented 7 years ago

@chrisbianca Okay, this is weird. I updated my code with your change. Basically I would assume, that initialised would be set to true now at some point anyway, right?

But this never happens… If I hit refresh, it works, but If i exit and reopen the app, initialise stays false forever...

chrisbianca commented 7 years ago

@florianbepunkt just to check, you do definitely have the very latest version of Salakar's fork don't you?

What seems to be happening, is that when launching from scratch, you're missing the very first auth event from the native code. I had a similar issue at the end of last week and it was simply that the listener was being added in the JS after requesting the auth information. I updated this on Friday so you should have the change, but can you check in auth.js that the constructor looks like this:

  constructor(firestack: Object, options: Object = {}) {
    super(firestack, options);
    this._user = null;
    this._authResult = null;
    this.authenticated = false;

    // start listening straight away
    // generally though the initial event fired will get ignored
    // but this is ok as we fake it with the getCurrentUser below
    FirestackAuthEvt.addListener('listenForAuth', this._onAuthStateChanged.bind(this));
    FirestackAuth.listenForAuth();
  }

The important part is that the listener gets added before the listenForAuth() method gets called...

florianbepunkt commented 7 years ago

@chrisbianca I cloned his repo an hour before I opened this issue. I just checked and I can confirm, the constructor looks like yours.

chrisbianca commented 7 years ago

That's strange. I've just tried again to reproduce this, but I'm having no luck at all!

Are you able to put some break points in: 1) In the iOS code within the listenForAuth method callback (should be line 135) 2) In the JS code within the onAuthStateChanged callback

You're looking to check that firstly the event is received in the listenForAuth method in the native code, and secondly that it makes it through to the JS code successfully.

florianbepunkt commented 7 years ago

the iOS part is beyond my capabilities I'm afraid.

What I did: I added a console.log in react-native-firestack/lib/modules/auth.js in _onAuthStateChanged(auth: AuthResultType)method as well as in onAuthStateChanged(listener: Function)

so code now looks like this…

 _onAuthStateChanged(auth: AuthResultType) {
    console.log('_onAuthStateChanged auth', auth);
    this._authResult = auth;
    this.authenticated = auth ? auth.authenticated || false : false;
    if (auth && auth.user && !this._user) this._user = new User(this, auth);
    else if ((!auth || !auth.user) && this._user) this._user = null;
    else if (this._user) this._user._updateValues(auth);
    this.emit('onAuthStateChanged', this._authResult.user || null);
  }

  /*
   * WEB API
   */

  /**
   * Listen for auth changes.
   * @param listener
   */
  onAuthStateChanged(listener: Function) {
    console.log('onAuthStateChanged called', listener)
    this.log.info('Creating onAuthStateChanged listener');
    this.on('onAuthStateChanged', listener);
    if (this._authResult) listener(this._authResult.user || null);
    return this._offAuthStateChanged.bind(this, listener);
  }

what happens on refresh is that both console.logs are printed. when I restart from scratch, only the onAuthStateChanged console.log is printed, the private method is never called.

florianbepunkt commented 7 years ago

@chrisbianca

okay, i read up on how to set breakpoints in ios. I set the following breakpoint (see screenshot)

when I launch the app, the first breakpoint gets called, the others not. when I refresh in app (command + r), all breakpoints are called.

bildschirmfoto 2016-11-21 um 17 52 27
Salakar commented 7 years ago

@chrisbianca @florianbepunkt maybe this is something to do with react - doesn't react only reload the JS bundle on Cmd + r - the native modules still remain running - so it won't resend the event, I have no idea, thought it did?

florianbepunkt commented 7 years ago

according to the docs it only reloads javascript (https://facebook.github.io/react-native/docs/debugging.html) . but the auth state listener should return something on app start.

unsure, why this issue seems not reproducable... shall I upload my smaple project with the credentials somewhere private? banging my head against a wall here :/

chrisbianca commented 7 years ago

Have you checked through the iOS logs to see if there are any errors being reported at all?

Also, can you move the second breakpoint up a line so that it's on the if (user != nil) line (132) and see whether it's triggered on the restart path?

florianbepunkt commented 7 years ago

@chrisbianca Thanks for sticking with me. I moved the second breakpoint. On first launch it never gets called. When I hit Cmd+R all breakpoints are called. When I close the app, the debug process gets detached.

Regarding the logs… in the simulator I used Debug > Open system log. Output is here: http://pastebin.com/FgrsPetn

chrisbianca commented 7 years ago

So there's one line that interests me in the logs:

Nov 21 18:47:54 MacBook-Pro stackTest[28880]: [Firebase/Core][I-COR000003] The default Firebase app has not yet been configured. Add [FIRApp configure] to your application initialization. Read more: https://goo.gl/ctyzm8.

Can you try adding the following to the didFinishLaunchingWithOptions method in AppDelegate.m:

[FIRApp configure];

You may also need to add this near the top of AppDelegate.m if you haven't got the import already:

#import <Firebase.h>
florianbepunkt commented 7 years ago

I modified ./ios/stackTest/AppDelegate.m (hope this is the right one, stackTest is my app name).

the file looks like this now


#import "AppDelegate.h"

#import "RCTBundleURLProvider.h"
#import "RCTRootView.h"
#import <Firebase.h>

@implementation AppDelegate

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{
  [FIRApp configure];
  NSURL *jsCodeLocation;

  jsCodeLocation = [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index.ios" fallbackResource:nil];

  RCTRootView *rootView = [[RCTRootView alloc] initWithBundleURL:jsCodeLocation
                                                      moduleName:@"stackTest"
                                               initialProperties:nil
                                                   launchOptions:launchOptions];
  rootView.backgroundColor = [[UIColor alloc] initWithRed:1.0f green:1.0f blue:1.0f alpha:1];

  self.window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
  UIViewController *rootViewController = [UIViewController new];
  rootViewController.view = rootView;
  self.window.rootViewController = rootViewController;
  [self.window makeKeyAndVisible];

  return YES;
}

@end

import was not set

the logs for this one are here, but the "not configured" line is still there and the issue still persists: http://pastebin.com/KNuvPu9p

I have to leave university now since they close for today, but I'll be back tomorrow. I appreaciate all the time you invest helping.

florianbepunkt commented 7 years ago

@Salakar @chrisbianca

Just wanted to follow up on this since I did some more testing this morning. IMO the problem is definitely linked to your fork, Michael, or me making a mistake when copying it. I tried with the original react-native-firestack repo and the problem does not occur. When I download your fork and use it to replace the files in /node_modules/react-native-firestack I see the problem described above.

Here are my steps with a clean react-native install and a fresh project (RN 0.37.0)

react-native init appName
cd appName
npm install react-native-firestack --save

Now I manually create a Pofile in /ios with this content


source 'https://github.com/CocoaPods/Specs.git'
platform :ios, '8.0'

def firestack_pods
  [
    'Firebase',
    'Firebase/Core',
    'Firebase/Auth',
    'Firebase/Storage',
    'Firebase/Database',
    'Firebase/RemoteConfig',
    'Firebase/Messaging'
  ].each do |lib|
    pod lib
  end
end

target 'appName' do
  firestack_pods
end

then I link the library and update my pods

react-native link react-native-firestack
cd ios && pod update --verbose

last step is to setup my app in firebase. then I drag GoogleService-Info.plist to Xcode below appName, in prompt I check "Copy items if needed", leave "added folders" at "Create groups" and make sure appName is checked in "Add to targets"

Now I have working version of the original react-native-firestack. Everything works as exspected, auth state correctly persists and I can exit the app and restart it.

For the sake of completeness, here is my index.ios.js I use for testing (note that I change the firestack methods according to whether I use the original firestack or Michael's fork, below is for the latter)


/**
 * Sample React Native App
 * https://github.com/facebook/react-native
 * @flow
 */

import React, { Component } from 'react';
import {
  AppRegistry,
  Alert,
  Button,
  StyleSheet,
  Text,
  View
} from 'react-native';

import Firestack from 'react-native-firestack'

const configurationOptions = {
  debug: true 
};
const firestack = new Firestack(configurationOptions);
firestack.on('debug', msg => console.log('Received debug message', msg))

export default class appName extends Component {

  constructor(props) {
    super(props);

    this.state = {
       user: null,
       initialised: false
    };
  }

  componentWillMount() {
    this.unsubscribe = firestack.auth().onAuthStateChanged((user) => {
      if (user) {
        Alert.alert('loggedin', user.uid);
        // User is signed in.
        this.setState({user, initialised: true});
      }
      else {
        // user is not signed in
        this.setState({user: null, initialised: true});
      }
    });
  }

  render() {
    if (this.state.initialised) {
      return (
        <View style={styles.container}>
          <Text style={styles.welcome}>
            Welcome to React Native {this.state.user ? this.state.user.uid : ''}
          </Text>
          <Button title="Login" onPress={this.login}></Button>
        </View>
      )
    } else {
      return (
        <View style={styles.container}>
       <Text>Initialising</Text>
      </View>
      )
    }
  }

  login() {
    console.log('login func called');
    firestack.auth().signInWithEmailAndPassword('mail@florian-bischoff.de', '123456')
      .then((user) => {
        console.log('logged in user. ', user);
        Alert.alert('logged in user');
      })
      .catch((ex) => {
        console.error('error during login', ex.message);
        Alert.alert('Error during login', ex.message);
      })
  }

}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
    backgroundColor: '#F5FCFF',
  },
  welcome: {
    fontSize: 20,
    textAlign: 'center',
    margin: 10,
  },
  instructions: {
    textAlign: 'center',
    color: '#333333',
    marginBottom: 5,
  },
});

AppRegistry.registerComponent('appName', () => appName);

Now I change to Michael's fork by downloading it from https://github.com/Salakar/react-native-firestack. then I delete the copy of appName/node_modules/react-native-firestack and copy the files from Salakars git.

Hope this helps. If you read through this issue you'll see that the problem probably lies within the ios portion..

chrisbianca commented 7 years ago

@florianbepunkt thanks for the detailed instructions, I'll try and work through them and see if I can reproduce the issue.

As an FYI, you can install directly from the fork by using the following:

npm install --save https://github.com/Salakar/react-native-firestack.git

chrisbianca commented 7 years ago

Ok, so this is a bit random. I saw exactly what you've been describing the first time I launched the app on the simulator.

However, I can't for the life of me get it to happen a second time. Every time I close the app, re-launch it, re-install it, even re-install it to a different simulator, it works perfectly!

florianbepunkt commented 7 years ago

Okay, to add to the odditiy of this problem: Installing it directly from npm like you sugessted made the problem disappaear for me (also when opening app first time in sim)

Can I get you guys a coffee or beer or something? I appreciate all your help on this.

chrisbianca commented 7 years ago

So presumably, by not using npm to install the app, something is being missed out from the setup which causes it to break. I've just recreated the project from scratch and everything worked first time! Very strange, but are you happy this is resolved?

Happy to help!!

florianbepunkt commented 7 years ago

Yes, happy this is resolved! Thank you! So this is CLOSED

Salakar commented 7 years ago

@florianbepunkt sorry you had problems with this, I should of probably made it clear to install via npm git, glad its sorted now though!

Side note, you may want to switch to the v3 branch on here as opposed to my fork - my fork can break sometimes if me or someone else pushes some potato code to it 🍠 - it's going fast so bound to happen. v3 branch on here is updated from my fork - but only when its stable-ish. (going to push another lot up tomorrow to the v3 branch).

florianbepunkt commented 7 years ago

@Salakar I will do that, thanks. Is there a roadmap somewhere for this project? I noticed small things here and there like different syntax from web api with some methods or stuff not working, but was not sure whether this is an issue or a side effect of the project being rehauled to mimic web js api syntax of firebase

Salakar commented 7 years ago

not sure whether this is an issue or a side effect of the project being rehauled to mimic web js api syntax of firebase

@florianbepunkt it's a bit of both really - feel free to create issues on here and prefix the title with [v3]- they'll get picked really quickly and are super helpful for me; or you could also send PR's over to my fork.

Roadmap though - urm not really (yet), however there is this milestone - the main focus (for me at least right now) is refactor to mirror the web api as close as possible and add anything missing from already implemented API modules i.e. https://github.com/fullstackreact/react-native-firestack/issues/118 (which is a perfect example of how to submit an issue for things missing/not in line with web api), after that it's complete the half implemented/started modules such as presence and remoteConfig.

Once that's all done and docs updated then my vote is release as v3.0.0, any missing modules can then be added in later releases.

SamMatthewsIsACommonName commented 7 years ago

I'm also having this issue currently, I installed via npm too :/ Any ideas what might be causing it? Basically the first time the app loads on either my phone or the simulator and I log in the auth state listener doesn't pick it up. Then if I reload the javascript it picks it up fine. Edit: same f I close the app and re-open it, won't pick up auth state.

SamMatthewsIsACommonName commented 7 years ago

OK, so if anyone has the same problem I've made a solution. Basically I have a separate 'auth user' piece of state which is just for the authenticated user object. Combining online presence, I know the user must be there so it's safe to use auth().getCurrentUser(). So basically I check if there's not already a user (i.e the auth listener failed initially), and then pull it when I listen to the presence like so:

export const startListeningForPresence = (user) =>
((dispatch) => {
  const connectedRef = firestack.database().ref('.info/connected');
    connectedRef.on('value', snap => {
      if (snap.val() == true) {
        if (!user) {
          firestack.auth().getCurrentUser()
          .then((user) => {
            if (user) {
              dispatch({
                type: UPDATE_AUTH_USER,
                user,
              });
            }
          })
          .catch(err => console.log('error reloading user, ', err));
        }
        console.log('user connected');
        dispatch({ type: USER_CONNECTED });
      } else {
        console.log('user disconnected');
        dispatch({ type: USER_DISCONNECTED });
      }
    });
});

Hope that's helpful for someone :)