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

Merged
merged 8 commits into from
Jul 1, 2025
Merged

fix(firestore): type definitions #8378

merged 8 commits into from
Jul 1, 2025

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:

Siteproxy
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 Jul 1, 2025 0:50am

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.

@MichaelVerdon
Copy link
Collaborator

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

This comment was marked as outdated.

Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

These look ready to go in, with exception of a question about the empty VectorValue.d.ts file - should that exist, or should it be deleted before mergin?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is there an empty file in here? 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

I removed it, I think that was the right call?

@mikehardy mikehardy added Workflow: Waiting for User Response Blocked waiting for user response. Workflow: Pending Merge Waiting on CI or similar labels Jun 30, 2025
SelaseKay and others added 8 commits June 30, 2025 19:46

Verified

This commit was signed with the committer’s verified signature.
mikehardy Mike Hardy

Verified

This commit was signed with the committer’s verified signature.
mikehardy Mike Hardy

Verified

This commit was signed with the committer’s verified signature.
mikehardy Mike Hardy

Verified

This commit was signed with the committer’s verified signature.
mikehardy Mike Hardy

Verified

This commit was signed with the committer’s verified signature.
mikehardy Mike Hardy

Verified

This commit was signed with the committer’s verified signature.
mikehardy Mike Hardy

Verified

This commit was signed with the committer’s verified signature.
mikehardy Mike Hardy

Verified

This commit was signed with the committer’s verified signature.
mikehardy Mike Hardy
@mikehardy mikehardy force-pushed the firestore_type_def_check branch from ee9af2b to 13411a2 Compare July 1, 2025 00:48
@mikehardy
Copy link
Collaborator

@russellwheatley / @MichaelVerdon I rebased this to main and pushed it out, minus the empty modular/VectorValue.d.ts file - I think this is ready to go though ? +1 from me for merge anyway. Any last thoughts from you two no this one?

@mikehardy mikehardy added Workflow: Needs Second Review Waiting on a second review before merge and removed Workflow: Waiting for User Response Blocked waiting for user response. labels Jul 1, 2025
@mikehardy mikehardy merged commit 497c6d1 into main Jul 1, 2025
18 checks passed
@mikehardy mikehardy deleted the firestore_type_def_check branch July 1, 2025 12:33
@mikehardy mikehardy removed Workflow: Pending Merge Waiting on CI or similar Workflow: Needs Second Review Waiting on a second review before merge labels Jul 1, 2025
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.

None yet

4 participants