Skip to content

Refactor Firebase app initialization to fail fast and remove nullable fields#90

Merged
Lyokone merged 1 commit intomainfrom
less_null
Mar 30, 2026
Merged

Refactor Firebase app initialization to fail fast and remove nullable fields#90
Lyokone merged 1 commit intomainfrom
less_null

Conversation

@kevmoo
Copy link
Copy Markdown
Collaborator

@kevmoo kevmoo commented Mar 27, 2026

  • Replaced nullable and accessors with properties.
  • Removed the try...catch wrapper around FirebaseApp.initializeApp() to fail hard on initialization errors rather than suppressing them.
  • Removed the early emulator exit check; the SDK now proceeds with resolving default credentials even under emulator conditions.
  • Refactored token extractors (extractAuthToken, extractAppCheckToken) to allow a nullable auth or appCheck instead replacing the the skip bit.
  • Updated https_auth_test.dart to be a lot more simple.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the Firebase class to use a factory constructor and makes the adminApp and firestoreAdmin properties non-nullable and required. This change necessitates updates to the authentication and App Check token extraction functions, which now require a FirebaseApp instance as a parameter. Feedback suggests improving the unit tests by using a mock FirebaseApp instead of initializing a real Firebase instance to ensure better test isolation and avoid potential side effects from the Admin SDK.

@kevmoo kevmoo force-pushed the cleanup branch 2 times, most recently from 7d546c8 to c98e127 Compare March 27, 2026 03:20
Base automatically changed from cleanup to main March 27, 2026 10:36
- Replace `skipTokenVerification` flag in token checking with `adminApp` presence check
- Use `firebase.$env` for accessing environment details
- Consolidate token extraction tests to reflect simplified API
- Resolve trace context testing issues
@Lyokone Lyokone merged commit 76ce0f0 into main Mar 30, 2026
8 checks passed
@Lyokone Lyokone deleted the less_null branch March 30, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants