Skip to content

fix(firestore): type definitions #8378

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

SelaseKay
Copy link
Collaborator

Release Summary

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
    • Other (macOS, web)
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan


Think react-native-firebase is great? Please consider supporting the project with any of the below:

Copy link

vercel bot commented Feb 28, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-firebase ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 29, 2025 9:46am

@@ -701,7 +722,7 @@ export function loadBundle(
* @param name - The name of the query.
* @returns A named Query.
*/
export function namedQuery(firestore: Firestore, name: string): Query<DocumentData>;
export function namedQuery(firestore: Firestore, name: string): Promise<Query | null>;
Copy link
Member

Choose a reason for hiding this comment

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

Just double checking, @mikehardy, in the firebase-js-sdk documentation, it returns a Promise<Query | null>. But we currently return a Query. Should we match the firebase-js-sdk in this instance? (I assume yes!).

Copy link

Hello 👋, this PR has been opened for more than 2 months with no activity on it.

If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing!

You have 15 days until this gets closed automatically

@github-actions github-actions bot added the Stale label Mar 28, 2025
@github-actions github-actions bot closed this Apr 12, 2025
@mikehardy mikehardy reopened this Apr 24, 2025
Copy link

codecov bot commented Apr 24, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 69.79%. Comparing base (8f59579) to head (796493f).
Report is 52 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8378      +/-   ##
==========================================
- Coverage   71.78%   69.79%   -1.99%     
==========================================
  Files         163      187      +24     
  Lines        6626     8039    +1413     
  Branches     1524     1840     +316     
==========================================
+ Hits         4756     5610     +854     
- Misses       1749     2238     +489     
- Partials      121      191      +70     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions bot removed the Stale label Apr 24, 2025
@MichaelVerdon
Copy link
Collaborator

Kept definition for onSnapshotsInSync() which will merge with other PR that actually implements it.

@MichaelVerdon MichaelVerdon marked this pull request as ready for review April 29, 2025 09:35
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.

4 participants