diff --git a/reactfire/auth/auth.test.tsx b/reactfire/auth/auth.test.tsx index 546bd396..352813b2 100644 --- a/reactfire/auth/auth.test.tsx +++ b/reactfire/auth/auth.test.tsx @@ -3,7 +3,7 @@ import { auth, User } from 'firebase/app'; import '@testing-library/jest-dom/extend-expect'; import * as React from 'react'; import { AuthCheck, useUser } from '.'; -import { FirebaseAppProvider } from '..'; +import { FirebaseAppProvider, clearCache } from '..'; import { Observable, Observer } from 'rxjs'; import { act } from 'react-dom/test-utils'; @@ -62,6 +62,7 @@ describe('AuthCheck', () => { afterEach(() => { cleanup(); + clearCache(); jest.clearAllMocks(); }); @@ -115,6 +116,7 @@ describe('AuthCheck', () => { await wait(() => expect(getByTestId('signed-out')).toBeInTheDocument()); }); + // implement this once we have an auth emulator test.todo('checks requiredClaims'); }); diff --git a/reactfire/auth/index.tsx b/reactfire/auth/index.tsx index d7c8bf18..4837b340 100644 --- a/reactfire/auth/index.tsx +++ b/reactfire/auth/index.tsx @@ -12,11 +12,10 @@ import { from } from 'rxjs'; export function preloadUser(firebaseApp: firebase.app.App) { return preloadAuth(firebaseApp).then(auth => { - const result = preloadObservable( + return preloadObservable( user(auth() as firebase.auth.Auth), 'auth: user' - ); - return result.request.promise; + ).getPromise(); }); } diff --git a/reactfire/firebaseApp/sdk.tsx b/reactfire/firebaseApp/sdk.tsx index fc2c3035..b4761d79 100644 --- a/reactfire/firebaseApp/sdk.tsx +++ b/reactfire/firebaseApp/sdk.tsx @@ -1,4 +1,4 @@ -import { useFirebaseApp, preloadRequest, usePreloadedRequest } from '..'; +import { useFirebaseApp, preloadPromise, usePreloadedObservable } from '..'; enum SDK { ANALYTICS = 'analytics', AUTH = 'auth', @@ -74,13 +74,17 @@ function fetchSDK(sdk: SDK, firebaseApp: firebase.app.App) { function useSDK(sdk: SDK, firebaseApp?: firebase.app.App) { firebaseApp = firebaseApp || useFirebaseApp(); + // before we worry about fetching, check to see if the SDK is already loaded + if (firebaseApp[sdk]) { + return firebaseApp[sdk]; + } + + const requestId = `firebase-sdk-${sdk}`; + // use the request cache so we don't issue multiple fetches for the sdk - const result = preloadRequest( - () => fetchSDK(sdk, firebaseApp), - `firebase-sdk-${sdk}` - ); + preloadPromise(() => fetchSDK(sdk, firebaseApp), requestId); - return usePreloadedRequest(result); + return usePreloadedObservable(requestId); } export function preloadAuth(firebaseApp: firebase.app.App) { diff --git a/reactfire/firestore/index.tsx b/reactfire/firestore/index.tsx index da5ee8e7..046c79a5 100644 --- a/reactfire/firestore/index.tsx +++ b/reactfire/firestore/index.tsx @@ -28,7 +28,7 @@ export function preloadFirestoreDoc( ) { return preloadFirestore(firebaseApp).then(firestore => { const ref = refProvider(firestore() as firebase.firestore.Firestore); - return preloadObservable(doc(ref), ref.path); + return preloadObservable(doc(ref), 'firestore doc: ' + ref.path); }); } @@ -110,7 +110,7 @@ export function useFirestoreCollectionData( query: firestore.Query, options?: ReactFireOptions ): T[] { - const queryId = getHashFromFirestoreQuery(query); + const queryId = getHashFromFirestoreQuery(query) + ' data only'; return useObservable( collectionData(query, checkIdField(options)), diff --git a/reactfire/package.json b/reactfire/package.json index 0bdba95c..b9eb8bd8 100644 --- a/reactfire/package.json +++ b/reactfire/package.json @@ -9,7 +9,7 @@ "build-dev": "tsc --watch", "test-dev": "jest --verbose --watch", "emulators": "firebase emulators:start --only firestore,database", - "test": "firebase emulators:exec --only firestore,database \"jest --no-cache --verbose --detectOpenHandles --forceExit\"", + "test": "firebase emulators:exec --only firestore,database \"jest --no-cache --detectOpenHandles --forceExit\"", "copy-package-json": "cp package.pub.json pub/reactfire/package.json", "watch": "yarn build && tsc --watch", "build": "rm -rf pub && tsc && yarn copy-package-json && cp ../README.md pub/reactfire/README.md && cp ../LICENSE pub/reactfire/LICENSE && rollup -c" diff --git a/reactfire/performance/performance.test.tsx b/reactfire/performance/performance.test.tsx index 14d3caa2..218c7395 100644 --- a/reactfire/performance/performance.test.tsx +++ b/reactfire/performance/performance.test.tsx @@ -4,7 +4,7 @@ import '@testing-library/jest-dom/extend-expect'; import * as React from 'react'; import { Subject } from 'rxjs'; import { SuspenseWithPerf } from '.'; -import { FirebaseAppProvider, useObservable } from '..'; +import { FirebaseAppProvider, useObservable, clearCache } from '..'; const traceStart = jest.fn(); const traceEnd = jest.fn(); @@ -36,6 +36,7 @@ const Provider = ({ children }) => ( describe('SuspenseWithPerf', () => { afterEach(() => { cleanup(); + clearCache(); jest.clearAllMocks(); }); @@ -206,7 +207,7 @@ describe('SuspenseWithPerf', () => { return ( Actual} firePerf={(mockPerf() as unknown) as performance.Performance} > @@ -215,7 +216,8 @@ describe('SuspenseWithPerf', () => { }; // render SuspenseWithPerf and go through normal trace start -> trace stop - const { getByTestId, rerender } = render(); + const { getByTestId } = render(); + await waitForElement(() => getByTestId('fallback')); expect(createTrace).toHaveBeenCalledTimes(1); act(() => o$.next('some value')); await waitForElement(() => getByTestId('child')); diff --git a/reactfire/storage/index.tsx b/reactfire/storage/index.tsx index 3852333d..168af50e 100644 --- a/reactfire/storage/index.tsx +++ b/reactfire/storage/index.tsx @@ -3,6 +3,7 @@ import { storage } from 'firebase/app'; import { getDownloadURL } from 'rxfire/storage'; import { Observable } from 'rxjs'; import { ReactFireOptions, useObservable, useFirebaseApp } from '..'; +import { useStorage } from '../firebaseApp'; /** * modified version of rxFire's _fromTask @@ -75,7 +76,7 @@ export function StorageImage( ) { let { storage, storagePath, ...imgProps } = props; - storage = storage || useFirebaseApp().storage(); + storage = storage || useStorage()(); const imgSrc = useStorageDownloadURL(storage.ref(storagePath)); return ; diff --git a/reactfire/useObservable/ActiveObservable.ts b/reactfire/useObservable/ActiveObservable.ts new file mode 100644 index 00000000..47898663 --- /dev/null +++ b/reactfire/useObservable/ActiveObservable.ts @@ -0,0 +1,81 @@ +import { first, tap, takeUntil, shareReplay, startWith } from 'rxjs/operators'; +import { Observable, timer, observable } from 'rxjs'; +type Observer = import('rxjs').Observer; + +export class ActiveObservable { + observable$: Observable; + isReady: boolean; + value: any = undefined; + error: Error; + subscribers: number = 0; + + constructor(observable$: Observable, startWithValue?) { + if (startWithValue) { + this.value = startWithValue; + this.isReady = true; + observable$ = observable$.pipe(startWith(startWithValue)); + } else { + this.isReady = false; + } + + // create a shared observable + // we need to keep track of the latest value emitted so that we can tell React + // whether the component subscribed to this observable is ready to render + this.observable$ = observable$.pipe( + tap( + newVal => { + this.setValue(newVal); + }, + error => { + this.setError(error); + throw error; + } + ), + shareReplay({ + bufferSize: 1, // only cache the latest value + refCount: true // clean up the subscription if we don't have any subscribers + }) + ); + } + + subscribeTemporarily(timeout: number, onComplete) { + this.subscribers++; + + this.observable$ + .pipe(takeUntil(timer(timeout))) + .subscribe( + () => {}, + () => {}, + () => { + this.subscribers--; + // onComplete() + } + ) + .add(onComplete); + } + + subscribe(observer: Observer) { + this.subscribers++; + const subscription = this.observable$.subscribe(observer); + + subscription.add(() => { + console.log('UNSUBSCRIBE CALLED'); + this.subscribers--; + }); + return subscription; + } + + getPromise() { + return this.observable$.pipe(first()).toPromise(); + } + + setValue = value => { + this.value = value; + this.isReady = true; + }; + + setError = err => { + this.error = err; + this.isReady = true; + }; +} diff --git a/reactfire/useObservable/index.ts b/reactfire/useObservable/index.ts index 28c28029..1520b909 100644 --- a/reactfire/useObservable/index.ts +++ b/reactfire/useObservable/index.ts @@ -1,48 +1,61 @@ import * as React from 'react'; -import { Observable } from 'rxjs'; -import { first, startWith } from 'rxjs/operators'; -import { ActiveRequest, ObservablePromiseCache } from './requestCache'; - -const requestCache = new ObservablePromiseCache(); - -export function preloadRequest( - getPromise, - requestId: string -): { requestId: string; request: ActiveRequest } { - const request = requestCache.createDedupedRequest(getPromise, requestId); - - return { - requestId: requestId, - request - }; +import { Observable, timer, from } from 'rxjs'; +import { first, startWith, shareReplay, takeUntil } from 'rxjs/operators'; +import { ActiveObservable } from './ActiveObservable'; + +const PRELOAD_SUBSCRIBE_TIME = 5 /*seconds*/ * 1000; +const observableCache = new Map(); + +export function preloadPromise(getPromise, requestId: string): Promise { + const activeObservable = preloadObservable(from(getPromise()), requestId); + + return activeObservable.getPromise(); } -// Starts listening to an Observable. -// Call this once you know you're going to render a -// child that will consume the observable +// Registers an Observable and starts listening to it to prime it for when a real +// subscriber starts listening export function preloadObservable( observable$: Observable, - observableId: string -): { requestId: string; request: ActiveRequest } { - return preloadRequest( - () => observable$.pipe(first()).toPromise(), - observableId - ); + observableId: string, + startWithValue? +): ActiveObservable { + // If we already have something in the cache with that ID, re-use it + if (observableCache.has(observableId)) { + return observableCache.get(observableId); + } + + const activeObservable = new ActiveObservable(observable$, startWithValue); + observableCache.set(observableId, activeObservable); + + // subscribe to the observable so that we can get a value + // this subscription cleans itself up after PRELOAD_SUBSCRIBE_TIME passes + activeObservable.subscribeTemporarily(PRELOAD_SUBSCRIBE_TIME, () => { + if (activeObservable.subscribers === 0) { + console.log(`CLEANING UP ${observableId}`); + observableCache.delete(observableId); + } + }); + + return activeObservable; } -export function usePreloadedRequest(preloadResult: { requestId: string }) { - const request = requestCache.getRequest(preloadResult.requestId); +export function usePreloadedObservable(observableId: string) { + if (!observableCache.has(observableId)) { + throw new Error(`Observable "${observableId}" doesn't exist!`); + } + + const activeObservable = observableCache.get(observableId); // Suspend if we're not ready yet - if (!request.isComplete) { - throw request.promise; + if (!activeObservable.isReady) { + throw activeObservable.getPromise(); } - if (request.error) { - throw request.error; + if (activeObservable.error) { + throw activeObservable.error; } - return request.value; + return activeObservable.value; } export function useObservable( @@ -54,36 +67,52 @@ export function useObservable( throw new Error('cannot call useObservable without an observableId'); } - const result = preloadObservable(observable$, observableId); + // register observable in the cache + const activeObservable = preloadObservable( + observable$, + observableId, + startWithValue + ); - let initialValue = startWithValue; - if (initialValue === undefined) { + if (activeObservable.isReady === false) { // this will Suspend until the Promise resolves - initialValue = usePreloadedRequest(result); + usePreloadedObservable(observableId); } - const [latestValue, setValue] = React.useState(initialValue); + if (activeObservable.error) { + throw activeObservable.error; + } - React.useEffect(() => { - const subscription = observable$.pipe(startWith(initialValue)).subscribe( - newVal => { - // update the value in requestCache - result.request.setValue(newVal); + const [latestValue, setValue] = React.useState(activeObservable.value); - // update state + React.useEffect(() => { + const subscription = activeObservable.subscribe({ + next: newVal => { setValue(newVal); }, - error => { - console.error('There was an error', error); + error: error => { throw error; - } - ); + }, + complete: () => {} + }); return () => { subscription.unsubscribe(); - requestCache.removeRequest(observableId); + + // if we were the last subscriber, remove the observable from the cache + if (activeObservable.subscribers === 0) { + observableCache.delete(observableId); + } }; - }, [observableId]); + }, [activeObservable]); return latestValue; } + +export function clearCache() { + observableCache.clear(); +} + +export function getCache() { + return observableCache; +} diff --git a/reactfire/useObservable/requestCache.ts b/reactfire/useObservable/requestCache.ts deleted file mode 100644 index 526bd5e0..00000000 --- a/reactfire/useObservable/requestCache.ts +++ /dev/null @@ -1,80 +0,0 @@ -import { first, take } from 'rxjs/operators'; -import { Observable } from 'rxjs'; - -export class ActiveRequest { - promise: Promise; - isComplete: boolean; - value: any; - error: Error; - - constructor(promise) { - this.isComplete = false; - this.promise = promise - .then(result => { - this.setValue(result); - return result; - }) - .catch(err => { - this.isComplete = true; - this.setError(err); - }); - } - - setValue = value => { - this.value = value; - this.isComplete = true; - }; - - setError = err => { - this.error = err; - this.isComplete = true; - }; -} - -/* - * this will probably be replaced by something - * like react-cache (https://www.npmjs.com/package/react-cache) - * once that is stable. - * - * Full Suspense roadmap: https://reactjs.org/blog/2018/11/27/react-16-roadmap.html - */ -export class ObservablePromiseCache { - activeRequests: Map; - - constructor() { - this.activeRequests = new Map(); - } - - getRequest(requestId) { - const request = this.activeRequests.get(requestId); - if (request === undefined) { - throw new Error(`No request with ID "${requestId}" exists`); - } - return request; - } - - createRequest(promise: Promise, requestId): ActiveRequest { - if (this.activeRequests.get(requestId) !== undefined) { - throw new Error(`request "${requestId}" is already in use.`); - } - - const request = new ActiveRequest(promise); - this.activeRequests.set(requestId, request); - - return request; - } - - createDedupedRequest(getPromise: () => Promise, requestId) { - let request = this.activeRequests.get(requestId); - - if (request === undefined) { - request = this.createRequest(getPromise(), requestId); - } - - return request; - } - - removeRequest(requestId: string) { - this.activeRequests.delete(requestId); - } -} diff --git a/reactfire/useObservable/useObservable.test.tsx b/reactfire/useObservable/useObservable.test.tsx index 7378b76d..f3eda898 100644 --- a/reactfire/useObservable/useObservable.test.tsx +++ b/reactfire/useObservable/useObservable.test.tsx @@ -1,18 +1,26 @@ 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 { + act as actOnHook, + renderHook, + cleanup as cleanupHooks +} from '@testing-library/react-hooks'; import * as React from 'react'; -import { of, Subject, BehaviorSubject, throwError } from 'rxjs'; -import { useObservable } from '.'; +import { of, Subject, BehaviorSubject, throwError, Observable } from 'rxjs'; +import { useObservable, clearCache } from '.'; describe('useObservable', () => { - afterEach(cleanup); + afterEach(() => { + cleanupHooks(); + cleanup(); + clearCache(); + }); it('throws a promise if the observable has no initial value', () => { const observable$: Subject = new Subject(); try { - useObservable(observable$, 'test'); + renderHook(() => useObservable(observable$, 'test')); } catch (thingThatWasThrown) { expect(thingThatWasThrown).toBeInstanceOf(Promise); } @@ -22,7 +30,7 @@ describe('useObservable', () => { const observable$: Subject = new Subject(); try { - useObservable(observable$, undefined); + renderHook(() => useObservable(observable$, undefined)); } catch (thingThatWasThrown) { expect(thingThatWasThrown).toBeInstanceOf(Error); } @@ -46,12 +54,23 @@ describe('useObservable', () => { it('throws an error if there is an error on initial fetch', async () => { const error = new Error('I am an error'); - const observable$ = throwError(error); + let observable$ = throwError(error); - // stop a nasty-looking console error + // stop a nasty-looking console log of the error we're trying to throw // https://github.com/facebook/react/issues/11098#issuecomment-523977830 const spy = jest.spyOn(console, 'error'); - spy.mockImplementation(() => {}); + spy.mockImplementation(e => { + if ( + typeof e === 'string' && + (e.includes('I am an error') || + e.includes('React will try to recreate')) + ) { + return; + } + + // log any error that isn't one we expect + console.log(e); + }); class ErrorBoundary extends React.Component<{}, { hasError: boolean }> { constructor(props) { @@ -79,6 +98,7 @@ describe('useObservable', () => { const Component = () => { const val = useObservable(observable$, 'test-error'); + console.log('I SHOULD NEVER GET HERE', val); return

{val}

; }; diff --git a/sample/src/App.js b/sample/src/App.js index 55e21f25..e8167f9d 100644 --- a/sample/src/App.js +++ b/sample/src/App.js @@ -1,16 +1,19 @@ -import React from 'react'; +import React, { useState, useEffect } from 'react'; import AuthButton from './Auth'; import FirestoreCounter from './Firestore'; import Storage from './Storage'; import RealtimeDatabase from './RealtimeDatabase'; import { preloadFirestoreDoc, + preloadFirestoreCollection, useFirebaseApp, preloadUser, preloadAuth, preloadFirestore, preloadDatabase, - preloadStorage + preloadStorage, + useFirestore, + getCache } from 'reactfire'; const Fire = () => ( @@ -20,12 +23,16 @@ const Fire = () => ( ); const Card = ({ title, children }) => { + const [mounted, setMounted] = useState(title !== 'Firestore'); + return (

{title} +

- {children} + + {mounted ? children : null}
); }; @@ -50,10 +57,65 @@ const preloadData = async firebaseApp => { preloadFirestoreDoc( firestore => firestore.doc('count/counter'), firebaseApp - ); + ) + .then(() => console.log('PRELOAD COMPLETE')) + .catch(console.error); + + // preloadFirestoreCollection( + // firestore => firestore.collection('animals').orderBy('commonName', 'asc'), + // firebaseApp + // ); } }; +function FirestoreMonitor() { + const firestore = useFirestore(); + const [time, setTime] = useState(1); + useEffect(() => { + const interval = setInterval(() => setTime(Date.now()), 500); + + return () => clearInterval(interval); + }); + let activeQueries = []; + const queries = firestore()._firestoreClient?.eventMgr?.queries; + const cacheKeys = Array.from(getCache().keys()); + + if (queries) { + queries.forEach(query => { + activeQueries.push(query.path.toString()); + }); + } + + return ( + <> + Active Firestore Queries +
    + {activeQueries.map(q => { + return ( +
  • +
    {q}
    +
  • + ); + })} +
+ Cache keys +
    + {cacheKeys.map(q => { + const numSubs = getCache().get(q).subscribers; + + return ( +
  • +
    +                {numSubs}: {q}
    +              
    +
  • + ); + })} +
+ + ); +} + const App = () => { const firebaseApp = useFirebaseApp(); @@ -62,29 +124,38 @@ const App = () => { // // This is OPTIONAL but encouraged as part of the render-as-you-fetch pattern // https://reactjs.org/docs/concurrent-mode-suspense.html#approach-3-render-as-you-fetch-using-suspense - preloadSDKs(firebaseApp).then(preloadData(firebaseApp)); + preloadSDKs(firebaseApp); + preloadData(firebaseApp); return ( <> +  

ReactFire Demo

+ + +
- + {/* + */} + {/* + + - + */} - + {/* - + */}
);