Make disable internal sync and update notes about OnlineState (#380)

* Make disableNetworkInternal explicitly synchronous

* Add notes to OnlineState type

* [AUTOMATED]: Prettier Code Styling

* [AUTOMATED]: Prettier Code Styling

* Update comments per Michael's suggestions

* More fixup

Author: Greg Soltis

packages/firestore/src/core/types.ts

@@ -30,12 +30,19 @@ export type TargetId = number;
 // they're strings. We should probably (de-)serialize to a common internal type.
 export type ProtoByteString = Uint8Array | string;
 
-/** Describes the online state of the Firestore client */
+/**
+ * Describes the online state of the Firestore client. Note that this does not
+ * indicate whether or not the remote store is trying to connect or not. This is
+ * primarily used by the View / EventManager code to change their behavior while
+ * offline (e.g. get() calls shouldn't wait for data from the server and
+ * snapshot events should set metadata.isFromCache=true).
+ */
 export enum OnlineState {
   /**
    * The Firestore client is in an unknown online state. This means the client
    * is either not actively trying to establish a connection or it is currently
    * trying to establish a connection, but it has not succeeded or failed yet.
+   * Higher-level components should not operate in offline mode.
    */
   Unknown,
 
@@ -47,9 +54,9 @@ export enum OnlineState {
   Healthy,
 
   /**
-   * The client considers itself offline. It is either trying to establish a
-   * connection but failing, or it has been explicitly marked offline via a call
-   * to disableNetwork().
+   * The client is either trying to establish a connection but failing, or it
+   * has been explicitly marked offline via a call to disableNetwork().
+   * Higher-level components should operate in offline mode.
    */
   Failed
 }

packages/firestore/src/remote/remote_store.ts

@@ -243,16 +243,15 @@ export class RemoteStore {
    */
   disableNetwork(): Promise<void> {
     // Set the OnlineState to failed so get()'s return from cache, etc.
-    return this.disableNetworkInternal(OnlineState.Failed);
+    this.disableNetworkInternal(OnlineState.Failed);
+    return Promise.resolve();
   }
 
   /**
    * Disables the network, setting the OnlineState to the specified
    * targetOnlineState.
    */
-  private disableNetworkInternal(
-    targetOnlineState: OnlineState
-  ): Promise<void> {
+  private disableNetworkInternal(targetOnlineState: OnlineState): void {
     // NOTE: We're guaranteed not to get any further events from these streams (not even a close
     // event).
     this.watchStream.stop();
@@ -265,8 +264,6 @@ export class RemoteStore {
     this.watchStream = null;
 
     this.updateOnlineState(targetOnlineState);
-
-    return Promise.resolve();
   }
 
   shutdown(): Promise<void> {