Make sure useUser works predictably in AuthCheck

reactfire/auth/auth.test.tsx

@@ -1,18 +1,41 @@
-import { cleanup, render } from '@testing-library/react';
-import { auth } from 'firebase/app';
+import { cleanup, render, waitForElement, wait } from '@testing-library/react';
+import { auth, User } from 'firebase/app';
 import '@testing-library/jest-dom/extend-expect';
 import * as React from 'react';
-import { AuthCheck } from '.';
+import { AuthCheck, useUser } from '.';
 import { FirebaseAppProvider } from '..';
+import { Observable, Observer } from 'rxjs';
+import { act } from 'react-dom/test-utils';
 
-const mockAuth = jest.fn(() => {
-  return {
-    onIdTokenChanged: jest.fn()
-  };
-});
+class MockAuth {
+  user: Object | null;
+  subscriber: Observer<any> | null;
+  constructor() {
+    this.user = null;
+    this.subscriber = null;
+  }
+
+  notifySubscriber() {
+    if (this.subscriber) {
+      this.subscriber.next(this.user);
+    }
+  }
+
+  onIdTokenChanged(s) {
+    this.subscriber = s;
+    this.notifySubscriber();
+  }
+
+  updateUser(u: Object) {
+    this.user = u;
+    this.notifySubscriber();
+  }
+}
+
+const mockAuth = new MockAuth();
 
 const mockFirebase = {
-  auth: mockAuth
+  auth: () => mockAuth
 };
 
 const Provider = ({ children }) => (
@@ -21,22 +44,29 @@ const Provider = ({ children }) => (
   </FirebaseAppProvider>
 );
 
+const Component = ({ children }) => (
+  <Provider>
+    <React.Suspense fallback={'loading'}>
+      <AuthCheck fallback={<h1 data-testid="signed-out">not signed in</h1>}>
+        {children || <h1 data-testid="signed-in">signed in</h1>}
+      </AuthCheck>
+    </React.Suspense>
+  </Provider>
+);
+
 describe('AuthCheck', () => {
+  beforeEach(() => {
+    // clear the signed in user
+    mockFirebase.auth().updateUser(null);
+  });
+
   afterEach(() => {
     cleanup();
     jest.clearAllMocks();
   });
 
   it('can find firebase Auth from Context', () => {
-    expect(() =>
-      render(
-        <Provider>
-          <React.Suspense fallback={'loading'}>
-            <AuthCheck fallback={'loading'}>{'children'}</AuthCheck>
-          </React.Suspense>
-        </Provider>
-      )
-    ).not.toThrow();
+    expect(() => render(<Component />)).not.toThrow();
   });
 
   it('can use firebase Auth from props', () => {
@@ -54,14 +84,62 @@ describe('AuthCheck', () => {
     ).not.toThrow();
   });
 
-  test.todo('renders the fallback if a user is not signed in');
+  it('renders the fallback if a user is not signed in', async () => {
+    const { getByTestId } = render(<Component />);
+
+    await wait(() => expect(getByTestId('signed-out')).toBeInTheDocument());
+
+    act(() => mockFirebase.auth().updateUser({ uid: 'testuser' }));
+
+    await wait(() => expect(getByTestId('signed-in')).toBeInTheDocument());
+  });
+
+  it('renders children if a user is logged in', async () => {
+    mockFirebase.auth().updateUser({ uid: 'testuser' });
+    const { getByTestId } = render(<Component />);
+
+    await wait(() => expect(getByTestId('signed-in')).toBeInTheDocument());
+  });
+
+  it('can switch between logged in and logged out', async () => {
+    const { getByTestId } = render(<Component />);
+
+    await wait(() => expect(getByTestId('signed-out')).toBeInTheDocument());
+
+    act(() => mockFirebase.auth().updateUser({ uid: 'testuser' }));
+
+    await wait(() => expect(getByTestId('signed-in')).toBeInTheDocument());
 
-  test.todo('renders children if a user is logged in');
+    act(() => mockFirebase.auth().updateUser(null));
+
+    await wait(() => expect(getByTestId('signed-out')).toBeInTheDocument());
+  });
 
   test.todo('checks requiredClaims');
 });
 
 describe('useUser', () => {
+  it('always returns a user if inside an <AuthCheck> component', () => {
+    const UserDetails = () => {
+      const user = useUser();
+
+      expect(user).not.toBeNull();
+      expect(user).toBeDefined();
+
+      return <h1>Hello</h1>;
+    };
+
+    render(
+      <>
+        <Component>
+          <UserDetails />
+        </Component>
+      </>
+    );
+
+    act(() => mockFirebase.auth().updateUser({ uid: 'testuser' }));
+  });
+
   test.todo('can find firebase.auth() from Context');
 
   test.todo('throws an error if firebase.auth() is not available');

reactfire/package.json

@@ -38,7 +38,7 @@
     "@firebase/testing": "^0.11.4",
     "@testing-library/jest-dom": "^4.1.1",
     "@testing-library/react": "^9.3.0",
-    "@testing-library/react-hooks": "^2.0.3",
+    "@testing-library/react-hooks": "^3.1.0",
     "@types/jest": "^24.0.11",
     "babel-jest": "^24.7.1",
     "firebase-tools": "^7.1.0",

reactfire/useObservable/index.ts

@@ -31,14 +31,22 @@ export function useObservable(
   observableId: string,
   startWithValue?: any
 ) {
+  const request = requestCache.getRequest(observable$, observableId);
+
   const initialValue =
-    startWithValue || suspendUntilFirst(observable$, observableId);
+    request.value ||
+    startWithValue ||
+    suspendUntilFirst(observable$, observableId);
 
   const [latestValue, setValue] = React.useState(initialValue);
 
   React.useEffect(() => {
     const subscription = observable$.pipe(startWith(initialValue)).subscribe(
       newVal => {
+        // update the value in requestCache
+        request.setValue(newVal);
+
+        // update state
         setValue(newVal);
       },
       error => {

reactfire/useObservable/useObservable.test.tsx

@@ -1,11 +1,9 @@
-import { useObservable } from '.';
-import { renderHook, act } from '@testing-library/react-hooks';
-import { of, Subject, Observable, observable } from 'rxjs';
-import { delay } from 'rxjs/operators';
-import { render, waitForElement, cleanup } from '@testing-library/react';
-import { ReactFireOptions } from '..';
-import * as React from 'react';
 import '@testing-library/jest-dom/extend-expect';
+import { act, cleanup, render, waitForElement } from '@testing-library/react';
+import { act as actOnHook, renderHook } from '@testing-library/react-hooks';
+import * as React from 'react';
+import { of, Subject } from 'rxjs';
+import { useObservable } from '.';
 
 describe('useObservable', () => {
   afterEach(cleanup);
@@ -32,20 +30,33 @@ describe('useObservable', () => {
     expect(result.current).toEqual(startVal);
 
     // prove that it actually does emit the value from the observable too
-    act(() => observable$.next(observableVal));
+    actOnHook(() => observable$.next(observableVal));
     expect(result.current).toEqual(observableVal);
   });
 
-  it('ignores provided initial value if the observable is ready right away', () => {
+  it('returns the provided startWithValue first even if the observable is ready right away', () => {
+    // This behavior is a consequense of how observables work. There is
+    // not a synchronous way to ask an observable if it has a value to emit.
+
     const startVal = 'howdy';
     const observableVal = "y'all";
     const observable$ = of(observableVal);
+    let hasReturnedStartWithValue = false;
 
-    const { result, waitForNextUpdate } = renderHook(() =>
-      useObservable(observable$, 'test', startVal)
-    );
+    const Component = () => {
+      const val = useObservable(observable$, 'test', startVal);
 
-    expect(result.current).toEqual(observableVal);
+      if (hasReturnedStartWithValue) {
+        expect(val).toEqual(observableVal);
+      } else {
+        expect(val).toEqual(startVal);
+        hasReturnedStartWithValue = true;
+      }
+
+      return <h1>Hello</h1>;
+    };
+
+    render(<Component />);
   });
 
   it('works with Suspense', async () => {
@@ -73,7 +84,7 @@ describe('useObservable', () => {
     expect(getByTestId(fallbackComponentId)).toBeInTheDocument();
     expect(queryByTestId(actualComponentId)).toBeNull();
 
-    act(() => observable$.next(observableFinalVal));
+    actOnHook(() => observable$.next(observableFinalVal));
     await waitForElement(() => getByTestId(actualComponentId));
 
     // make sure Suspense correctly renders its child after the observable emits a value
@@ -87,7 +98,6 @@ describe('useObservable', () => {
   it('emits new values as the observable changes', async () => {
     const startVal = 'start';
     const values = ['a', 'b', 'c'];
-    const observableSecondValue = 'b';
     const observable$ = new Subject();
 
     const { result } = renderHook(() =>
@@ -97,8 +107,55 @@ describe('useObservable', () => {
     expect(result.current).toEqual(startVal);
 
     values.forEach(value => {
-      act(() => observable$.next(value));
+      actOnHook(() => observable$.next(value));
       expect(result.current).toEqual(value);
     });
   });
+
+  it('returns the most recent value of an observable to all subscribers of an observableId', async () => {
+    const values = ['a', 'b', 'c'];
+    const observable$ = new Subject();
+    const observableId = 'my-observable-id';
+    const firstComponentId = 'first';
+    const secondComponentId = 'second';
+
+    const ObservableConsumer = props => {
+      const val = useObservable(observable$, observableId);
+
+      return <h1 {...props}>{val}</h1>;
+    };
+
+    const Component = ({ renderSecondComponent }) => {
+      return (
+        <React.Suspense fallback="loading">
+          <ObservableConsumer data-testid={firstComponentId} />
+          {renderSecondComponent ? (
+            <ObservableConsumer data-testid={secondComponentId} />
+          ) : null}
+        </React.Suspense>
+      );
+    };
+
+    const { getByTestId, rerender } = render(
+      <Component renderSecondComponent={false} />
+    );
+
+    // emit one value to the first component (second one isn't rendered yet)
+    act(() => observable$.next(values[0]));
+    const comp = await waitForElement(() => getByTestId(firstComponentId));
+    expect(comp).toHaveTextContent(values[0]);
+
+    // emit a second value to the first component (second one still isn't rendered)
+    act(() => observable$.next(values[1]));
+    expect(comp).toHaveTextContent(values[1]);
+
+    // keep the original component around, but now render the second one.
+    // they both use the same observableId
+    rerender(<Component renderSecondComponent={true} />);
+
+    // the second component should start by receiving the latest value
+    // since the first component has already been subscribed
+    const comp2 = await waitForElement(() => getByTestId(secondComponentId));
+    expect(comp2).toHaveTextContent(values[1]);
+  });
 });

yarn.lock

@@ -1885,10 +1885,10 @@
     pretty-format "^24.0.0"
     redent "^3.0.0"
 
-"@testing-library/react-hooks@^2.0.3":
-  version "2.0.3"
-  resolved "https://registry.yarnpkg.com/@testing-library/react-hooks/-/react-hooks-2.0.3.tgz#305a6c76facb5fa1d185792b9eb11b1ca1b63fb7"
-  integrity sha512-adm+7b1gcysGka8VuYq/ObBrIBJTT9QmCEIqPpuxozWFfVDgxSbzBGc44ia/WYLGVt2dqFIOc6/DmAmu/pa0gQ==
+"@testing-library/react-hooks@^3.1.0":
+  version "3.1.0"
+  resolved "https://registry.yarnpkg.com/@testing-library/react-hooks/-/react-hooks-3.1.0.tgz#f186c4f3b32db153d30d646faacb043ef4089807"
+  integrity sha512-mwFDHXCQiyr0tQkYU4VkcwlCzR5YQ5k1/TCrL3hPslCM5MvS6pBhbl2z4UnCMV4DOyiUUXIvoMAf5kHT/hibTg==
   dependencies:
     "@babel/runtime" "^7.5.4"
     "@types/testing-library__react-hooks" "^2.0.0"