Skip to content

Commit 996468d

Browse files
authored
Merge pull request #796 from FirebaseExtended/chore/forward-integrate-main-to-v5-0820
chore: forward-integrate main into v5
2 parents e7b18c2 + e4c38db commit 996468d

6 files changed

Lines changed: 173 additions & 28 deletions

File tree

‎README.md‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ Firebase.
1010
auth state, realtime data, and all other Firebase SDK events. Plus, they automatically unsubscribe when your component unmounts.
1111
- **Access Firebase libraries from any component** - Need the Firestore SDK? `useFirestore`. Remote Config? `useRemoteConfig`.
1212
- **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.
13+
- **Optional `<Suspense>` 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.
1314

1415
## Platform support
1516

@@ -93,19 +94,16 @@ render(
9394

9495
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.
9596

96-
### Extra Experimental [concurrent mode](https://reactjs.org/docs/concurrent-mode-suspense.html) features
97+
## Suspense
9798

98-
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).
99+
ReactFire's hooks can throw promises for [`<Suspense>`](https://react.dev/reference/react/Suspense) to catch, so React handles loading states for you instead of you checking `status` on each result.
99100

100-
- **Loading states handled by `<Suspense>`** - ReactFire's hooks throw promises
101-
that Suspense can catch. Let React
102-
[handle loading states for you](https://reactjs.org/docs/concurrent-mode-suspense.html).
103-
- **Automatically instrument your `Suspense` load times** - Need to automatically instrument your `Suspense` load times with [RUM](https://firebase.google.com/docs/perf-mon)? Use `<SuspenseWithPerf />`.
104-
105-
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`:
101+
This is **off by default**. Opt in with the `suspense` prop on `FirebaseAppProvider`:
106102

107103
```jsx
108104
<FirebaseAppProvider firebaseConfig={firebaseConfig} suspense={true}>
109105
```
110106

111-
See concurrent mode code samples in [example/withSuspense](https://github.com/FirebaseExtended/reactfire/tree/main/example/withSuspense)
107+
`<SuspenseWithPerf />` 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).
108+
109+
See [example/withSuspense](https://github.com/FirebaseExtended/reactfire/tree/main/example/withSuspense) for full samples.

‎example/index.tsx‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,16 @@ import * as React from 'react';
22
import * as ReactDOM from 'react-dom';
33

44
/**
5-
* Use this instead of NonConcurrentModeApp to see a ReactFire demo with Suspense/Concurrent mode enabled
5+
* This demo renders without Suspense. The Suspense version is the commented-out import
6+
* below plus the render block at the bottom of this file.
67
*
7-
* You'll need to use an experimental build of React to use Concurrent mode
8-
* https://reactjs.org/docs/concurrent-mode-adoption.html#installation
8+
* That path does not run as checked in: it needs react and react-dom on 18 or later, which
9+
* this example is not yet on, and the `ReactDOM.render` call below has to be replaced
10+
* rather than left alongside it. See #781 for the details.
11+
*
12+
* Suspense is off by default in ReactFire and is opted into with the `suspense` prop on
13+
* `FirebaseAppProvider`. See the Suspense section of the README.
914
*/
10-
// import {} from 'react/experimental' // make TS aware of experimental features
11-
// import {} from 'react-dom/experimental' // make TS aware of experimental features
1215
// import { App as ConcurrentModeApp } from './withSuspense/App';
1316
import { App as NonConcurrentModeApp } from './withoutSuspense/App';
1417
import './index.css';
@@ -37,7 +40,7 @@ ReactDOM.render(
3740
);
3841

3942
/**
40-
* FOR CONCURRENT MODE
43+
* FOR THE SUSPENSE VERSION
4144
*/
4245
// ReactDOM.createRoot(rootElement).render(
4346
// <FirebaseAppProvider firebaseConfig={firebaseConfig} suspense={true}>

‎package-lock.json‎

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/useObservable.ts‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,34 @@ export function useObservable<T = unknown>(observableId: string, source: Observa
104104
return observable.immutableStatus;
105105
}, [observable]);
106106

107-
const update = useSyncExternalStore(subscribe, getSnapshot);
107+
// Reads only `config`, never `observable.immutableStatus`: `preloadedObservables` is a
108+
// `globalThis` cache keyed only by `observableId`, so a server shares it across concurrent
109+
// requests, and seeding from it would render one request's data into another's HTML.
110+
// React 18 and up only: below that the shim's server path ignores this function and returns
111+
// `getSnapshot()`, so the cached value still reaches the markup there.
112+
// Held in a ref because React requires a stable value across renders.
113+
const serverSnapshotRef = React.useRef<ObservableStatus<T> | undefined>(undefined);
114+
const getServerSnapshot = React.useCallback<() => ObservableStatus<T>>(() => {
115+
if (serverSnapshotRef.current === undefined) {
116+
const initialDataValue = config?.initialData ?? config?.startWithValue;
117+
118+
serverSnapshotRef.current = {
119+
status: hasInitialData ? 'success' : 'loading',
120+
hasEmitted: hasInitialData,
121+
isComplete: false,
122+
data: initialDataValue,
123+
error: undefined,
124+
firstValuePromise: observable.firstEmission
125+
};
126+
}
127+
128+
return serverSnapshotRef.current;
129+
// Callers pass a fresh `config` literal each render, so the fields read above are kept
130+
// out of the deps; the ref computes the value once per instance anyway.
131+
// eslint-disable-next-line react-hooks/exhaustive-deps
132+
}, [observable, hasInitialData]);
133+
134+
const update = useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot);
108135

109136
// Return a new object with initialData overlaid rather than mutating the shared
110137
// _immutableStatus reference, which is the same object across all components

‎test/firestore.test.tsx‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,22 @@ describe('Firestore', () => {
3030
</FirebaseAppProvider>
3131
);
3232

33+
// The Firestore emulator intermittently corrupts a Listen frame
34+
// (firebase/firebase-tools#8654, unresolved upstream). The SDK reads it as
35+
// RESOURCE_EXHAUSTED and parks the stream on a 60s maximum backoff, but a
36+
// reconnect is not what rescues these tests: the same failure drives the
37+
// client to OnlineState.Offline after ONLINE_STATE_TIMEOUT_MS (10s), and an
38+
// offline client raises the pending snapshot from the local cache, empty
39+
// cache included. These two tests assert that a document is absent, which is
40+
// what the empty cache reports, so they go green at ~10s with no server
41+
// involved. They are the only two whose first snapshot cannot be served from
42+
// local data. The budget is a ceiling, not a delay, since `waitFor` polls.
43+
// Remove when #8654 is fixed upstream. See #776.
44+
const WAIT_FOR_OFFLINE_FALLBACK = 120_000;
45+
// vitest enforces its own per-test ceiling, so each test below gets more than
46+
// the sum of the budgets under it; otherwise only the first `waitFor` could
47+
// ever spend what it is given.
48+
3349
afterEach(async () => {
3450
cleanup();
3551

@@ -108,11 +124,11 @@ describe('Firestore', () => {
108124

109125
const { result } = renderHook(() => useFirestoreDocData<any>(ref, { idField: 'id' }), { wrapper: Provider });
110126

111-
await waitFor(() => expect(result.current.status).toEqual('success'));
127+
await waitFor(() => expect(result.current.status).toEqual('success'), { timeout: WAIT_FOR_OFFLINE_FALLBACK });
112128

113129
expect(result.current.status).toEqual('success');
114130
expect(result.current.data).toBeUndefined();
115-
});
131+
}, 150_000);
116132

117133
it('goes back into a loading state if you swap the query', async () => {
118134
const mockData = { a: 'hello' };
@@ -177,17 +193,20 @@ describe('Firestore', () => {
177193
const { result: subscribeResult } = renderHook(() => useFirestoreDoc(ref), { wrapper: Provider });
178194
const { result: onceResult } = renderHook(() => useFirestoreDocOnce(ref), { wrapper: Provider });
179195

180-
await waitFor(() => expect(subscribeResult.current.status).toEqual('success'));
181-
await waitFor(() => expect(onceResult.current.status).toEqual('success'));
196+
await waitFor(() => expect(subscribeResult.current.status).toEqual('success'), { timeout: WAIT_FOR_OFFLINE_FALLBACK });
197+
await waitFor(() => expect(onceResult.current.status).toEqual('success'), { timeout: WAIT_FOR_OFFLINE_FALLBACK });
182198

183199
expect(onceResult.current.data.exists()).toEqual(false);
184200

185201
await act(() => setDoc(ref, { a: 'test' }));
186202

203+
// No budget: this waits on the client's own write, which is raised from
204+
// the local cache before the acknowledgement returns (measured at 8ms
205+
// with the client offline).
187206
await waitFor(() => expect(subscribeResult.current.data.exists()).toEqual(true));
188207

189208
expect(onceResult.current.data.exists()).toEqual(false);
190-
});
209+
}, 270_000);
191210
});
192211

193212
describe('useFirestoreDocDataOnce', () => {

‎test/useObservable.test.tsx‎

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import '@testing-library/jest-dom/extend-expect';
22
import { act, cleanup, render, renderHook, waitFor } from '@testing-library/react';
33
import * as React from 'react';
4+
import { Writable } from 'node:stream';
5+
import { renderToString, renderToPipeableStream } from 'react-dom/server';
46
import { of, Subject, BehaviorSubject, throwError } from 'rxjs';
5-
import { useObservable, FirebaseAppProvider } from '../src/index';
7+
import { useObservable, ReactFireOptions, FirebaseAppProvider } from '../src/index';
68
import { initializeApp } from 'firebase/app';
79
import { baseConfig } from './appConfig';
810

@@ -391,4 +393,100 @@ describe('useObservable', () => {
391393
window.removeEventListener('error', onError);
392394
});
393395
});
396+
397+
describe('Server rendering', () => {
398+
// Renders `status` and `data` so assertions read the snapshot React actually used.
399+
const Probe = ({ observableId, observable$, config }: { observableId: string; observable$: Subject<any>; config?: ReactFireOptions }) => {
400+
const { status, data } = useObservable(observableId, observable$, { suspense: false, ...config });
401+
// One interpolated child: adjacent JSX text nodes render with `<!-- -->` between them.
402+
return <div>{`${status}:${String(data)}`}</div>;
403+
};
404+
405+
it('renders on the server instead of throwing', () => {
406+
const observable$: Subject<any> = new Subject();
407+
408+
// The #748 regression test: delete the third argument to useSyncExternalStore and
409+
// this fails with "Missing getServerSnapshot".
410+
expect(() => renderToString(<Probe observableId="ssr-renders" observable$={observable$} />)).not.toThrow();
411+
});
412+
413+
// The App Router streams rather than calling renderToString, and streaming surfaces
414+
// failures the synchronous renderer does not, so the fix is checked against both.
415+
it('renders on the server under the streaming renderer', async () => {
416+
const observable$: Subject<any> = new Subject();
417+
let error: unknown;
418+
419+
const html = await new Promise<string>((resolve, reject) => {
420+
const chunks: string[] = [];
421+
const sink = new Writable({
422+
write(chunk, _encoding, callback) {
423+
chunks.push(chunk.toString());
424+
callback();
425+
}
426+
});
427+
sink.on('finish', () => resolve(chunks.join('')));
428+
sink.on('error', reject);
429+
430+
const stream = renderToPipeableStream(<Probe observableId="ssr-streaming" observable$={observable$} />, {
431+
onError(caughtError) {
432+
error = caughtError;
433+
},
434+
onAllReady() {
435+
stream.pipe(sink);
436+
}
437+
});
438+
});
439+
440+
expect(error).toBeUndefined();
441+
expect(html).toContain('loading:undefined');
442+
});
443+
444+
it('reports loading on the server when there is no initialData', () => {
445+
const observable$: Subject<any> = new Subject();
446+
447+
const html = renderToString(<Probe observableId="ssr-loading" observable$={observable$} />);
448+
449+
expect(html).toContain('loading:undefined');
450+
});
451+
452+
it('reports initialData on the server when it is provided', () => {
453+
const observable$: Subject<any> = new Subject();
454+
455+
const html = renderToString(<Probe observableId="ssr-initial-data" observable$={observable$} config={{ initialData: 'seeded' }} />);
456+
457+
expect(html).toContain('success:seeded');
458+
});
459+
460+
it('does not leak a cached value from another request into the server snapshot', async () => {
461+
// `preloadedObservables` is on `globalThis`, keyed only by observableId, so concurrent
462+
// server requests share it. The first render below stands in for an earlier request.
463+
const observable$: Subject<any> = new Subject();
464+
const observableId = 'ssr-no-cross-request-leak';
465+
466+
const { result } = renderHook(() => useObservable(observableId, observable$, { suspense: false }));
467+
act(() => observable$.next('first-request-secret'));
468+
await waitFor(() => expect(result.current.data).toEqual('first-request-secret'));
469+
470+
const html = renderToString(<Probe observableId={observableId} observable$={observable$} />);
471+
472+
expect(html).not.toContain('first-request-secret');
473+
expect(html).toContain('loading:undefined');
474+
});
475+
476+
it('prefers the callers initialData over a value already in the shared cache', async () => {
477+
// The `initialData` branch is only reachable when the cache already holds a value for
478+
// this id; otherwise `useObservable`'s overlay decides and the snapshot never does.
479+
const observable$: Subject<any> = new Subject();
480+
const observableId = 'ssr-initial-data-beats-cache';
481+
482+
const { result } = renderHook(() => useObservable(observableId, observable$, { suspense: false }));
483+
act(() => observable$.next('another-requests-value'));
484+
await waitFor(() => expect(result.current.data).toEqual('another-requests-value'));
485+
486+
const html = renderToString(<Probe observableId={observableId} observable$={observable$} config={{ initialData: 'my-own-data' }} />);
487+
488+
expect(html).toContain('success:my-own-data');
489+
expect(html).not.toContain('another-requests-value');
490+
});
491+
});
394492
});

0 commit comments

Comments
 (0)