Make sure Firestore hooks return undefined when data does not exist (#446)

More context in discussion #438

Author: jhuleatt

docs/reference/classes/SuspenseSubject.SuspenseSubject-1.md

@@ -82,7 +82,7 @@
     "babel-jest": "^26.6.3",
     "babel-plugin-minify-replace": "^0.5.0",
     "eslint-plugin-no-only-tests": "^2.6.0",
-    "firebase": "^9.0.0",
+    "firebase": "^9.0.1",
     "firebase-tools": "^9.16.0",
     "globalthis": "^1.0.1",
     "husky": "^4.3.0",

docs/reference/modules/useObservable.md

@@ -48,14 +48,16 @@ export class SuspenseSubject<T> extends Subject<T> {
     return this._hasValue || !!this._error;
   }
 
-  get value(): T | undefined {
+  get value(): T {
     // TODO figure out how to reset the cache here, if I _reset() here before throwing
     // it doesn't seem to work.
     // As it is now, this will burn the cache entry until the timeout fires.
     if (this._error) {
       throw this._error;
+    } else if (!this.hasValue) {
+      throw Error('Can only get value if SuspenseSubject has a value');
     }
-    return this._value;
+    return this._value as T;
   }
 
   get firstEmission(): Promise<void> {

package.json

@@ -63,55 +63,75 @@ export interface ObservableStatus<T> {
   firstValuePromise: Promise<void>;
 }
 
+function reducerFactory<T>(observable: SuspenseSubject<T>) {
+  return function reducer(state: ObservableStatus<T>, action: 'value' | 'error' | 'complete'): ObservableStatus<T> {
+    // always make sure these values are in sync with the observable
+    const newState = {
+      ...state,
+      hasEmitted: state.hasEmitted || observable.hasValue,
+      error: observable.ourError,
+      firstValuePromise: observable.firstEmission,
+    };
+    if (observable.hasValue) {
+      newState.data = observable.value;
+    }
+
+    switch (action) {
+      case 'value':
+        newState.status = 'success';
+        return newState;
+      case 'error':
+        newState.status = 'error';
+        return newState;
+      case 'complete':
+        newState.isComplete = true;
+        return newState;
+      default:
+        throw new Error(`invalid action "${action}"`);
+    }
+  };
+}
+
 export function useObservable<T = unknown>(observableId: string, source: Observable<T>, config: ReactFireOptions = {}): ObservableStatus<T> {
+  // Register the observable with the cache
   if (!observableId) {
     throw new Error('cannot call useObservable without an observableId');
   }
   const observable = preloadObservable(source, observableId);
 
+  // Suspend if suspense is enabled and no initial data exists
   const hasInitialData = config.hasOwnProperty('initialData') || config.hasOwnProperty('startWithValue');
-
+  const hasData = observable.hasValue || hasInitialData;
   const suspenseEnabled = useSuspenseEnabledFromConfigAndContext(config.suspense);
-
-  if (suspenseEnabled === true && !observable.hasValue && (!config?.initialData ?? !config?.startWithValue)) {
+  if (suspenseEnabled === true && !hasData) {
     throw observable.firstEmission;
   }
 
-  const [latest, setValue] = React.useState(() => (observable.hasValue ? observable.value : config.initialData ?? config.startWithValue));
-  const [isComplete, setIsComplete] = React.useState(false);
-  const [hasError, setHasError] = React.useState(false);
+  const initialState: ObservableStatus<T> = {
+    status: hasData ? 'success' : 'loading',
+    hasEmitted: hasData,
+    isComplete: false,
+    data: observable.hasValue ? observable.value : config?.initialData ?? config?.startWithValue,
+    error: observable.ourError,
+    firstValuePromise: observable.firstEmission,
+  };
+  const [status, dispatch] = React.useReducer<React.Reducer<ObservableStatus<T>, 'value' | 'error' | 'complete'>>(reducerFactory<T>(observable), initialState);
+
   React.useEffect(() => {
     const subscription = observable.subscribe({
-      next: (v) => {
-        setValue(() => v);
+      next: () => {
+        dispatch('value');
       },
       error: (e) => {
-        setHasError(true);
+        dispatch('error');
         throw e;
       },
       complete: () => {
-        setIsComplete(true);
+        dispatch('complete');
       },
     });
     return () => subscription.unsubscribe();
   }, [observable]);
 
-  let status: ObservableStatus<T>['status'];
-
-  if (hasError) {
-    status = 'error';
-  } else if (observable.hasValue || hasInitialData) {
-    status = 'success';
-  } else {
-    status = 'loading';
-  }
-
-  return {
-    status,
-    hasEmitted: observable.hasValue || hasInitialData,
-    isComplete: isComplete,
-    data: latest,
-    error: observable.ourError,
-    firstValuePromise: observable.firstEmission,
-  };
+  return status;
 }

src/SuspenseSubject.ts

@@ -85,6 +85,23 @@ describe('Firestore', () => {
       expect(data.a).toEqual(mockData.a);
       expect(data.id).toBeDefined();
     });
+
+    it('returns undefined if document does not exist', async () => {
+      const collectionRef = collection(db, randomString());
+      const docIdThatExists = randomString();
+      const docIdThatDoesNotExist = randomString();
+      await setDoc(doc(collectionRef, docIdThatExists), { a: randomString() });
+
+      // reference a doc that doesn't exist
+      const ref = doc(collectionRef, docIdThatDoesNotExist);
+
+      const { result, waitFor } = renderHook(() => useFirestoreDocData<any>(ref, { idField: 'id' }), { wrapper: Provider });
+
+      await waitFor(() => result.current.status === 'success');
+
+      expect(result.current.status).toEqual('success');
+      expect(result.current.data).toBeUndefined();
+    });
   });
 
   describe('useFirestoreDocOnce', () => {

src/useObservable.ts

@@ -63,6 +63,19 @@ describe('useObservable', () => {
       expect(result.current.data).toEqual(startVal);
     });
 
+    it('emits even if data is undefined', async () => {
+      const observable$: Subject<any> = new Subject();
+
+      const { result } = renderHook(() => useObservable('test-undefined', observable$, { suspense: false }));
+
+      expect(result.current.status).toEqual('loading');
+
+      actOnHook(() => observable$.next(undefined));
+
+      expect(result.current.status).toEqual('success');
+      expect(result.current.data).toBeUndefined();
+    });
+
     it('does not show stale data after navigating away', async () => {
       const startVal = 'start';
       const newVal = 'anotherValue';