fix: Do not assume navigator is defined (#1977)

Instead get navigator from shared/src/navigator.ts which annotates the type as `Navigator | undefined` which forces us to check it before using it.
rocicorp · Jun 5, 2024 · ff18b04 · ff18b04
1 parent 6ec3986
commit ff18b04
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 15 deletions.
diff --git a/packages/reflect-client/src/client/connect-checks.ts b/packages/reflect-client/src/client/connect-checks.ts
@@ -1,5 +1,6 @@
 import type {LogContext} from '@rocicorp/logger';
 import {resolver} from '@rocicorp/resolver';
+import {navigator} from 'shared/src/navigator.js';
 import {sleep} from 'shared/src/sleep.js';
 import {nanoid} from '../util/nanoid.js';
 import {
@@ -24,7 +25,7 @@ export async function checkConnectivity(
   const id = nanoid();
   lc = lc.withContext('connectCheckID', id).withContext('checkReason', reason);
   lc.info?.('Starting connectivity checks', {
-    navigatorOnline: navigator.onLine,
+    navigatorOnline: navigator?.onLine,
   });
   const socketOrigin = toWSString(server);
   const cfChecks: Checks = {
@@ -80,7 +81,7 @@ export async function checkConnectivity(
       (checkName, i) => `${checkName}=${results[i].success}\n`,
     ),
     {
-      navigatorOnline: navigator.onLine,
+      navigatorOnline: navigator?.onLine,
     },
   );
   lc.info?.(
@@ -89,7 +90,7 @@ export async function checkConnectivity(
       (checkName, i) => `${checkName}=${results[i].detail}\n`,
     ),
     {
-      navigatorOnline: navigator.onLine,
+      navigatorOnline: navigator?.onLine,
     },
   );
 }

diff --git a/packages/reflect-client/src/client/reflect.ts b/packages/reflect-client/src/client/reflect.ts
@@ -43,6 +43,7 @@ import {assert} from 'shared/src/asserts.js';
 import {getDocumentVisibilityWatcher} from 'shared/src/document-visible.js';
 import {getDocument} from 'shared/src/get-document.js';
 import {getWindow} from 'shared/src/get-window.js';
+import {navigator} from 'shared/src/navigator.js';
 import {sleep, sleepWithAbort} from 'shared/src/sleep.js';
 import * as valita from 'shared/src/valita.js';
 import {nanoid} from '../util/nanoid.js';
@@ -637,7 +638,7 @@ export class Reflect<MD extends MutatorDefs> {
       const now = Date.now();
       const timeToOpenMs = now - this.#connectStart;
       l.info?.('Got socket open event', {
-        navigatorOnline: navigator.onLine,
+        navigatorOnline: navigator?.onLine,
         timeToOpenMs,
       });
     }
@@ -729,7 +730,7 @@ export class Reflect<MD extends MutatorDefs> {
     this.#metrics.setConnected(timeToConnectMs ?? 0, totalTimeToConnectMs ?? 0);
 
     lc.info?.('Connected', {
-      navigatorOnline: navigator.onLine,
+      navigatorOnline: navigator?.onLine,
       timeToConnectMs,
       totalTimeToConnectMs,
       connectMsgLatencyMs,
@@ -767,7 +768,7 @@ export class Reflect<MD extends MutatorDefs> {
 
     const wsid = nanoid();
     l = addWebSocketIDToLogContext(wsid, l);
-    l.info?.('Connecting...', {navigatorOnline: navigator.onLine});
+    l.info?.('Connecting...', {navigatorOnline: navigator?.onLine});
 
     this.#setConnectionState(ConnectionState.Connecting);
 
@@ -837,7 +838,7 @@ export class Reflect<MD extends MutatorDefs> {
       this.#connectErrorCount++;
     }
     l.info?.('disconnecting', {
-      navigatorOnline: navigator.onLine,
+      navigatorOnline: navigator?.onLine,
       reason,
       connectStart: this.#connectStart,
       totalToConnectStart: this.#totalToConnectStart,

diff --git a/packages/replicache/src/kv/idb-store-with-mem-fallback.ts b/packages/replicache/src/kv/idb-store-with-mem-fallback.ts
@@ -1,4 +1,5 @@
 import type {LogContext} from '@rocicorp/logger';
+import {navigator} from 'shared/src/navigator.js';
 import {promiseVoid} from 'shared/src/resolved-promises.js';
 import {IDBStore} from './idb-store.js';
 import {MemStore, dropMemStore} from './mem-store.js';
@@ -76,7 +77,7 @@ function isFirefoxPrivateBrowsingError(e: unknown): e is DOMException {
 }
 
 function isFirefox(): boolean {
-  return navigator.userAgent.includes('Firefox');
+  return navigator?.userAgent.includes('Firefox') ?? false;
 }
 
 export function newIDBStoreWithMemFallback(

diff --git a/packages/shared/src/navigator.ts b/packages/shared/src/navigator.ts
@@ -0,0 +1,10 @@
+type Navigator = {
+  onLine: boolean;
+  userAgent: string;
+  // add more as needed
+};
+
+const localNavigator: Navigator | undefined =
+  typeof navigator !== 'undefined' ? navigator : undefined;
+
+export {localNavigator as navigator};
diff --git a/packages/zero-client/src/client/connect-checks.ts b/packages/zero-client/src/client/connect-checks.ts
@@ -1,5 +1,6 @@
 import type {LogContext} from '@rocicorp/logger';
 import {resolver} from '@rocicorp/resolver';
+import {navigator} from 'shared/src/navigator.js';
 import {sleep} from 'shared/src/sleep.js';
 import {nanoid} from '../util/nanoid.js';
 import {
@@ -24,7 +25,7 @@ export async function checkConnectivity(
   const id = nanoid();
   lc = lc.withContext('connectCheckID', id).withContext('checkReason', reason);
   lc.info?.('Starting connectivity checks', {
-    navigatorOnline: navigator.onLine,
+    navigatorOnline: navigator?.onLine,
   });
   const socketOrigin = toWSString(server);
   const cfChecks: Checks = {
@@ -80,7 +81,7 @@ export async function checkConnectivity(
       (checkName, i) => `${checkName}=${results[i].success}\n`,
     ),
     {
-      navigatorOnline: navigator.onLine,
+      navigatorOnline: navigator?.onLine,
     },
   );
   lc.info?.(
@@ -89,7 +90,7 @@ export async function checkConnectivity(
       (checkName, i) => `${checkName}=${results[i].detail}\n`,
     ),
     {
-      navigatorOnline: navigator.onLine,
+      navigatorOnline: navigator?.onLine,
     },
   );
 }

diff --git a/packages/zero-client/src/client/zero.ts b/packages/zero-client/src/client/zero.ts
@@ -24,6 +24,7 @@ import {assert} from 'shared/src/asserts.js';
 import {getDocumentVisibilityWatcher} from 'shared/src/document-visible.js';
 import {getDocument} from 'shared/src/get-document.js';
 import {must} from 'shared/src/must.js';
+import {navigator} from 'shared/src/navigator.js';
 import {sleep, sleepWithAbort} from 'shared/src/sleep.js';
 import type {MaybePromise} from 'shared/src/types.js';
 import * as valita from 'shared/src/valita.js';
@@ -658,7 +659,7 @@ export class Zero<QD extends QueryDefs> {
       const now = Date.now();
       const timeToOpenMs = now - this.#connectStart;
       l.info?.('Got socket open event', {
-        navigatorOnline: navigator.onLine,
+        navigatorOnline: navigator?.onLine,
         timeToOpenMs,
       });
     }
@@ -753,7 +754,7 @@ export class Zero<QD extends QueryDefs> {
     this.#metrics.setConnected(timeToConnectMs ?? 0, totalTimeToConnectMs ?? 0);
 
     lc.info?.('Connected', {
-      navigatorOnline: navigator.onLine,
+      navigatorOnline: navigator?.onLine,
       timeToConnectMs,
       totalTimeToConnectMs,
       connectMsgLatencyMs,
@@ -801,7 +802,7 @@ export class Zero<QD extends QueryDefs> {
 
     const wsid = nanoid();
     l = addWebSocketIDToLogContext(wsid, l);
-    l.info?.('Connecting...', {navigatorOnline: navigator.onLine});
+    l.info?.('Connecting...', {navigatorOnline: navigator?.onLine});
 
     this.#setConnectionState(ConnectionState.Connecting);
 
@@ -879,7 +880,7 @@ export class Zero<QD extends QueryDefs> {
       this.#connectErrorCount++;
     }
     l.info?.('disconnecting', {
-      navigatorOnline: navigator.onLine,
+      navigatorOnline: navigator?.onLine,
       reason,
       connectStart: this.#connectStart,
       totalToConnectStart: this.#totalToConnectStart,