nodejs-firestore icon indicating copy to clipboard operation
nodejs-firestore copied to clipboard

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

Open CaptainCodeman opened this issue 1 year ago • 0 comments

The toFirestore method of converter is called multiple times when adding a new document to a collection which can cause issues if the object is mutated as part of the conversion (it seems reasonable to assume it should only be called once).

Similar to this client-side issue: https://github.com/firebase/firebase-js-sdk/issues/3742

Environment details

  • OS: macOS 14.6.1
  • Node.js version: 22.4.0
  • npm version: pnpm 9.7.1
  • @google-cloud/firestore version: 7.9.0

Steps to reproduce

Use the .add method on a collection ref with a converter, mutating the object:

const testConverter = {
  toFirestore(test) {
    test.hash = Blob.fromBase64String(test.hash)
    return test
  },
  fromFirestore(snapshot, options) {
    const data = snapshot.data(options)!;
    data.hash = data.hash.toBase64()
    return data
  }
};

const ref = await firestore
		.collection(`test`)
		.withConverter(testConverter)
		.add({ name: 'test', hash: 'some-base64-string-that-we-want-to-store-as-a-blob' })

The converter would work fine for regular set, and update methods, but .add causes .toFirestore to be called twice. Although it's a good idea for the converters to use immutable patterns which would avoid this, it's an easy mistake to just mutate the existing object which can cause runtime errors or data corruption and if nothing else is wasted extra work.

CaptainCodeman avatar Aug 19 '24 19:08 CaptainCodeman