Skip to content

fix: Auth env detection for capacitor #6236

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

Merged
merged 7 commits into from
May 23, 2022
Merged

fix: Auth env detection for capacitor #6236

merged 7 commits into from
May 23, 2022

Conversation

riderx
Copy link
Contributor

@riderx riderx commented May 5, 2022

No change to current codebase is just addition to make the SDK work as expected in capacitor app, like in Cordova app

@changeset-bot
Copy link

changeset-bot bot commented May 5, 2022

🦋 Changeset detected

Latest commit: 0de2c76

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@firebase/auth-compat Patch
firebase Patch

Not sure what this means? Click here to learn what changesets are.

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

@google-cla
Copy link

google-cla bot commented May 5, 2022

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

For more information, open the CLA check for this pull request.

Copy link
Contributor

@sam-gc sam-gc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR. Have you verified this works in a capacitor project?

@riderx
Copy link
Contributor Author

riderx commented May 9, 2022

@sam-gc it's a solution was proposed and test in the issue #5020 (comment)

Copy link
Contributor

@sam-gc sam-gc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again!

@riderx
Copy link
Contributor Author

riderx commented May 20, 2022

@avolkovi @lisajian @yuchenshi, i know you have ton of work but please take 5 min to review it.
That a tiny one who will unlock all capacitor/ionic devs and close complains :)

@sam-gc
Copy link
Contributor

sam-gc commented May 20, 2022

Hi @riderx, it looks like the formatting check failed. Please run yarn format and we'll rerun the analysis

@riderx
Copy link
Contributor Author

riderx commented May 20, 2022

@sam-gc i did yarn lint and yarn format and there no change.
The ci For formatter fail at the step Check for changes (fail if so) witch is normal no ?
Edit: ok i read the message better and i see the issue, but i cannot reproduce in local, i'm trying to find why

@sam-gc
Copy link
Contributor

sam-gc commented May 20, 2022

@riderx yeah it appears that file is updated. I'm rerunning the status checks

@riderx
Copy link
Contributor Author

riderx commented May 20, 2022

@sam-gc ok i fixed with the feedback of the CI
In local i have no issue i don't know why
Screenshot 2022-05-20 at 20 15 50

@riderx
Copy link
Contributor Author

riderx commented May 20, 2022

@sam-gc the test have failed again and i fixed again, i still don't know why i cannot reproduce in local, sorry about that.
If you can allow again all should be good now

@sam-gc
Copy link
Contributor

sam-gc commented May 20, 2022

Okay it's looking good, @riderx would you mind adding a changeset before mergning (see the comment up above for instructions).

@riderx
Copy link
Contributor Author

riderx commented May 20, 2022

Of course @sam-gc i will :)

@riderx riderx requested a review from egilmorez as a code owner May 20, 2022 21:16
@riderx
Copy link
Contributor Author

riderx commented May 20, 2022

@sam-gc done, i hope i understand it well :)

@sam-gc
Copy link
Contributor

sam-gc commented May 20, 2022

Looks great, thanks again! Once the workflow run completes please feel free to merge when the button is enabled

@riderx
Copy link
Contributor Author

riderx commented May 21, 2022

@sam-gc one CI is still failing it says the token is missing, so i can't merge myself

@sam-gc
Copy link
Contributor

sam-gc commented May 23, 2022

Odd, I don't know why it's blocking you but I'll go ahead and merge.

@sam-gc sam-gc merged commit b6196d2 into firebase:master May 23, 2022
@firebase firebase locked and limited conversation to collaborators Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants