Skip to content

Issue #462 fix : Add new method that returns the logged-in user as Promise : #4288

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

harshshredding
Copy link

@harshshredding harshshredding commented Jan 13, 2021

Issue #462
Hi, firebase team! I am sure this pull-request is far from complete since I lack an understanding of the codebase.

onAuthStateChanged has the following caveats that seem to be making it difficult for new-comers to get started with firebase-auth :

  • The method name doesn't clearly indicate a very common use-case: getting the current user info. Also, the method name implies that some state has to be changed in order for the callback to be triggered, but the callback is also triggered when auth is not pending (no user is logged in).
  • Callbacks have caveats when compared to promises, which can be chained and utilized with the async keyword.

I was asked to tag @avolkovi and @bojeil-google.

@changeset-bot
Copy link

changeset-bot bot commented Jan 13, 2021

⚠️ No Changeset found

Latest commit: 6c56c98

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@google-cla
Copy link

google-cla bot commented Jan 13, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla
Copy link

google-cla bot commented Jan 13, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla
Copy link

google-cla bot commented Jan 14, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla
Copy link

google-cla bot commented Jan 14, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@sam-gc
Copy link
Contributor

sam-gc commented Jan 28, 2021

Hi @harshshredding, thank you very much for this pull request! API changes of this nature go through an internal process for approval. I'm sponsoring this and will get back to you in a few weeks!

@sam-gc
Copy link
Contributor

sam-gc commented Jun 3, 2021

Hi @harshshredding, thanks so much for your patience. After an internal review, we've concluded that this function should not itself return a user object. Instead, the function should return a Promise<void> that resolves when the auth state is ready and settled (i.e. when reading auth().currentUser is valid).

That function should be called authStateReady() and have the following signature:

interface Auth {
  authStateReady(): Promise<void>;
}

The function could then be used like:

async function doStuff() {
  await firebase.auth().authStateReady();
  nowSafeToRead(firebase.auth().currentUser);
}

I believe your function in this PR can be easily updated to do the same thing but simply not resolve with any value.

@harshshredding
Copy link
Author

@sam-gc I will update the PR very soon. Thanks a lot for the feedback.

@Deliana90
Copy link

wow

@harshshredding harshshredding removed their assignment Feb 17, 2023
@Malix-Labs
Copy link

@harshshredding

#4288 (comment)

???
What happened?

@harshshredding
Copy link
Author

Sorry guys, I don't have the bandwidth. @sam-gc can you assign this to someone else?

@prameshj
Copy link
Contributor

prameshj commented Jul 5, 2023

Sorry guys, I don't have the bandwidth. Sam can you assign this to someone else?

We got API review feedback to expose this slightly differently. We are working on it in #7384

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants