Skip to content
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

feat(firestore): data bundles API #6199

Merged
merged 40 commits into from
Apr 27, 2022
Merged

Conversation

kmsbernard
Copy link
Contributor

@kmsbernard kmsbernard commented Apr 14, 2022

Description

Add Firestore bundles API: loadBundle() and namedQuery()

Related issues

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
  • 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:

@vercel
Copy link

vercel bot commented Apr 14, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

react-native-firebase – ./

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/9AyXnLYQ6CKHZ4DSLMuZpQvJVXqF
✅ Preview: https://react-native-firebase-git-fork-bernard-kms-fir-1af0bd-invertase.vercel.app

react-native-firebase-next – ./website_modular

🔍 Inspect: https://vercel.com/invertase/react-native-firebase-next/CRJYE2X3tGbgsKpHjaxZmNnX6CTp
✅ Preview: Ignored

[Deployment for 00d0009 canceled]

@vercel vercel bot temporarily deployed to Preview – react-native-firebase-next April 14, 2022 15:09 Inactive
@kmsbernard kmsbernard changed the title feat(firestone): data bundles API feat(firestore): data bundles API Apr 14, 2022
@codecov
Copy link

codecov bot commented Apr 14, 2022

Codecov Report

Merging #6199 (9bfb0b2) into main (ef7895c) will decrease coverage by 18.64%.
The diff coverage is 84.56%.

❗ Current head 9bfb0b2 differs from pull request most recent head bdb2a80. Consider uploading reports for the commit bdb2a80 to get more accurate results

@@              Coverage Diff              @@
##               main    #6199       +/-   ##
=============================================
- Coverage     72.11%   53.48%   -18.63%     
- Complexity        0      649      +649     
=============================================
  Files           109      208       +99     
  Lines          4618    10302     +5684     
  Branches       1040     1633      +593     
=============================================
+ Hits           3330     5509     +2179     
- Misses         1210     4539     +3329     
- Partials         78      254      +176     

@mikehardy mikehardy added the Workflow: Needs Review Pending feedback or review from a maintainer. label Apr 14, 2022
@mikehardy
Copy link
Collaborator

I think the android test failure in e2e ci was a flaky test, I restarted it
the documentation one is really easy to fix: just add CDN to spellcheck.dict in root and that one will go green

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.

This is fantastic! I pulled it locally and stabilized it across repeated runs via test:<platform>:test-reuse where you need to call firestore().clearPersistence() on startup so that there are no cached non-bundle docs before the run, but otherwise it just worked.

This was one of the flagship features added to firebase in 2021 and it really nagged me that we didn't have it yet, I truly appreciate your posting this! Hopefully lots of people put it to use and enjoy some cost savings

@mikehardy mikehardy added Workflow: Pending Merge Waiting on CI or similar and removed Workflow: Needs Review Pending feedback or review from a maintainer. labels Apr 26, 2022
@mikehardy mikehardy merged commit 96591e0 into invertase:main Apr 27, 2022
@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Apr 27, 2022
@kmsbernard
Copy link
Contributor Author

Wow, thanks for a quick merge & release!

@mikehardy
Copy link
Collaborator

Ha! I actually felt like 7 days after you were done before merge was patient on your part! There is no real bureaucracy here so things can move really quickly, I was pretty behind, from work on other repos 😅

@kmsbernard
Copy link
Contributor Author

I think my last week went by like crazy that I felt it short. 😂 And don't have to be sorry about the "behind", I always appreciate your tremendous work in maintaining the library!

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