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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃敟 [firestore] update with FieldPath doesn't deep merge #2532

Closed
5 of 10 tasks
iwikal opened this issue Aug 30, 2019 · 4 comments
Closed
5 of 10 tasks

馃敟 [firestore] update with FieldPath doesn't deep merge #2532

iwikal opened this issue Aug 30, 2019 · 4 comments
Assignees

Comments

@iwikal
Copy link
Contributor

iwikal commented Aug 30, 2019

Issue

When specifying a field path with a firestore.FieldPath instead of a string, both WriteBatch.update and DocumentReference.update fails to deep merge. For instance, given this document:

firestore().doc('test/test').set({
  myMap: {
    initialKey: 'initialValue',
  },
  separateField: 'x',
})

...the following code...

firestore().doc('test/test')
  .update('myMap.newKey', 'newValue')

...does exactly what I expect:

{
  "myMap": {
    "initialKey": "initialValue",
    "newKey": "newValue"
  },
  "separateField": "x"
}

But this code does something unexpected!

firestore()
  .doc('test/test')
  .update(new firestore.FieldPath('myMap', 'newKey'), 'newValue')

Result:

{
  "myMap": {
    "newKey": "newValue"
  },
  "separateField": "x"
}

Note that the inital value in the map is gone, but the separate field remains. I tried the same thing with the firebase-admin sdk, and got the expected behaviour.


Project Files

iOS

Click To Expand

ios/Podfile:

  • I'm not using Pods
  • I'm using Pods and my Podfile looks like:
# N/A

AppDelegate.m:

// N/A


Android

Click To Expand

Have you converted to AndroidX?

  • my application is an AndroidX application?
  • I am using android/gradle.settings jetifier=true for Android compatibility?
  • I am using the NPM package jetifier for react-native compatibility?

android/build.gradle:

// N/A

android/app/build.gradle:

// N/A

android/settings.gradle:

// N/A

MainApplication.java:

// N/A

AndroidManifest.xml:

<!-- N/A -->


Environment

Click To Expand

react-native info output:

info Fetching system and libraries information...
System:
    OS: Linux 4.15 Ubuntu 18.04.3 LTS (Bionic Beaver)
    CPU: (4) x64 Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz
    Memory: 2.61 GB / 15.55 GB
    Shell: 5.4.2 - /usr/bin/zsh
  Binaries:
    Node: 10.16.2 - ~/.nvm/versions/node/v10.16.2/bin/node
    Yarn: 1.17.3 - ~/.yarn/bin/yarn
    npm: 6.10.3 - ~/.nvm/versions/node/v10.16.2/bin/npm
  npmPackages:
    @react-native-community/cli: ^2.8.0 => 2.8.0 
    react: 16.8.6 => 16.8.6 
    react-native: 0.60.4 => 0.60.4

  • Platform that you're experiencing the issue on:
    • iOS
    • Android
    • iOS but have not tested behavior on Android
    • Android but have not tested behavior on iOS
    • Both
  • react-native-firebase version you're using that has this issue:
    • 5.5.6
  • Firebase module(s) you're using that has the issue:
    • Firestore
  • Are you using TypeScript?
    • Y


Think react-native-firebase is great? Please consider supporting all of the project maintainers and contributors by donating via our Open Collective where all contributors can submit expenses. [Learn More]

@iwikal iwikal changed the title 馃敟 [firestore] update with FieldPath doesn't deep merge :fire [firestore] update with FieldPath doesn't deep merge Aug 30, 2019
@iwikal iwikal changed the title :fire [firestore] update with FieldPath doesn't deep merge 馃敟 [firestore] update with FieldPath doesn't deep merge Aug 30, 2019
@iwikal iwikal changed the title 馃敟 [firestore] update with FieldPath doesn't deep merge 馃敟 [firestore] update with FieldPath doesn't deep merge Aug 30, 2019
@Ehesp
Copy link
Member

Ehesp commented Aug 30, 2019

Interesting - thanks for the detailed report. I'll get this looked into.

@stale
Copy link

stale bot commented Sep 27, 2019

Hello 馃憢, to help manage issues we automatically close stale issues.
This issue has been automatically marked as stale because it has not had activity for quite some time. Has this issue been fixed, or does it still require the community's attention?

This issue will be closed in 15 days if no further activity occurs.
Thank you for your contributions.

@stale stale bot added the Type: Stale Issue has become stale - automatically added by Stale bot label Sep 27, 2019
@Ehesp Ehesp self-assigned this Oct 3, 2019
@stale stale bot removed the Type: Stale Issue has become stale - automatically added by Stale bot label Oct 3, 2019
@Ehesp Ehesp closed this as completed in 0810cfc Oct 3, 2019
@Ehesp
Copy link
Member

Ehesp commented Oct 3, 2019

Thanks for reporting. Turns out the fix was simple one! For some reason it was overcomplicating FieldPaths, trying to merge objects together when it can just pass the dot notated paths to native land.

@Salakar
Copy link
Member

Salakar commented Oct 7, 2019

Hey 馃憢 this issue should now be fixed in v6.0.1, thanks. [release notes]

androidIsForVivek pushed a commit to androidIsForVivek/react-native-firebase that referenced this issue Aug 9, 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

No branches or pull requests

3 participants