Merge

Author: schmidt-sebastian

package.json

@@ -22,8 +22,10 @@
   ],
   "scripts": {
     "dev": "lerna run --parallel --scope @firebase/* --scope firebase --scope rxfire dev",
-    "build": "lerna run --scope @firebase/* --scope firebase --scope rxfire prepare",
+    "build": "lerna run --scope @firebase/* --scope firebase --scope rxfire build",
     "build:exp": "lerna run --scope @firebase/*-exp --scope firebase-exp build",
+    "build:release": "lerna run --scope @firebase/* --scope firebase --ignore @firebase/*-exp --ignore firebase-exp prepare",
+    "build:exp:release": "lerna run --scope @firebase/*-exp --scope firebase-exp prepare && yarn --cwd packages-exp/app-exp typings:public",
     "link:packages": "lerna exec --scope @firebase/* --scope firebase --scope rxfire -- yarn link",
     "stage:packages": "./scripts/prepublish.sh",
     "repl": "node tools/repl.js",

packages-exp/app-exp/package.json

@@ -15,19 +15,21 @@
     "lint": "eslint -c .eslintrc.js '**/*.ts' --ignore-path '../../.gitignore'",
     "lint:fix": "eslint --fix -c .eslintrc.js '**/*.ts' --ignore-path '../../.gitignore'",
     "build": "rollup -c && yarn api-report",
-    "build:release": "rollup -c rollup.config.release.js && node ./use_public_typings.js",
+    "build:release": "rollup -c rollup.config.release.js && yarn api-report && yarn typings:public",
     "build:deps": "lerna run --scope @firebase/app-exp --include-dependencies build",
     "dev": "rollup -c -w",
     "test": "yarn type-check && run-p lint test:browser test:node",
     "test:ci": "node ../../scripts/run_tests_in_ci.js",
     "test:browser": "karma start --single-run",
     "test:node": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' nyc --reporter lcovonly -- mocha src/**/*.test.ts --config ../../config/mocharc.node.js",
     "type-check": "tsc -p . --noEmit",
-    "prepare": "yarn build:release",
+    "prepare": "rollup -c rollup.config.release.js",
     "api-report": "api-extractor run --local --verbose",
     "predoc": "node ../../scripts/exp/remove-exp.js temp",
     "doc": "api-documenter markdown --input temp --output docs",
-    "build:doc": "yarn build && yarn doc"
+    "build:doc": "yarn build && yarn doc",
+    "typings:public": "node ./use_public_typings.js --public",
+    "typings:internal": "node ./use_public_typings.js"
   },
   "dependencies": {
     "@firebase/app-types-exp": "0.0.800",
@@ -52,7 +54,7 @@
   "bugs": {
     "url": "https://github.com/firebase/firebase-js-sdk/issues"
   },
-  "typings": "./dist/app-exp-public.d.ts",
+  "typings": "./dist/app-exp.d.ts",
   "nyc": {
     "extension": [
       ".ts"

packages-exp/app-exp/use_public_typings.js renamed to packages-exp/app-exp/use_typings.js

@@ -1,3 +1,4 @@
+/* eslint-disable @typescript-eslint/no-require-imports */
 /**
  * @license
  * Copyright 2020 Google LLC
@@ -16,16 +17,23 @@
  */
 
 const { writeFileSync } = require('fs');
+const { argv } = require('yargs');
+
+const path = require('path');
+const packageJsonPath = path.resolve(__dirname, './package.json');
 
 // point typings field to the public d.ts file in package.json
-const PUBLIC_TYPINGS_PATH = './dist/app-exp-public.d.ts';
+const TYPINGS_PATH = argv.public
+  ? './dist/app-exp-public.d.ts'
+  : './dist/app-exp.d.ts';
 console.log(
-  `Updating the typings field to the public d.ts file ${PUBLIC_TYPINGS_PATH}`
+  `Updating the packages-exp/app-exp typings field to the ${
+    argv.public ? 'public' : 'internal'
+  } d.ts file ${TYPINGS_PATH}`
 );
+const packageJson = require(packageJsonPath);
+packageJson.typings = TYPINGS_PATH;
 
-const packageJson = require('./package.json');
-packageJson.typings = PUBLIC_TYPINGS_PATH;
-
-writeFileSync('./package.json', `${JSON.stringify(packageJson, null, 2)}\n`, {
+writeFileSync(packageJsonPath, `${JSON.stringify(packageJson, null, 2)}\n`, {
   encoding: 'utf-8'
 });

packages/firebase/index.d.ts

@@ -6920,15 +6920,15 @@ declare namespace firebase.database.ServerValue {
    * ```
    */
   var TIMESTAMP: Object;
-  
+
   /**
-   * Returns a placeholder value that can be used to atomically increment the 
+   * Returns a placeholder value that can be used to atomically increment the
    * current database value by the provided delta.
    *
    * @param delta the amount to modify the current value atomically.
    * @return a placeholder value for modifying data atomically server-side.
    */
-  function increment(delta: number) : Object;
+  function increment(delta: number): Object;
 }
 
 /**
@@ -7743,6 +7743,14 @@ declare namespace firebase.firestore {
      * @webonly
      */
     experimentalForceLongPolling?: boolean;
+
+    /**
+     * Whether to skip nested properties that are set to `undefined` during
+     * object serialization. If set to `true`, these properties are skipped
+     * and not written to Firestore. If set `false` or omitted, the SDK throws
+     * an exception when it encounters properties of type `undefined`.
+     */
+    ignoreUndefinedProperties?: boolean;
   }
 
   /**

packages/firestore-types/index.d.ts

@@ -29,6 +29,7 @@ export interface Settings {
   timestampsInSnapshots?: boolean;
   cacheSizeBytes?: number;
   experimentalForceLongPolling?: boolean;
+  ignoreUndefinedProperties?: boolean;
 }
 
 export interface PersistenceSettings {

packages/firestore/CHANGELOG.md

@@ -1,4 +1,9 @@
 # Unreleased
+- [feature] Added support for calling  `FirebaseFiresore.settings` with 
+  `{ ignoreUndefinedProperties: true }`. When set, Firestore ignores 
+  undefined properties inside objects rather than rejecting the API call.
+
+# Released
 - [fixed] Fixed a regression introduced in v7.14.2 that incorrectly applied
   a `FieldValue.increment` in combination with `set({...}, {merge: true})`.
 - [fixed] Firestore now rejects `onSnapshot()` listeners if they cannot be

packages/firestore/src/api/database.ts

@@ -98,6 +98,7 @@ const DEFAULT_HOST = 'firestore.googleapis.com';
 const DEFAULT_SSL = true;
 const DEFAULT_TIMESTAMPS_IN_SNAPSHOTS = true;
 const DEFAULT_FORCE_LONG_POLLING = false;
+const DEFAULT_IGNORE_UNDEFINED_PROPERTIES = false;
 
 /**
  * Constant used to indicate the LRU garbage collection should be disabled.
@@ -142,6 +143,8 @@ class FirestoreSettings {
 
   readonly forceLongPolling: boolean;
 
+  readonly ignoreUndefinedProperties: boolean;
+
   // Can be a google-auth-library or gapi client.
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   credentials?: any;
@@ -169,7 +172,8 @@ class FirestoreSettings {
       'credentials',
       'timestampsInSnapshots',
       'cacheSizeBytes',
-      'experimentalForceLongPolling'
+      'experimentalForceLongPolling',
+      'ignoreUndefinedProperties'
     ]);
 
     validateNamedOptionalType(
@@ -187,6 +191,13 @@ class FirestoreSettings {
       settings.timestampsInSnapshots
     );
 
+    validateNamedOptionalType(
+      'settings',
+      'boolean',
+      'ignoreUndefinedProperties',
+      settings.ignoreUndefinedProperties
+    );
+
     // Nobody should set timestampsInSnapshots anymore, but the error depends on
     // whether they set it to true or false...
     if (settings.timestampsInSnapshots === true) {
@@ -202,6 +213,8 @@ class FirestoreSettings {
     }
     this.timestampsInSnapshots =
       settings.timestampsInSnapshots ?? DEFAULT_TIMESTAMPS_IN_SNAPSHOTS;
+    this.ignoreUndefinedProperties =
+      settings.ignoreUndefinedProperties ?? DEFAULT_IGNORE_UNDEFINED_PROPERTIES;
 
     validateNamedOptionalType(
       'settings',
@@ -232,9 +245,7 @@ class FirestoreSettings {
       settings.experimentalForceLongPolling
     );
     this.forceLongPolling =
-      settings.experimentalForceLongPolling === undefined
-        ? DEFAULT_FORCE_LONG_POLLING
-        : settings.experimentalForceLongPolling;
+      settings.experimentalForceLongPolling ?? DEFAULT_FORCE_LONG_POLLING;
   }
 
   isEqual(other: FirestoreSettings): boolean {
@@ -244,7 +255,8 @@ class FirestoreSettings {
       this.timestampsInSnapshots === other.timestampsInSnapshots &&
       this.credentials === other.credentials &&
       this.cacheSizeBytes === other.cacheSizeBytes &&
-      this.forceLongPolling === other.forceLongPolling
+      this.forceLongPolling === other.forceLongPolling &&
+      this.ignoreUndefinedProperties === other.ignoreUndefinedProperties
     );
   }
 }
@@ -275,7 +287,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
   // TODO(mikelehen): Use modularized initialization instead.
   readonly _queue = new AsyncQueue();
 
-  readonly _dataReader: UserDataReader;
+  _userDataReader: UserDataReader | undefined;
 
   // Note: We are using `MemoryComponentProvider` as a default
   // ComponentProvider to ensure backwards compatibility with the format
@@ -310,7 +322,21 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
 
     this._componentProvider = componentProvider;
     this._settings = new FirestoreSettings({});
-    this._dataReader = new UserDataReader(this._databaseId);
+  }
+
+  get _dataReader(): UserDataReader {
+    debugAssert(
+      !!this._firestoreClient,
+      'Cannot obtain UserDataReader before instance is intitialized'
+    );
+    if (!this._userDataReader) {
+      // Lazy initialize UserDataReader once the settings are frozen
+      this._userDataReader = new UserDataReader(
+        this._databaseId,
+        this._settings.ignoreUndefinedProperties
+      );
+    }
+    return this._userDataReader;
   }
 
   settings(settingsLiteral: firestore.Settings): void {

packages/firestore/src/api/field_value.ts

@@ -109,7 +109,8 @@ export class ArrayUnionFieldValueImpl extends FieldValueImpl {
         arrayElement: true
       },
       context.databaseId,
-      context.serializer
+      context.serializer,
+      context.ignoreUndefinedProperties
     );
     const parsedElements = this._elements.map(
       element => parseData(element, parseContext)!
@@ -140,7 +141,8 @@ export class ArrayRemoveFieldValueImpl extends FieldValueImpl {
         arrayElement: true
       },
       context.databaseId,
-      context.serializer
+      context.serializer,
+      context.ignoreUndefinedProperties
     );
     const parsedElements = this._elements.map(
       element => parseData(element, parseContext)!
@@ -167,7 +169,8 @@ export class NumericIncrementFieldValueImpl extends FieldValueImpl {
         methodName: this._methodName
       },
       context.databaseId,
-      context.serializer
+      context.serializer,
+      context.ignoreUndefinedProperties
     );
     const operand = parseData(this._operand, parseContext)!;
     const numericIncrement = new NumericIncrementTransformOperation(

packages/firestore/src/api/user_data_reader.ts

@@ -158,6 +158,8 @@ export class ParseContext {
    * @param settings The settings for the parser.
    * @param databaseId The database ID of the Firestore instance.
    * @param serializer The serializer to use to generate the Value proto.
+   * @param ignoreUndefinedProperties Whether to ignore undefined properties
+   * rather than throw.
    * @param fieldTransforms A mutable list of field transforms encountered while
    *     parsing the data.
    * @param fieldMask A mutable list of field paths encountered while parsing
@@ -172,6 +174,7 @@ export class ParseContext {
     readonly settings: ContextSettings,
     readonly databaseId: DatabaseId,
     readonly serializer: JsonProtoSerializer,
+    readonly ignoreUndefinedProperties: boolean,
     fieldTransforms?: FieldTransform[],
     fieldMask?: FieldPath[]
   ) {
@@ -198,6 +201,7 @@ export class ParseContext {
       { ...this.settings, ...configuration },
       this.databaseId,
       this.serializer,
+      this.ignoreUndefinedProperties,
       this.fieldTransforms,
       this.fieldMask
     );
@@ -276,6 +280,7 @@ export class UserDataReader {
 
   constructor(
     private readonly databaseId: DatabaseId,
+    private readonly ignoreUndefinedProperties: boolean,
     serializer?: JsonProtoSerializer
   ) {
     this.serializer =
@@ -458,7 +463,8 @@ export class UserDataReader {
         arrayElement: false
       },
       this.databaseId,
-      this.serializer
+      this.serializer,
+      this.ignoreUndefinedProperties
     );
   }
 
@@ -613,7 +619,10 @@ function parseSentinelFieldValue(
  *
  * @return The parsed value
  */
-function parseScalarValue(value: unknown, context: ParseContext): api.Value {
+function parseScalarValue(
+  value: unknown,
+  context: ParseContext
+): api.Value | null {
   if (value === null) {
     return { nullValue: 'NULL_VALUE' };
   } else if (typeof value === 'number') {
@@ -659,6 +668,8 @@ function parseScalarValue(value: unknown, context: ParseContext): api.Value {
         value.firestore._databaseId
       )
     };
+  } else if (value === undefined && context.ignoreUndefinedProperties) {
+    return null;
   } else {
     throw context.createError(
       `Unsupported field value: ${valueDescription(value)}`

packages/firestore/src/core/firestore_client.ts

@@ -178,7 +178,7 @@ export class FirestoreClient {
           persistenceResult
         ).then(initializationDone.resolve, initializationDone.reject);
       } else {
-        this.asyncQueue.enqueueAndForget(() => {
+        this.asyncQueue.enqueueRetryable(() => {
           return this.handleCredentialChange(user);
         });
       }
@@ -497,7 +497,10 @@ export class FirestoreClient {
     if (this.clientTerminated) {
       return;
     }
-    this.eventMgr.removeSnapshotsInSyncListener(observer);
+    this.asyncQueue.enqueueAndForget(() => {
+      this.eventMgr.removeSnapshotsInSyncListener(observer);
+      return Promise.resolve();
+    });
   }
 
   get clientTerminated(): boolean {

packages/firestore/src/core/sync_engine.ts

@@ -860,15 +860,15 @@ export class SyncEngine implements RemoteSyncer {
 
   async handleCredentialChange(user: User): Promise<void> {
     const userChanged = !this.currentUser.isEqual(user);
-    this.currentUser = user;
 
     if (userChanged) {
+      const result = await this.localStore.handleUserChange(user);
+      this.currentUser = user;
+
       // Fails tasks waiting for pending writes requested by previous user.
       this.rejectOutstandingPendingWritesCallbacks(
         "'waitForPendingWrites' promise is rejected due to a user change."
       );
-
-      const result = await this.localStore.handleUserChange(user);
       // TODO(b/114226417): Consider calling this only in the primary tab.
       this.sharedClientState.handleUserChange(
         user,

packages/firestore/src/util/input_validation.ts

@@ -461,7 +461,7 @@ export function validatePositiveNumber(
   if (n <= 0) {
     throw new FirestoreError(
       Code.INVALID_ARGUMENT,
-      `Function "${functionName}()" requires its ${ordinal(
+      `Function ${functionName}() requires its ${ordinal(
         position
       )} argument to be a positive number, but it was: ${n}.`
     );