refactor(auth)!: migrate to TypeScript and bring auth closer in alignment with firebase-js-sdk API#8991
refactor(auth)!: migrate to TypeScript and bring auth closer in alignment with firebase-js-sdk API#8991russellwheatley wants to merge 7 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the modular API for the Auth package and refactors the namespaced API with improved TypeScript support. Key updates include the integration of react-native-builder-bob for builds, a fix for a logic error in TOTP secret handling, and the addition of type comparison configurations. Feedback was provided to improve the clarity of an error message in the password validation logic and to refine the emulator URL parsing to handle missing ports more explicitly.
| export function validatePassword(auth: Auth, password: string): Promise<PasswordValidationStatus> { | ||
| if (!auth || !('app' in auth)) { | ||
| throw new Error( | ||
| "firebase.auth().validatePassword(*) 'auth' must be a valid Auth instance with an 'app' property. Received: undefined", |
There was a problem hiding this comment.
The error message hardcodes undefined for the received value, which is misleading if the auth parameter is null or an object that simply lacks the app property. It's better to dynamically include the actual value in the error message to aid debugging.
| "firebase.auth().validatePassword(*) 'auth' must be a valid Auth instance with an 'app' property. Received: undefined", | |
| `firebase.auth().validatePassword(*) 'auth' must be a valid Auth instance with an 'app' property. Received: ${auth}` |
| if (!host || !portString) { | ||
| throw new Error('firebase.auth().useEmulator() unable to parse host and port from URL'); | ||
| } |
There was a problem hiding this comment.
The current logic throws an error if the port is missing from the URL. While the Firebase Auth Emulator typically requires a port, the regex match for the port is optional. If the intention is to require a port, the regex should be updated, or the error message should explicitly state that the port is mandatory. If a default port is acceptable, it should be handled here.
1 participant
Description
Related issues
Release Summary
Checklist
AndroidiOSOther(macOS, web)e2etests added or updated inpackages/\*\*/e2ejesttests added or updated inpackages/\*\*/__tests__Test Plan
Think
react-native-firebaseis great? Please consider supporting the project with any of the below:React Native FirebaseandInvertaseon Twitter