fix(firestore): restore generic root type exports for DocumentSnapshot, Transaction, and WriteBatch#8997
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Code Review
This pull request adds a comprehensive suite of consumer-facing API type tests for Firestore, covering namespaced, modular, and the new pipelines API. It includes a new test file, a dedicated TypeScript configuration, and a package script for type checking. Feedback was provided regarding the testConverter implementation, which should handle both full and partial object types in toFirestore to properly support merge and update operations.
mikehardy
left a comment
There was a problem hiding this comment.
seems reasonable - I like that the consumer stuff now isn't per-package and the glob for "consumer-type-test" means this should be easy to use as a pattern for other packages - ideally all of the type tests are "consumer" style ones with rare / maybe not existing non-consumer ones
| }); | ||
| }); | ||
| console.log(queryEqual(modQuery1, modQuery2)); | ||
| console.log(queryEqual(modQuery1, modQuery2)); |
There was a problem hiding this comment.
interesting that passes lint with trailing white space
DocumentSnapshot, Transaction, and WriteBatch
Description
Fix a Firestore TypeScript regression where root imports like
DocumentSnapshot,Transaction, andWriteBatchresolved to non-generic runtime class declarations instead of the intended public generic types.This updates the Firestore root export surface and emitted declarations so root-imported Firestore types behave correctly in type positions again, and adds regression coverage for the reported usage patterns.
Why compare-types did not catch it
compare-typescurrently compares Firestore modular/type files such astypes/firestore.d.tsandmodular.d.ts, not the package rootindex.d.tsconsumer surface.Because the generic types already existed in the modular/type declarations, the script saw the Firestore shapes as matching. The regression was in how the root package entrypoint re-exported those symbols for consumers importing from
@react-native-firebase/firestore, which sits outside the current compare-types boundary.Introduces Consumer Type Tests
Related issues
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