fix: update useFirestoreDocData return type to include undefined#733
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates type definitions across several files. It replaces JSX.Element with React.ReactElement in auth.tsx, performance.tsx, and storage.tsx, and adds an explicit React.ReactElement return type in sdk.tsx. Additionally, it updates the return types of useFirestoreDocData and useFirestoreDocDataOnce in firestore.tsx to allow undefined values (ObservableStatus<T | undefined>). There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
docData() from rxfire returns Observable<T | undefined> because a Firestore document may not exist. The previous ObservableStatus<T> return type was a lie — callers would get undefined at runtime with no type-level warning. Fixes FirebaseExtended#577
11e645d to
2bdb63a
Compare
Problem
useFirestoreDocDataanduseFirestoreDocDataOncewere declared to returnObservableStatus<T>, butdocData()from rxfire returnsObservable<T | undefined>because a Firestore document may not exist. This meant callers could accessdata.somePropertywithout any type-level warning, only to get a runtime error whendatawasundefined.Fix
Update the return types to
ObservableStatus<T | undefined>, which accurately reflects what the observable emits. The discriminated union inObservableStatusalready handles this correctly —status: 'loading'hasdata: undefined,status: 'success'hasdata: T | undefinedfor these hooks.TypeScript now correctly flags:
Fixes #577