Cleanup

Author: schmidt-sebastian

packages/firestore/exp/src/api/components.ts

@@ -63,16 +63,16 @@ export async function setOfflineComponentProvider(
   await offlineComponentProvider.initialize(configuration);
 
   firestore._setCredentialChangeListener(user =>
-    firestore._queue.enqueueRetryable(() =>
-      handleUserChange(offlineComponentProvider.localStore, user).then(() => {})
-    )
+    firestore._queue.enqueueRetryable(async () => {
+      await handleUserChange(offlineComponentProvider.localStore, user);
+    })
   );
-
   // When a user calls clearPersistence() in one client, all other clients
   // need to be terminated to allow the delete to succeed.
   offlineComponentProvider.persistence.setDatabaseDeletedListener(() =>
     firestore._delete()
   );
+
   offlineComponentProviders.set(firestore, offlineComponentProvider);
 }
 
@@ -90,7 +90,6 @@ export async function setOnlineComponentProvider(
     offlineComponentProvider,
     configuration
   );
-
   // The CredentialChangeListener of the online component provider takes
   // precedence over the offline component provider.
   firestore._setCredentialChangeListener(user =>
@@ -101,6 +100,7 @@ export async function setOnlineComponentProvider(
       )
     )
   );
+
   onlineComponentProviders.set(firestore, onlineComponentProvider);
 }
 
@@ -117,6 +117,7 @@ export async function getOfflineComponentProvider(
       new MemoryOfflineComponentProvider()
     );
   }
+
   return offlineComponentProviders.get(firestore)!;
 }
 
@@ -130,53 +131,52 @@ export async function getOnlineComponentProvider(
     logDebug(LOG_TAG, 'Using default OnlineComponentProvider');
     await setOnlineComponentProvider(firestore, new OnlineComponentProvider());
   }
+
   return onlineComponentProviders.get(firestore)!;
 }
 
-export function getSyncEngine(
+export async function getSyncEngine(
   firestore: FirebaseFirestore
 ): Promise<SyncEngine> {
-  return getOnlineComponentProvider(firestore).then(
-    components => components.syncEngine
-  );
+  const onlineComponentProvider = await getOnlineComponentProvider(firestore);
+  return onlineComponentProvider.syncEngine;
 }
 
-export function getRemoteStore(
-  firestore: FirestoreCompat
+export async function getRemoteStore(
+  firestore: FirebaseFirestore
 ): Promise<RemoteStore> {
-  return getOnlineComponentProvider(firestore).then(
-    components => components.remoteStore
-  );
+  const onlineComponentProvider = await getOnlineComponentProvider(firestore);
+  return onlineComponentProvider.remoteStore;
 }
 
-export function getEventManager(
+export async function getEventManager(
   firestore: FirebaseFirestore
 ): Promise<EventManager> {
-  return getOnlineComponentProvider(firestore).then(components => {
-    const eventManager = components.eventManager;
-    eventManager.onListen = syncEngineListen.bind(null, components.syncEngine);
-    eventManager.onUnlisten = syncEngineUnlisten.bind(
-      null,
-      components.syncEngine
-    );
-    return eventManager;
-  });
+  const onlineComponentProvider = await getOnlineComponentProvider(firestore);
+  const eventManager = onlineComponentProvider.eventManager;
+  eventManager.onListen = syncEngineListen.bind(
+    null,
+    onlineComponentProvider.syncEngine
+  );
+  eventManager.onUnlisten = syncEngineUnlisten.bind(
+    null,
+    onlineComponentProvider.syncEngine
+  );
+  return eventManager;
 }
 
-export function getPersistence(
+export async function getPersistence(
   firestore: FirebaseFirestore
 ): Promise<Persistence> {
-  return getOfflineComponentProvider(firestore).then(
-    components => components.persistence
-  );
+  const offlineComponentProvider = await getOfflineComponentProvider(firestore);
+  return offlineComponentProvider.persistence;
 }
 
-export function getLocalStore(
+export async function getLocalStore(
   firestore: FirebaseFirestore
 ): Promise<LocalStore> {
-  return getOfflineComponentProvider(firestore).then(
-    provider => provider.localStore
-  );
+  const offlineComponentProvider = await getOfflineComponentProvider(firestore);
+  return offlineComponentProvider.localStore;
 }
 
 /**

packages/firestore/exp/src/api/database.ts

@@ -283,7 +283,7 @@ export function enableIndexedDbPersistence(
   const offlineComponentProvider = new IndexedDbOfflineComponentProvider(
     onlineComponentProvider,
     settings.cacheSizeBytes,
-    !!persistenceSettings?.forceOwnership
+    persistenceSettings?.forceOwnership
   );
   return setPersistenceProviders(
     firestore,
@@ -356,12 +356,12 @@ function setPersistenceProviders(
         await setOnlineComponentProvider(firestore, onlineComponentProvider);
         persistenceResult.resolve();
       } catch (e) {
-        if (!canRecoverFromIndexedDbError(e)) {
+        if (!canFallbackFromIndexedDbError(e)) {
           throw e;
         }
         console.warn(
-          'Error enabling offline persistence. Falling back to persistenc' +
-            'disabled: ' +
+          'Error enabling offline persistence. Falling back to ' +
+            'persistence disabled: ' +
             e
         );
         persistenceResult.reject(e);
@@ -374,7 +374,9 @@ function setPersistenceProviders(
  * Decides whether the provided error allows us to gracefully disable
  * persistence (as opposed to crashing the client).
  */
-function canRecoverFromIndexedDbError(
+// TODO(schmidt-sebastian): Remove `export` in
+// https://github.com/firebase/firebase-js-sdk/pull/3901
+export function canFallbackFromIndexedDbError(
   error: FirestoreError | DOMException
 ): boolean {
   if (error.name === 'FirebaseError') {

packages/firestore/index.node.ts

@@ -17,10 +17,7 @@
 import firebase from '@firebase/app';
 import { FirebaseNamespace } from '@firebase/app-types';
 
-import {
-  Firestore,
-  MultiTabIndexedDbPersistenceProvider
-} from './src/api/database';
+import { Firestore, IndexedDbPersistenceProvider } from './src/api/database';
 import { configureForFirebase } from './src/config';
 
 import './register-module';
@@ -33,7 +30,7 @@ import { name, version } from './package.json';
  */
 export function registerFirestore(instance: FirebaseNamespace): void {
   configureForFirebase(instance, (app, auth) => {
-    return new Firestore(app, auth, new MultiTabIndexedDbPersistenceProvider());
+    return new Firestore(app, auth, new IndexedDbPersistenceProvider());
   });
   instance.registerVersion(name, version, 'node');
 }

packages/firestore/index.rn.ts

@@ -17,10 +17,7 @@
 import firebase from '@firebase/app';
 import { FirebaseNamespace } from '@firebase/app-types';
 
-import {
-  Firestore,
-  MultiTabIndexedDbPersistenceProvider
-} from './src/api/database';
+import { Firestore, IndexedDbPersistenceProvider } from './src/api/database';
 import { configureForFirebase } from './src/config';
 
 import './register-module';
@@ -32,7 +29,7 @@ import { name, version } from './package.json';
  */
 export function registerFirestore(instance: FirebaseNamespace): void {
   configureForFirebase(instance, (app, auth) => {
-    return new Firestore(app, auth, new MultiTabIndexedDbPersistenceProvider());
+    return new Firestore(app, auth, new IndexedDbPersistenceProvider());
   });
   instance.registerVersion(name, version, 'rn');
 }

packages/firestore/index.ts

@@ -18,10 +18,7 @@
 import firebase from '@firebase/app';
 import { FirebaseNamespace } from '@firebase/app-types';
 
-import {
-  Firestore,
-  MultiTabIndexedDbPersistenceProvider
-} from './src/api/database';
+import { Firestore, IndexedDbPersistenceProvider } from './src/api/database';
 import { configureForFirebase } from './src/config';
 import { name, version } from './package.json';
 
@@ -33,7 +30,7 @@ import './register-module';
  */
 export function registerFirestore(instance: FirebaseNamespace): void {
   configureForFirebase(instance, (app, auth) => {
-    return new Firestore(app, auth, new MultiTabIndexedDbPersistenceProvider());
+    return new Firestore(app, auth, new IndexedDbPersistenceProvider());
   });
   instance.registerVersion(name, version);
 }

packages/firestore/src/api/database.ts

@@ -163,7 +163,6 @@ import {
 // settings() defaults:
 const DEFAULT_HOST = 'firestore.googleapis.com';
 const DEFAULT_SSL = true;
-const DEFAULT_TIMESTAMPS_IN_SNAPSHOTS = true;
 const DEFAULT_FORCE_LONG_POLLING = false;
 const DEFAULT_FORCE_AUTO_DETECT_LONG_POLLING = false;
 const DEFAULT_IGNORE_UNDEFINED_PROPERTIES = false;
@@ -177,6 +176,7 @@ export const CACHE_SIZE_UNLIMITED = LruParams.COLLECTION_DISABLED;
 
 // enablePersistence() defaults:
 const DEFAULT_SYNCHRONIZE_TABS = false;
+const DEFAULT_FORCE_OWNING_TAB = false;
 
 /** Undocumented, private additional settings not exposed in our public API. */
 interface PrivateSettings extends PublicSettings {
@@ -205,8 +205,6 @@ class FirestoreSettings {
   /** Whether to use SSL when connecting. */
   readonly ssl: boolean;
 
-  readonly timestampsInSnapshots: boolean;
-
   readonly cacheSizeBytes: number;
 
   readonly experimentalForceLongPolling: boolean;
@@ -240,7 +238,6 @@ class FirestoreSettings {
       'host',
       'ssl',
       'credentials',
-      'timestampsInSnapshots',
       'cacheSizeBytes',
       'experimentalForceLongPolling',
       'experimentalAutoDetectLongPolling',
@@ -255,35 +252,13 @@ class FirestoreSettings {
     );
     this.credentials = settings.credentials;
 
-    validateNamedOptionalType(
-      'settings',
-      'boolean',
-      'timestampsInSnapshots',
-      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) {
-      logError(
-        "The setting 'timestampsInSnapshots: true' is no longer required " +
-          'and should be removed.'
-      );
-    } else if (settings.timestampsInSnapshots === false) {
-      logError(
-        "Support for 'timestampsInSnapshots: false' will be removed soon. " +
-          'You must update your code to handle Timestamp objects.'
-      );
-    }
-    this.timestampsInSnapshots =
-      settings.timestampsInSnapshots ?? DEFAULT_TIMESTAMPS_IN_SNAPSHOTS;
     this.ignoreUndefinedProperties =
       settings.ignoreUndefinedProperties ?? DEFAULT_IGNORE_UNDEFINED_PROPERTIES;
 
@@ -340,7 +315,6 @@ class FirestoreSettings {
     return (
       this.host === other.host &&
       this.ssl === other.ssl &&
-      this.timestampsInSnapshots === other.timestampsInSnapshots &&
       this.credentials === other.credentials &&
       this.cacheSizeBytes === other.cacheSizeBytes &&
       this.experimentalForceLongPolling ===
@@ -354,8 +328,7 @@ class FirestoreSettings {
 
 /**
  * A persistence provider for either memory-only or IndexedDB persistence.
- * Mainly used to ensure that IndexedDB code is not included in memory-only
- * builds.
+ * Mainly used to allow optional inclusion of IndexedDB code.
  */
 export interface PersistenceProvider {
   enableIndexedDbPersistence(
@@ -385,6 +358,7 @@ export class MemoryPersistenceProvider implements PersistenceProvider {
       MEMORY_ONLY_PERSISTENCE_ERROR_MESSAGE
     );
   }
+
   enableMultiTabIndexedDbPersistence(
     firestore: FirestoreCompat
   ): Promise<void> {
@@ -393,6 +367,7 @@ export class MemoryPersistenceProvider implements PersistenceProvider {
       MEMORY_ONLY_PERSISTENCE_ERROR_MESSAGE
     );
   }
+
   clearIndexedDbPersistence(firestore: FirestoreCompat): Promise<void> {
     throw new FirestoreError(
       Code.FAILED_PRECONDITION,
@@ -446,6 +421,7 @@ export class Firestore
   // Public for use in tests.
   // TODO(mikelehen): Use modularized initialization instead.
   readonly _queue = new AsyncQueue();
+
   _userDataReader: UserDataReader | undefined;
 
   // Note: We are using `MemoryPersistenceProvider` as a default
@@ -582,26 +558,16 @@ export class Firestore
     let experimentalForceOwningTab = false;
 
     if (settings) {
-      if (settings.experimentalTabSynchronization !== undefined) {
-        logError(
-          "The 'experimentalTabSynchronization' setting will be removed. Use 'synchronizeTabs' instead."
-        );
-      }
-      synchronizeTabs =
-        settings.synchronizeTabs ??
-        settings.experimentalTabSynchronization ??
-        DEFAULT_SYNCHRONIZE_TABS;
-
-      experimentalForceOwningTab = settings.experimentalForceOwningTab
-        ? settings.experimentalForceOwningTab
-        : false;
-
-      if (synchronizeTabs && experimentalForceOwningTab) {
-        throw new FirestoreError(
-          Code.INVALID_ARGUMENT,
-          "The 'experimentalForceOwningTab' setting cannot be used with 'synchronizeTabs'."
-        );
-      }
+      synchronizeTabs = settings.synchronizeTabs ?? DEFAULT_SYNCHRONIZE_TABS;
+      experimentalForceOwningTab =
+        settings.experimentalForceOwningTab ?? DEFAULT_FORCE_OWNING_TAB;
+
+      validateIsNotUsedTogether(
+        'synchronizeTabs',
+        synchronizeTabs,
+        'experimentalForceOwningTab',
+        experimentalForceOwningTab
+      );
     }
 
     return synchronizeTabs

packages/firestore/src/core/component_provider.ts

@@ -156,8 +156,8 @@ export class IndexedDbOfflineComponentProvider extends MemoryOfflineComponentPro
 
   constructor(
     protected readonly onlineComponentProvider: OnlineComponentProvider,
-    protected readonly cacheSizeBytes?: number,
-    protected readonly forceOwnership?: boolean
+    protected readonly cacheSizeBytes: number | undefined,
+    protected readonly forceOwnership: boolean | undefined
   ) {
     super();
   }
@@ -224,7 +224,7 @@ export class MultiTabOfflineComponentProvider extends IndexedDbOfflineComponentP
 
   constructor(
     protected readonly onlineComponentProvider: OnlineComponentProvider,
-    protected readonly cacheSizeBytes?: number
+    protected readonly cacheSizeBytes: number | undefined
   ) {
     super(onlineComponentProvider, cacheSizeBytes, /* forceOwnership= */ false);
   }