breaking: refactor network status interface (#368)

* breaking: refactor network status interface * breaking: change api for NetworkStatus interface * fix: change api in networkstatus related tests and implement new methods * fix: lint error from unnecessary import * fix: network integration tests
aerogear · Mar 3, 2020 · 05625f2 · 05625f2
1 parent 60c745c
commit 05625f2
Show file tree

Hide file tree

Showing 12 changed files with 115 additions and 41 deletions.
diff --git a/packages/offix-client-boost/integration_test/utils/network.js b/packages/offix-client-boost/integration_test/utils/network.js
@@ -14,7 +14,7 @@ export class ToggleableNetworkStatus {
     this.callbacks = []
   }
 
-  onStatusChangeListener(callback) {
+  addListener(callback) {
     this.callbacks.push(callback)
   }
 
@@ -26,7 +26,7 @@ export class ToggleableNetworkStatus {
   setOnline(online) {
     this.online = online;
     for (const callback of this.callbacks) {
-      callback.onStatusChange({ online })
+      callback({ online })
     }
   }
 };
diff --git a/packages/offix-client/integration_test/utils/network.js b/packages/offix-client/integration_test/utils/network.js
@@ -14,7 +14,7 @@ export class ToggleableNetworkStatus {
     this.callbacks = []
   }
 
-  onStatusChangeListener(callback) {
+  addListener(callback) {
     this.callbacks.push(callback)
   }
 
@@ -26,7 +26,7 @@ export class ToggleableNetworkStatus {
   setOnline(online) {
     this.online = online;
     for (const callback of this.callbacks) {
-      callback.onStatusChange({ online })
+      callback({ online })
     }
   }
 };
diff --git a/packages/offix-client/test/mock/MockNetworkStatus.ts b/packages/offix-client/test/mock/MockNetworkStatus.ts
@@ -7,10 +7,17 @@ export class MockNetworkStatus {
     this.callbacks = [];
   }
 
-  public onStatusChangeListener(callback: any) {
+  public addListener(callback: any) {
     this.callbacks.push(callback);
   }
 
+  public removeListener(callback: any) {
+    const index = this.callbacks.indexOf(callback);
+    if (index >= 0) {
+      this.callbacks.splice(index, 1);
+    }
+  }
+
   public isOffline(): Promise<boolean> {
     const online = this.online;
     return new Promise(resolve => resolve(!online));
@@ -19,7 +26,7 @@ export class MockNetworkStatus {
   public setOnline(online: boolean) {
     this.online = online;
     for (const callback of this.callbacks) {
-      callback.onStatusChange({ online });
+      callback({ online });
     }
   }
 }
diff --git a/packages/offix-offline/src/network/CordovaNetworkStatus.ts b/packages/offix-offline/src/network/CordovaNetworkStatus.ts
@@ -7,10 +7,22 @@ declare let document: any;
  * See: https://cordova.apache.org/docs/en/latest/reference/cordova-plugin-network-information
  */
 export class CordovaNetworkStatus implements NetworkStatus {
-  public onStatusChangeListener(callback: NetworkStatusChangeCallback): void {
-    if (document) {
-      document.addEventListener("online", () => callback.onStatusChange({online: true}), false);
-      document.addEventListener("offline", () => callback.onStatusChange({online: false}), false);
+
+  listeners: NetworkStatusChangeCallback[] = [];
+
+  constructor() {
+    document.addEventListener("online", this.handleNetworkStatusChange.bind(this), false);
+    document.addEventListener("offline", this.handleNetworkStatusChange.bind(this), false);
+  }
+
+  public addListener(listener: NetworkStatusChangeCallback): void {
+    this.listeners.push(listener);
+  }
+
+  public removeListener(listener: NetworkStatusChangeCallback): void {
+    const index = this.listeners.indexOf(listener);
+    if (index >= 0) {
+      this.listeners.splice(index, 1);
     }
   }
 
@@ -22,4 +34,11 @@ export class CordovaNetworkStatus implements NetworkStatus {
     });
   }
 
+  private handleNetworkStatusChange() {
+    const online = window.navigator.onLine;
+    this.listeners.forEach((listener) => {
+      listener({ online });
+    });
+  }
+
 }
diff --git a/packages/offix-offline/src/network/NetworkStatus.ts b/packages/offix-offline/src/network/NetworkStatus.ts
@@ -2,20 +2,25 @@ export interface NetworkInfo {
   online: boolean;
 }
 
-export interface NetworkStatusChangeCallback {
-  onStatusChange(info: NetworkInfo): void;
-}
+export type NetworkStatusChangeCallback = (info: NetworkInfo) => void;
 
 /**
  * Responsable to handle Networks status
  */
 export interface NetworkStatus {
   /**
-   * Trigger callback whenever the network status change
+   * Register callback whenever the network status change
+   *
+   * @param callback Callback to be added when network status change
+   */
+  addListener(listener: NetworkStatusChangeCallback): void;
+
+  /**
+   * Remove callback whenever the network status change
    *
-   * @param callback Callback to be called when network status change
+   * @param callback Callback to be removed when network status change
    */
-  onStatusChangeListener(callback: NetworkStatusChangeCallback): void;
+  removeListener(listener: NetworkStatusChangeCallback): void;
 
   /**
    * Check if device is offline

diff --git a/packages/offix-offline/src/network/WebNetworkStatus.ts b/packages/offix-offline/src/network/WebNetworkStatus.ts
@@ -7,10 +7,22 @@ declare let window: any;
  * See: https://developer.mozilla.org/en-US/docs/Web/API/NavigatorOnLine/onLine
  */
 export class WebNetworkStatus implements NetworkStatus {
-  public onStatusChangeListener(callback: NetworkStatusChangeCallback): void {
-    if (window) {
-      window.addEventListener("online", () => callback.onStatusChange({online: true}), false);
-      window.addEventListener("offline", () => callback.onStatusChange({online: false}), false);
+
+  listeners: NetworkStatusChangeCallback[] = [];
+
+  constructor() {
+    window.addEventListener("online", this.handleNetworkStatusChange.bind(this), false);
+    window.addEventListener("offline", this.handleNetworkStatusChange.bind(this), false);
+  }
+
+  public addListener(listener: NetworkStatusChangeCallback): void {
+    this.listeners.push(listener);
+  }
+
+  public removeListener(listener: NetworkStatusChangeCallback): void {
+    const index = this.listeners.indexOf(listener);
+    if (index >= 0) {
+      this.listeners.splice(index, 1);
     }
   }
 
@@ -19,4 +31,11 @@ export class WebNetworkStatus implements NetworkStatus {
       resolve(!window.navigator.onLine);
     });
   }
+
+  private handleNetworkStatusChange() {
+    const online = window.navigator.onLine;
+    this.listeners.forEach((listener) => {
+      listener({ online });
+    });
+  }
 }
diff --git a/packages/offix-offline/test/mock/NetworkState.ts b/packages/offix-offline/test/mock/NetworkState.ts
@@ -2,7 +2,10 @@ export const networkStatus = {
   isOffline() {
     return Promise.resolve(false);
   },
-  onStatusChangeListener() {
+  addListener() {
+    return;
+  },
+  removeListener() {
     return;
   }
 };
diff --git a/packages/offix-scheduler/src/OffixScheduler.ts b/packages/offix-scheduler/src/OffixScheduler.ts
@@ -123,13 +123,15 @@ export class OffixScheduler<T> {
     if (this.online) {
       queue.forwardOperations();
     }
-    this.networkStatus.onStatusChangeListener({
-      onStatusChange(networkInfo: NetworkInfo) {
-        self.online = networkInfo.online;
-        if (self.online) {
-          queue.forwardOperations();
-        }
+
+    const listener = (networkInfo: NetworkInfo) => {
+      self.online = networkInfo.online;
+      if (self.online) {
+        queue.forwardOperations();
       }
-    });
+    };
+
+    this.networkStatus.addListener(listener);
+
   }
 }
diff --git a/packages/offix-scheduler/test/mock/ToggleableNetworkStatus.ts b/packages/offix-scheduler/test/mock/ToggleableNetworkStatus.ts
@@ -7,10 +7,17 @@ export class ToggleableNetworkStatus {
     this.callbacks = [];
   }
 
-  public onStatusChangeListener(callback: any) {
+  public addListener(callback: any) {
     this.callbacks.push(callback);
   }
 
+  public removeListener(callback: any) {
+    const index = this.callbacks.indexOf(callback);
+    if (index >= 0) {
+      this.callbacks.splice(index, 1);
+    }
+  }
+
   public isOffline(): Promise<boolean> {
     const online = this.online;
     return new Promise(resolve => resolve(!online));
@@ -19,7 +26,7 @@ export class ToggleableNetworkStatus {
   public setOnline(online: boolean) {
     this.online = online;
     for (const callback of this.callbacks) {
-      callback.onStatusChange({ online });
+      callback({ online });
     }
   }
 }
diff --git a/packages/react-offix-hooks/src/useNetworkStatus.ts b/packages/react-offix-hooks/src/useNetworkStatus.ts
@@ -1,5 +1,6 @@
 import  { useEffect, useState } from "react";
 import { useApolloOfflineClient } from "./ApolloOfflineProvider";
+import { NetworkStatusChangeCallback } from "offix-client";
 
 /**
  * React hook to detect network changes
@@ -13,7 +14,7 @@ export function useNetworkStatus(){
   const [isOnline, setIsOnline] = useState();
 
   useEffect(() => {
-    const setOnlineStatus  = async () => {
+    async function setOnlineStatus() {
       // check if app is offline and return result
       const offline = await client.networkStatus.isOffline();
       // set network state with result of offline check
@@ -22,12 +23,16 @@ export function useNetworkStatus(){
 
     setOnlineStatus();
 
+    // get result and set online state to result
+    const listener: NetworkStatusChangeCallback = ({ online }) => setIsOnline(online);
+
     // set up network listener to
-    client.networkStatus.onStatusChangeListener({
-      // get result and set online state to result
-      onStatusChange: ({ online }) => setIsOnline(online)
-    });
-  }, []);
+    client.networkStatus.addListener(listener);
+
+    return function cleanup() {
+      client.networkStatus.removeListener(listener);
+    };
+  }, [client]);
 
   return isOnline;
 };
diff --git a/packages/react-offix-hooks/test/mock/MockNetworkStatus.ts b/packages/react-offix-hooks/test/mock/MockNetworkStatus.ts
@@ -7,10 +7,17 @@ export class MockNetworkStatus {
     this.callbacks = [];
   }
 
-  public onStatusChangeListener(callback: any) {
+  public addListener(callback: any) {
     this.callbacks.push(callback);
   }
 
+  public removeListener(callback: any) {
+    const index = this.callbacks.indexOf(callback);
+    if (index >= 0) {
+      this.callbacks.splice(index, 1);
+    }
+  }
+
   public isOffline(): Promise<boolean> {
     const online = this.online;
     return new Promise(resolve => resolve(!online));
@@ -19,7 +26,7 @@ export class MockNetworkStatus {
   public setOnline(online: boolean) {
     this.online = online;
     for (const callback of this.callbacks) {
-      callback.onStatusChange({ online });
+      callback({ online });
     }
   }
 }
diff --git a/packages/react-offix-hooks/test/useNetworkStatus.test.tsx b/packages/react-offix-hooks/test/useNetworkStatus.test.tsx
@@ -10,10 +10,10 @@ import { HttpLink } from "apollo-link-http";
 import { MockNetworkStatus } from "./mock/MockNetworkStatus";
 
 const createClient = async ({ online } : { online: boolean }) => {
-  let link = new HttpLink({ uri: "http://test" });
-  let networkStatus = new MockNetworkStatus();
+  const link = new HttpLink({ uri: "http://test" });
+  const networkStatus = new MockNetworkStatus();
   networkStatus.setOnline(online);
-  let client = new ApolloOfflineClient({
+  const client = new ApolloOfflineClient({
     cache: new InMemoryCache(),
     link,
     networkStatus