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

fix(database, update): allow empty objects in ref.update() #5220

Merged
merged 1 commit into from
Apr 28, 2021

Conversation

mikehardy
Copy link
Collaborator

Description

As pointed out by @daveGregorian firebase-js-sdk allows empty objects
while the code here was enforcing that the objects had at least one key

Related issues

Fixes #5218

Release Summary

Conventional commit message

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

Verified (with investigation in linked issue) that firebase-js-sdk allows empty objects through), added an e2e test that verifies firebase-ios-sdk and firebase-android-sdk handle empty objects, and those work, so removing the check in our javascript layer should be fine


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

As pointed out by @daveGregorian firebase-js-sdk allows empty objects
while the code here was enforcing that the objects had at least one key

Fixes #5218
@vercel
Copy link

vercel bot commented Apr 27, 2021

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

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/D6ojSKerixG6kKDJdwbV47PSoErx
✅ Preview: https://react-native-f-git-mikehardy-database-update-empty-allow-73626d.vercel.app

@mikehardy mikehardy added the Workflow: Pending Merge Waiting on CI or similar label Apr 27, 2021
@codecov
Copy link

codecov bot commented Apr 27, 2021

Codecov Report

Merging #5220 (f9b81f9) into master (589fcb0) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head f9b81f9 differs from pull request most recent head 269a3a8. Consider uploading reports for the commit 269a3a8 to get more accurate results

@@            Coverage Diff             @@
##           master    #5220      +/-   ##
==========================================
- Coverage   88.87%   88.86%   -0.00%     
==========================================
  Files         109      109              
  Lines        3744     3742       -2     
  Branches      360      360              
==========================================
- Hits         3327     3325       -2     
  Misses        370      370              
  Partials       47       47              

@mikehardy mikehardy merged commit 574f691 into master Apr 28, 2021
@mikehardy mikehardy deleted the @mikehardy/database-update-empty-allowed branch April 28, 2021 05:44
@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Apr 28, 2021
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.

🔥[🐛] Wrong empty check in the main DatabaseReference.update call
1 participant