-
Notifications
You must be signed in to change notification settings - Fork 4k
feat(auth): move to Pigeon for Platform channels #10802
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
Conversation
...th/firebase_auth_platform_interface/lib/src/method_channel/method_channel_firebase_auth.dart
Show resolved
Hide resolved
...uth/firebase_auth_platform_interface/lib/src/platform_interface/platform_interface_user.dart
Show resolved
Hide resolved
...rebase_auth/firebase_auth_platform_interface/lib/src/method_channel/method_channel_user.dart
Show resolved
Hide resolved
@@ -195,7 +195,7 @@ with the Facebook App ID and Secret set. | |||
} | |||
``` | |||
|
|||
Note: Firebase will not set the `User.emailVerified` property | |||
Note: Firebase will not set the `User.isEmailVerified` property |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.emailVerified is deprecated but still available. It's changed to match Java and the isAnonymous
..._auth/android/src/main/java/io/flutter/plugins/firebase/auth/FlutterFirebaseMultiFactor.java
Outdated
Show resolved
Hide resolved
...auth/android/src/main/java/io/flutter/plugins/firebase/auth/IdTokenChannelStreamHandler.java
Show resolved
Hide resolved
packages/firebase_auth/firebase_auth/ios/Classes/FLTFirebaseAuthPlugin.m
Outdated
Show resolved
Hide resolved
…flutter/plugins/firebase/auth/FlutterFirebaseMultiFactor.java Co-authored-by: Russell Wheatley <[email protected]>
…thPlugin.m Co-authored-by: Russell Wheatley <[email protected]>
...th/android/src/main/java/io/flutter/plugins/firebase/auth/AuthStateChannelStreamHandler.java
Show resolved
Hide resolved
@@ -34,6 +34,16 @@ Future<void> main() async { | |||
await auth.useAuthEmulator('localhost', 9099); | |||
} | |||
|
|||
// Setup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this still be here or is it from testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm removing this
@"isAnonymous" : @NO, | ||
@"isEmailVerified" : @YES, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these 2 BOOLs be hard-coded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Provider Data, the email is always verified and the user is never anonymous. I'm adding a comment
@Deprecated('Use isEmailVerified instead') | ||
bool get emailVerified { | ||
return _delegate.emailVerified; | ||
return _delegate.isEmailVerified; | ||
} | ||
|
||
/// Returns whether the users email address has been verified. | ||
/// | ||
/// To send a verification email, see [sendEmailVerification]. | ||
/// | ||
/// Once verified, call [reload] to ensure the latest user information is | ||
/// retrieved from Firebase. | ||
bool get isEmailVerified { | ||
return _delegate.isEmailVerified; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we changing this API? 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To match the underlying API and the isAnonymous API (felt weird to have a is
api and anoter one without it even if both API are returning a bool, I can do it in another PR)
@@ -26,7 +26,7 @@ on: | |||
|
|||
jobs: | |||
android: | |||
runs-on: macos-13 | |||
runs-on: macos-13-xl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason to upgrade to large runners? Seems in all other places we are using standard runner.
Description
Moving Auth to Pigeon to prepare for Windows C++ port.
Related Issues
Replace this paragraph with a list of issues related to this PR from the issue database. Indicate, which of these issues are resolved or fixed by this PR. Note that you'll have to prefix the issue numbers with flutter/flutter#.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
).This will ensure a smooth and quick review process. Updating the
pubspec.yaml
and changelogs is not required.///
).melos run analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?