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

Firestore: CollectionReference.withConverter(converter).add(data) invokes converter.toFirestore twice #3742

Closed
hecateball opened this issue Sep 4, 2020 · 1 comment · Fixed by #3755
Assignees

Comments

@hecateball
Copy link

hecateball commented Sep 4, 2020

[REQUIRED] Describe your environment

  • Operating System version: macOS Catalina 10.15.6
  • Browser version: N/A
  • Firebase SDK version: 7.19.1
  • Firebase Product: firestore

[REQUIRED] Describe the problem

firebase.firestore().collection('...').withConverter(converter).add(data) invokes converter.toFirestore twice while firebase.firestore().collection('...').withConverter(converter).doc().set(data) invokes converter.toFirestore only once.
Is this intended behavior?

Although this is not a desirable style of processing, if the toFirestore() process has a side effect on the input value, the second invocation fails (See example below).

Steps to reproduce:

try to run following code please.

Relevant Code:

const firebase = require('firebase')

// Please use your firebase config here
firebase.initializeApp(firebaseConfig)

const document = {
  someArray: [
    { text: '' },
  ],
}

const converter = {
  toFirestore: (modelObject) => {
    console.log('toFirestore', modelObject)
    modelObject.someArray = 'text'
    return { text: modelObject.someArray[0].text }
  },
  fromFirestore: () => {
    throw new ReferenceError()
  },
}

// log 'toFirestore' twice, and then fail second conversion.
firebase.firestore().collection('posts')
  .withConverter(converter)
  .add(document)

// log 'toFirestore' only once and complete as expected.
firebase.firestore().collection('posts')
  .withConverter(converter)
  .doc()
  .set(document)
@thebrianchen
Copy link

thebrianchen commented Sep 10, 2020

@hecateball Thanks for filing the bug with an easy repro! We should have a fix for this out in the coming weeks.

@firebase firebase locked and limited conversation to collaborators Oct 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants