diff --git a/README.md b/README.md index 880b18ed..ae630e37 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,7 @@ Firebase. auth state, realtime data, and all other Firebase SDK events. Plus, they automatically unsubscribe when your component unmounts. - **Access Firebase libraries from any component** - Need the Firestore SDK? `useFirestore`. Remote Config? `useRemoteConfig`. - **Safely configure Firebase libraries** - Libraries like Firestore and Remote Config require settings like `enablePersistence` to be set before any data fetches are made. This can be tough to support in React's world of re-renders. ReactFire gives you `useInitFirestore` and `useInitRemoteConfig` hooks that guarantee they're set before anything else. +- **Optional `` support** - Hand loading states to React instead of checking a status yourself. Off by default, opt in with the `suspense` prop. See [Suspense](#suspense) below. ## Platform support @@ -93,19 +94,16 @@ render( This repository is maintained by Googlers but is not a supported Firebase product. Issues here are answered by maintainers and other community members on GitHub on a best-effort basis. -### Extra Experimental [concurrent mode](https://reactjs.org/docs/concurrent-mode-suspense.html) features +## Suspense -These features are marked as *extra experimental* because they use experimental React features that [will not be stable until sometime after React 18 is released](https://github.com/reactwg/react-18/discussions/47#:~:text=Likely%20after%20React%2018.0%3A%20Suspense%20for%20Data%20Fetching). +ReactFire's hooks can throw promises for [``](https://react.dev/reference/react/Suspense) to catch, so React handles loading states for you instead of you checking `status` on each result. -- **Loading states handled by ``** - ReactFire's hooks throw promises - that Suspense can catch. Let React - [handle loading states for you](https://reactjs.org/docs/concurrent-mode-suspense.html). -- **Automatically instrument your `Suspense` load times** - Need to automatically instrument your `Suspense` load times with [RUM](https://firebase.google.com/docs/perf-mon)? Use ``. - -Enable concurrent mode features by following the [concurrent mode setup guide](https://reactjs.org/docs/concurrent-mode-adoption.html#installation) and then setting the `suspense` prop in `FirebaseAppProvider`: +This is **off by default**. Opt in with the `suspense` prop on `FirebaseAppProvider`: ```jsx ``` -See concurrent mode code samples in [example/withSuspense](https://github.com/FirebaseExtended/reactfire/tree/main/example/withSuspense) +`` does the same and also measures how long the fallback was shown, using the browser's [User Timing API](https://developer.mozilla.org/en-US/docs/Web/API/Performance_API/User_timing). + +See [example/withSuspense](https://github.com/FirebaseExtended/reactfire/tree/main/example/withSuspense) for full samples. diff --git a/example/index.tsx b/example/index.tsx index 536f0242..dc047c14 100644 --- a/example/index.tsx +++ b/example/index.tsx @@ -2,13 +2,16 @@ import * as React from 'react'; import * as ReactDOM from 'react-dom'; /** - * Use this instead of NonConcurrentModeApp to see a ReactFire demo with Suspense/Concurrent mode enabled + * This demo renders without Suspense. The Suspense version is the commented-out import + * below plus the render block at the bottom of this file. * - * You'll need to use an experimental build of React to use Concurrent mode - * https://reactjs.org/docs/concurrent-mode-adoption.html#installation + * That path does not run as checked in: it needs react and react-dom on 18 or later, which + * this example is not yet on, and the `ReactDOM.render` call below has to be replaced + * rather than left alongside it. See #781 for the details. + * + * Suspense is off by default in ReactFire and is opted into with the `suspense` prop on + * `FirebaseAppProvider`. See the Suspense section of the README. */ -// import {} from 'react/experimental' // make TS aware of experimental features -// import {} from 'react-dom/experimental' // make TS aware of experimental features // import { App as ConcurrentModeApp } from './withSuspense/App'; import { App as NonConcurrentModeApp } from './withoutSuspense/App'; import './index.css'; @@ -37,7 +40,7 @@ ReactDOM.render( ); /** - * FOR CONCURRENT MODE + * FOR THE SUSPENSE VERSION */ // ReactDOM.createRoot(rootElement).render( // diff --git a/package-lock.json b/package-lock.json index 5e7c64cb..e0f85bdf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1361,9 +1361,9 @@ } }, "node_modules/@hono/node-server": { - "version": "1.19.14", - "resolved": "https://registry.npmjs.org/@hono/node-server/-/node-server-1.19.14.tgz", - "integrity": "sha512-GwtvgtXxnWsucXvbQXkRgqksiH2Qed37H9xHZocE5sA3N8O8O8/8FA3uclQXxXVzc9XBZuEOMK7+r02FmSpHtw==", + "version": "1.19.17", + "resolved": "https://registry.npmjs.org/@hono/node-server/-/node-server-1.19.17.tgz", + "integrity": "sha512-dSneS5qhiauZWGDCeK4o695Xd9nUNjviSZCMQrj10eetr8Uln1ucn6bbphOM6UynAMMtNIzZNSpL9vnASJwrPQ==", "dev": true, "license": "MIT", "engines": { @@ -9118,9 +9118,9 @@ } }, "node_modules/hono": { - "version": "4.12.27", - "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.27.tgz", - "integrity": "sha512-1yrb/+w6HWQJrUCLkJ2IF5jNIPvvFkblV5RNOYl6bV+OA6p9GLcMpHFFGTosSvHvcAUibuUukRqhlYI4z32C7Q==", + "version": "4.13.1", + "resolved": "https://registry.npmjs.org/hono/-/hono-4.13.1.tgz", + "integrity": "sha512-kdJoFVv2xmayw6cY09H7AbMJMt8Jn5jdlEdXsP7AGBdF2DIptVlKlOLKXP41yPip4/a3yQPv9gVcJYI8YY04dw==", "dev": true, "license": "MIT", "engines": { diff --git a/src/useObservable.ts b/src/useObservable.ts index 6364c9bf..580bb17a 100644 --- a/src/useObservable.ts +++ b/src/useObservable.ts @@ -104,7 +104,34 @@ export function useObservable(observableId: string, source: Observa return observable.immutableStatus; }, [observable]); - const update = useSyncExternalStore(subscribe, getSnapshot); + // Reads only `config`, never `observable.immutableStatus`: `preloadedObservables` is a + // `globalThis` cache keyed only by `observableId`, so a server shares it across concurrent + // requests, and seeding from it would render one request's data into another's HTML. + // React 18 and up only: below that the shim's server path ignores this function and returns + // `getSnapshot()`, so the cached value still reaches the markup there. + // Held in a ref because React requires a stable value across renders. + const serverSnapshotRef = React.useRef | undefined>(undefined); + const getServerSnapshot = React.useCallback<() => ObservableStatus>(() => { + if (serverSnapshotRef.current === undefined) { + const initialDataValue = config?.initialData ?? config?.startWithValue; + + serverSnapshotRef.current = { + status: hasInitialData ? 'success' : 'loading', + hasEmitted: hasInitialData, + isComplete: false, + data: initialDataValue, + error: undefined, + firstValuePromise: observable.firstEmission + }; + } + + return serverSnapshotRef.current; + // Callers pass a fresh `config` literal each render, so the fields read above are kept + // out of the deps; the ref computes the value once per instance anyway. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [observable, hasInitialData]); + + const update = useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot); // Return a new object with initialData overlaid rather than mutating the shared // _immutableStatus reference, which is the same object across all components diff --git a/test/firestore.test.tsx b/test/firestore.test.tsx index 17ced14d..0dcf0c23 100644 --- a/test/firestore.test.tsx +++ b/test/firestore.test.tsx @@ -30,6 +30,22 @@ describe('Firestore', () => { ); + // The Firestore emulator intermittently corrupts a Listen frame + // (firebase/firebase-tools#8654, unresolved upstream). The SDK reads it as + // RESOURCE_EXHAUSTED and parks the stream on a 60s maximum backoff, but a + // reconnect is not what rescues these tests: the same failure drives the + // client to OnlineState.Offline after ONLINE_STATE_TIMEOUT_MS (10s), and an + // offline client raises the pending snapshot from the local cache, empty + // cache included. These two tests assert that a document is absent, which is + // what the empty cache reports, so they go green at ~10s with no server + // involved. They are the only two whose first snapshot cannot be served from + // local data. The budget is a ceiling, not a delay, since `waitFor` polls. + // Remove when #8654 is fixed upstream. See #776. + const WAIT_FOR_OFFLINE_FALLBACK = 120_000; + // vitest enforces its own per-test ceiling, so each test below gets more than + // the sum of the budgets under it; otherwise only the first `waitFor` could + // ever spend what it is given. + afterEach(async () => { cleanup(); @@ -108,11 +124,11 @@ describe('Firestore', () => { const { result } = renderHook(() => useFirestoreDocData(ref, { idField: 'id' }), { wrapper: Provider }); - await waitFor(() => expect(result.current.status).toEqual('success')); + await waitFor(() => expect(result.current.status).toEqual('success'), { timeout: WAIT_FOR_OFFLINE_FALLBACK }); expect(result.current.status).toEqual('success'); expect(result.current.data).toBeUndefined(); - }); + }, 150_000); it('goes back into a loading state if you swap the query', async () => { const mockData = { a: 'hello' }; @@ -177,17 +193,20 @@ describe('Firestore', () => { const { result: subscribeResult } = renderHook(() => useFirestoreDoc(ref), { wrapper: Provider }); const { result: onceResult } = renderHook(() => useFirestoreDocOnce(ref), { wrapper: Provider }); - await waitFor(() => expect(subscribeResult.current.status).toEqual('success')); - await waitFor(() => expect(onceResult.current.status).toEqual('success')); + await waitFor(() => expect(subscribeResult.current.status).toEqual('success'), { timeout: WAIT_FOR_OFFLINE_FALLBACK }); + await waitFor(() => expect(onceResult.current.status).toEqual('success'), { timeout: WAIT_FOR_OFFLINE_FALLBACK }); expect(onceResult.current.data.exists()).toEqual(false); await act(() => setDoc(ref, { a: 'test' })); + // No budget: this waits on the client's own write, which is raised from + // the local cache before the acknowledgement returns (measured at 8ms + // with the client offline). await waitFor(() => expect(subscribeResult.current.data.exists()).toEqual(true)); expect(onceResult.current.data.exists()).toEqual(false); - }); + }, 270_000); }); describe('useFirestoreDocDataOnce', () => { diff --git a/test/useObservable.test.tsx b/test/useObservable.test.tsx index fd7250a9..8e900de1 100644 --- a/test/useObservable.test.tsx +++ b/test/useObservable.test.tsx @@ -1,8 +1,10 @@ import '@testing-library/jest-dom/extend-expect'; import { act, cleanup, render, renderHook, waitFor } from '@testing-library/react'; import * as React from 'react'; +import { Writable } from 'node:stream'; +import { renderToString, renderToPipeableStream } from 'react-dom/server'; import { of, Subject, BehaviorSubject, throwError } from 'rxjs'; -import { useObservable, FirebaseAppProvider } from '../src/index'; +import { useObservable, ReactFireOptions, FirebaseAppProvider } from '../src/index'; import { initializeApp } from 'firebase/app'; import { baseConfig } from './appConfig'; @@ -391,4 +393,100 @@ describe('useObservable', () => { window.removeEventListener('error', onError); }); }); + + describe('Server rendering', () => { + // Renders `status` and `data` so assertions read the snapshot React actually used. + const Probe = ({ observableId, observable$, config }: { observableId: string; observable$: Subject; config?: ReactFireOptions }) => { + const { status, data } = useObservable(observableId, observable$, { suspense: false, ...config }); + // One interpolated child: adjacent JSX text nodes render with `` between them. + return
{`${status}:${String(data)}`}
; + }; + + it('renders on the server instead of throwing', () => { + const observable$: Subject = new Subject(); + + // The #748 regression test: delete the third argument to useSyncExternalStore and + // this fails with "Missing getServerSnapshot". + expect(() => renderToString()).not.toThrow(); + }); + + // The App Router streams rather than calling renderToString, and streaming surfaces + // failures the synchronous renderer does not, so the fix is checked against both. + it('renders on the server under the streaming renderer', async () => { + const observable$: Subject = new Subject(); + let error: unknown; + + const html = await new Promise((resolve, reject) => { + const chunks: string[] = []; + const sink = new Writable({ + write(chunk, _encoding, callback) { + chunks.push(chunk.toString()); + callback(); + } + }); + sink.on('finish', () => resolve(chunks.join(''))); + sink.on('error', reject); + + const stream = renderToPipeableStream(, { + onError(caughtError) { + error = caughtError; + }, + onAllReady() { + stream.pipe(sink); + } + }); + }); + + expect(error).toBeUndefined(); + expect(html).toContain('loading:undefined'); + }); + + it('reports loading on the server when there is no initialData', () => { + const observable$: Subject = new Subject(); + + const html = renderToString(); + + expect(html).toContain('loading:undefined'); + }); + + it('reports initialData on the server when it is provided', () => { + const observable$: Subject = new Subject(); + + const html = renderToString(); + + expect(html).toContain('success:seeded'); + }); + + it('does not leak a cached value from another request into the server snapshot', async () => { + // `preloadedObservables` is on `globalThis`, keyed only by observableId, so concurrent + // server requests share it. The first render below stands in for an earlier request. + const observable$: Subject = new Subject(); + const observableId = 'ssr-no-cross-request-leak'; + + const { result } = renderHook(() => useObservable(observableId, observable$, { suspense: false })); + act(() => observable$.next('first-request-secret')); + await waitFor(() => expect(result.current.data).toEqual('first-request-secret')); + + const html = renderToString(); + + expect(html).not.toContain('first-request-secret'); + expect(html).toContain('loading:undefined'); + }); + + it('prefers the callers initialData over a value already in the shared cache', async () => { + // The `initialData` branch is only reachable when the cache already holds a value for + // this id; otherwise `useObservable`'s overlay decides and the snapshot never does. + const observable$: Subject = new Subject(); + const observableId = 'ssr-initial-data-beats-cache'; + + const { result } = renderHook(() => useObservable(observableId, observable$, { suspense: false })); + act(() => observable$.next('another-requests-value')); + await waitFor(() => expect(result.current.data).toEqual('another-requests-value')); + + const html = renderToString(); + + expect(html).toContain('success:my-own-data'); + expect(html).not.toContain('another-requests-value'); + }); + }); });