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: Add string type to DocumentSnapshot.get #7593

Merged
merged 1 commit into from
Jan 30, 2024
Merged

fix: Add string type to DocumentSnapshot.get #7593

merged 1 commit into from
Jan 30, 2024

Conversation

ChromeQ
Copy link
Contributor

@ChromeQ ChromeQ commented Jan 27, 2024

Description

The comment above this change even gives an example of a dot notation string.

Taking string type from @google-cloud/firestore types: https://github.com/googleapis/nodejs-firestore/blob/22ac90f0a043e493f89834604bd249f69c5dc036/types/firestore.d.ts#L1587

Also the same on firebase-js-sdk:
https://github.com/firebase/firebase-js-sdk/blob/991fa271c867d59e2bed44c69c0512fdeb54bbb4/packages/firestore/src/lite-api/snapshot.ts#L359

Related issues

Fixes #7592

Release Summary

Fix the typing for DocumentSnapshot.get to allow dot notation object values

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:

Copy link

vercel bot commented Jan 27, 2024

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

Name Status Preview Comments Updated (UTC)
react-native-firebase ❌ Failed (Inspect) Jan 27, 2024 3:14am

Copy link

codecov bot commented Jan 27, 2024

Codecov Report

Merging #7593 (afe3b94) into main (970756d) will decrease coverage by 14.88%.
The diff coverage is n/a.

Additional details and impacted files
@@              Coverage Diff              @@
##               main    #7593       +/-   ##
=============================================
- Coverage     68.20%   53.31%   -14.88%     
- Complexity        0      742      +742     
=============================================
  Files           149      251      +102     
  Lines          5933    12420     +6487     
  Branches       1248     1938      +690     
=============================================
+ Hits           4046     6621     +2575     
- Misses         1790     5449     +3659     
- Partials         97      350      +253     

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.

Looks good - thank you again 🙌

@mikehardy mikehardy merged commit d5b66ca into invertase:main Jan 30, 2024
14 of 16 checks passed
@ChromeQ ChromeQ deleted the patch-1 branch February 12, 2024 14:17
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.

[🐛] 🔥Firestore document get string type is incorrect when using interface
2 participants