Ignore undefined values rather than throw exceptions

[nojvek]

Thanks for stopping by to let us know something could be better!

PLEASE READ: If you have a support contract with Google, please create an issue in the support console instead of filing on GitHub. This will ensure a timely response.

Please run down the following list and make sure you've tried the usual "quick fixes":

• Search the issues already opened: https://github.com/googleapis/nodejs-firestore/issues
• Search the issues on our "catch-all" repository: https://github.com/googleapis/google-cloud-node
• Search StackOverflow: http://stackoverflow.com/questions/tagged/google-cloud-platform+node.js

If you are still having issues, please be sure to include as much information as possible:

Environment details

• OS: Mac
• Node.js version: 12
• npm version: 6.13.6
• @google-cloud/firestore version: 3.7.4"

Steps to reproduce

1. Insert using fstore.collection(blah).doc(foo).set({"hello": undefined})

Expected

Like JSON.stringify, undefined keys are ignored.

Actual

Error: Value for argument "data" is not a valid Firestore document. Cannot use "undefined" as a Firestore value (found in field "hello").

Explanations

undefines are a routine part of js. Not gracefully handling undefines means firestore becomes a brittle library that barfs every now and then. The developer experience could be much better.

A quick dirty workaround is doc.set(JSON.parse(JSON.stringify(val)), which is yuck. The library can handle this elegantly and preserve JSON.stringify like behaviour.

[schmidt-sebastian]

@nojvek Thanks for this request! This has come up a couple of times. I will talk with the team to see if this is a behavior we can support.

[nojvek]

If you're open to a PR, I can submit one. I did a log of debugging yest to see where this was failing.

[schmidt-sebastian]

This is more of a political challenge rather than a technical one, as this might also lead to unexpected behavior. Stripping undefined breaks the assumption that data the read from Firestore is the same as the data written to, and as such I am expecting some pushback. If we do decide to change the behavior, then I would be more than happy to accept a PR.

[merlinnot]

That's definitely a two edged sword - I personally like the current behavior and I saw a couple of instances where it saved people from writing invalid data to the DB. We could also expose it as an option, so that people can decide for themselves.

[nojvek]

Yeah mongodb exposes an ignoreUndefined option as part of its connection settings and I’ve been a big fan of that. https://mongodb.github.io/node-mongodb-native/2.2/reference/connecting/connection-settings/ I can understand this being a polarized discussion by making it default

[schmidt-sebastian]

We will add an option to the SDK to turn on the requested behavior.

[schmidt-sebastian]

@nojvek If you have time, can you take a look at #1062 and let us know if this addresses your needs?

[elmasry]

As of today May 29, 2020, Firebase Added support for calling FirebaseFiresore.settings with { ignoreUndefinedProperties: true }. When this parameter is set, Cloud Firestore ignores undefined properties inside objects rather than rejecting the API call.

[samuelhulla]

I'm posting this is a work-around that if you're for whatever reason unable to update your legacy firebase version and can't access the ignoreUndefinedProperties argument.

Here's a similar custom version set that does the same thing, that actually exposes more functionality allowing you to decide if you want to check also nested fields for undefined, or only the root level of the passed data.

export const ignoreUndefinedSet = async (
  // depending on your namespace & /types version, you can use firebase.firestore.<name> instead
  reference: FirebaseFirestore.DocumentReference,
  data: FirebaseFirestore.DocumentData,
  options?: FirebaseFirestore.SetOptions,
  checkNestedObjects = true,
) => {
  const isPlainObject = (val: unknown) =>
     typeof val === 'object' && val !== null && !(val instanceof Date) && !Array.isArray(val)
  const keepDefinedProperties = (
     obj: FirebaseFirestore.DocumentData,
     nestedCheck = true,
  ) =>
   Object.entries(data).reduce(
      (result, [key, value]) => (
           value === undefined
                ? result
                : (nestedCheck && isPlainObject(value))
                   ? Object.assign(result, { [key]: keepDefinedProperties(value) })
                   : Object.assign(result, { [key]: value })
       ),
      {}
  )
  const onlyDefinedProperties = keepDefinedProperties(data, checkNestedObjects)
  await reference.set(onlyDefinedProperties, { ...options })
}

So essentially these two statements are equivalent

await reference.set(data, { ignoreUndefinedProperties: true }) // newer firebase version

await ignoreUndefinedSet(reference, data) // my polyfill

[lopugit]

As of today May 29, 2020, Firebase Added support for calling FirebaseFiresore.settings with { ignoreUndefinedProperties: true }. When this parameter is set, Cloud Firestore ignores undefined properties inside objects rather than rejecting the API call.

YAY

[lopugit]

wait ... this only works for root level properties? what in the ........... google u mad

[schmidt-sebastian]

This should work for nested undefined values as well.

[odbol]

For anyone using the v9 API:

import { getFirestore, connectFirestoreEmulator, initializeFirestore } from 'firebase/firestore'; // Firebase v9+

  // Must be called before getFirestore()
  initializeFirestore(app, {
    ignoreUndefinedProperties: true
  });
  const firestore = getFirestore(app);

[thorizer]

initializeFirestore(app, {
ignoreUndefinedProperties: true
});

Uncaught FirebaseError: initializeFirestore() has already been called with different options. To avoid this error, call initializeFirestore() with the same options as when it was originally called, or call getFirestore() to return the already initialized instance.

[odbol]

initializeFirestore(app, {
ignoreUndefinedProperties: true
});

Uncaught FirebaseError: initializeFirestore() has already been called with different options. To avoid this error, call initializeFirestore() with the same options as when it was originally called, or call getFirestore() to return the already initialized instance.

You can get around the issue where it's already initialized by storing the admin instance in a singleton and just reusing that: https://stackoverflow.com/a/70468753/473201