Skip to content
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

Global _signinPromise means only one method call can happen at a time #460

Closed
esprehn opened this issue Jul 18, 2018 · 1 comment
Closed

Comments

@esprehn
Copy link

esprehn commented Jul 18, 2018

Global _signinPromise Promise means that you can never call multiple methods at the same time. I think you want to use a final variable that's a wrapper around the Promise variable that you can then use inside the callbacks. This can happen for example if two parts of the code call currentUserAsync() at the same time. Only one will get a reply.

Note that you must not resolve promises more than once even though that's fine in JS since RN's Java Promise is implemented wrong: facebook/react-native#20262

Something like:

class PromiseResolver {
  Promise promise;
  PromiseResolver(Promise promise) {
    this.promise = promise;
  }
  resolve(Object value) {
     if (!promise) return;
    promise.resolve(value);
    promise = null;
  }
  reject(String code, String message) {
     if (!promise) return;
    promise.reject(code, message);
    promise = null;
  }
}

and then inside each method like configure(ReadableMap config, Promise promise) or signOut(Promise promise) you'd do:

final PromiseResolver resolver = new PromiseResolver(promise);

and use that resolver inside the Runnable on the UI thread. signIn() is the only one with an activityResult where you need to use a global. For that I'd suggest rejecting if a duplicate request comes in so no one is ever left hanging. So at the top of signIn() you'd have:

  if (_signInResolver) {
   promise.reject(MODULE_NAME, "Already doing signin.")
   return;
  }

Steps to Reproduce

GoogleSignIn.currentUserAsync().then(user => console.log(1, user));
GoogleSignIn.currentUserAsync().then(user => console.log(2, user));

It'll only log once the 2 because the global _signinPromise is overridden.

Expected Behavior

Can make multiple calls to methods like currentUserAsync() at the same time.

Actual Behavior

Only the last method call on the entire module ever gets a response.

Environment

n/a.

@jozan
Copy link
Collaborator

jozan commented Aug 4, 2018

Thanks for reporting! We've addressed this problem in #459. It's on master now but all those changes are not properly documented, yet. This will be included in to the next release!

@jozan jozan closed this as completed Aug 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants