diff --git a/.gitignore b/.gitignore index df34806e..b2ebc657 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ +CLAUDE.local* .DS_Store npm-debug.log diff --git a/docs/reference/functions/StorageImage.md b/docs/reference/functions/StorageImage.md index 24370e02..ea871ad4 100644 --- a/docs/reference/functions/StorageImage.md +++ b/docs/reference/functions/StorageImage.md @@ -8,7 +8,7 @@ > **StorageImage**(`props`): `Element` -Defined in: [src/storage.tsx:78](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L78) +Defined in: [src/storage.tsx:79](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L79) ## Parameters diff --git a/docs/reference/functions/useObservable.md b/docs/reference/functions/useObservable.md index e0e6db0c..996ab327 100644 --- a/docs/reference/functions/useObservable.md +++ b/docs/reference/functions/useObservable.md @@ -8,7 +8,17 @@ > **useObservable**\<`T`\>(`observableId`, `source`, `config?`): [`ObservableStatus`](../type-aliases/ObservableStatus.md)\<`T`\> -Defined in: [src/useObservable.ts:86](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L86) +Defined in: [src/useObservable.ts:96](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L96) + +Subscribe to an Observable and return its current status. + +Error handling depends on the suspense mode: +- Non-suspense mode (default): errors are returned as `{ status: 'error', error }` so the + component can handle them locally without needing a React Error Boundary. +- Suspense mode (`suspense: true`): errors are re-thrown so a React Error Boundary can catch them. + +If the observable emits a value and then errors, `data` retains the last emitted value and +`status` changes to `'error'`. There is no automatic retry path once an error occurs. ## Type Parameters diff --git a/docs/reference/functions/useStorageDownloadURL.md b/docs/reference/functions/useStorageDownloadURL.md index 2ab6fb3d..725952be 100644 --- a/docs/reference/functions/useStorageDownloadURL.md +++ b/docs/reference/functions/useStorageDownloadURL.md @@ -8,7 +8,7 @@ > **useStorageDownloadURL**\<`T`\>(`ref`, `options?`): [`ObservableStatus`](../type-aliases/ObservableStatus.md)\<`string` \| `T`\> -Defined in: [src/storage.tsx:29](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L29) +Defined in: [src/storage.tsx:30](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L30) Subscribe to a storage ref's download URL diff --git a/docs/reference/functions/useStorageTask.md b/docs/reference/functions/useStorageTask.md index 2d462c71..99e494dc 100644 --- a/docs/reference/functions/useStorageTask.md +++ b/docs/reference/functions/useStorageTask.md @@ -8,7 +8,7 @@ > **useStorageTask**\<`T`\>(`task`, `ref`, `options?`): [`ObservableStatus`](../type-aliases/ObservableStatus.md)\<`T` \| `UploadTaskSnapshot`\> -Defined in: [src/storage.tsx:16](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L16) +Defined in: [src/storage.tsx:17](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L17) Subscribe to the progress of a storage task diff --git a/docs/reference/interfaces/ObservableStatusError.md b/docs/reference/interfaces/ObservableStatusError.md index c6293b1d..87977b77 100644 --- a/docs/reference/interfaces/ObservableStatusError.md +++ b/docs/reference/interfaces/ObservableStatusError.md @@ -40,7 +40,7 @@ If `initialData` is passed in, the first value of `data` will be the valuea prov > **error**: `Error` -Defined in: [src/useObservable.ts:75](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L75) +Defined in: [src/useObservable.ts:74](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L74) Any error that may have occurred in the underlying observable @@ -82,13 +82,13 @@ If `initialData` is passed in, this will be `true`. ### isComplete -> **isComplete**: `true` +> **isComplete**: `boolean` -Defined in: [src/useObservable.ts:74](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L74) +Defined in: [src/useObservable.ts:50](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L50) If this is `true`, the hook will be emitting no further items. -#### Overrides +#### Inherited from `ObservableStatusBase.isComplete` diff --git a/docs/reference/interfaces/ObservableStatusLoading.md b/docs/reference/interfaces/ObservableStatusLoading.md index f0bf2fcd..4744410c 100644 --- a/docs/reference/interfaces/ObservableStatusLoading.md +++ b/docs/reference/interfaces/ObservableStatusLoading.md @@ -6,7 +6,7 @@ # Interface: ObservableStatusLoading\ -Defined in: [src/useObservable.ts:78](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L78) +Defined in: [src/useObservable.ts:77](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L77) ## Extends @@ -24,7 +24,7 @@ Defined in: [src/useObservable.ts:78](https://github.com/FirebaseExtended/reactf > **data**: `undefined` -Defined in: [src/useObservable.ts:80](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L80) +Defined in: [src/useObservable.ts:79](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L79) The most recent value. @@ -68,7 +68,7 @@ Promise that resolves after first emit from observable > **hasEmitted**: `false` -Defined in: [src/useObservable.ts:81](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L81) +Defined in: [src/useObservable.ts:80](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L80) Indicates whether the hook has emitted a value at some point @@ -98,7 +98,7 @@ If this is `true`, the hook will be emitting no further items. > **status**: `"loading"` -Defined in: [src/useObservable.ts:79](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L79) +Defined in: [src/useObservable.ts:78](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L78) The loading status. diff --git a/docs/reference/type-aliases/ObservableStatus.md b/docs/reference/type-aliases/ObservableStatus.md index bbf0276d..013ad25b 100644 --- a/docs/reference/type-aliases/ObservableStatus.md +++ b/docs/reference/type-aliases/ObservableStatus.md @@ -8,7 +8,7 @@ > **ObservableStatus**\<`T`\> = [`ObservableStatusLoading`](../interfaces/ObservableStatusLoading.md)\<`T`\> \| [`ObservableStatusError`](../interfaces/ObservableStatusError.md)\<`T`\> \| [`ObservableStatusSuccess`](../interfaces/ObservableStatusSuccess.md)\<`T`\> -Defined in: [src/useObservable.ts:84](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L84) +Defined in: [src/useObservable.ts:83](https://github.com/FirebaseExtended/reactfire/blob/main/src/useObservable.ts#L83) ## Type Parameters diff --git a/docs/reference/type-aliases/StorageImageProps.md b/docs/reference/type-aliases/StorageImageProps.md index b75fa337..f1206d9b 100644 --- a/docs/reference/type-aliases/StorageImageProps.md +++ b/docs/reference/type-aliases/StorageImageProps.md @@ -8,7 +8,7 @@ > **StorageImageProps** = `object` -Defined in: [src/storage.tsx:36](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L36) +Defined in: [src/storage.tsx:37](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L37) ## Properties @@ -16,7 +16,7 @@ Defined in: [src/storage.tsx:36](https://github.com/FirebaseExtended/reactfire/b > `optional` **placeHolder?**: `React.ReactNode` -Defined in: [src/storage.tsx:40](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L40) +Defined in: [src/storage.tsx:41](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L41) *** @@ -24,7 +24,7 @@ Defined in: [src/storage.tsx:40](https://github.com/FirebaseExtended/reactfire/b > `optional` **storage?**: `FirebaseStorage` -Defined in: [src/storage.tsx:38](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L38) +Defined in: [src/storage.tsx:39](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L39) *** @@ -32,7 +32,7 @@ Defined in: [src/storage.tsx:38](https://github.com/FirebaseExtended/reactfire/b > **storagePath**: `string` -Defined in: [src/storage.tsx:37](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L37) +Defined in: [src/storage.tsx:38](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L38) *** @@ -40,4 +40,4 @@ Defined in: [src/storage.tsx:37](https://github.com/FirebaseExtended/reactfire/b > `optional` **suspense?**: `boolean` -Defined in: [src/storage.tsx:39](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L39) +Defined in: [src/storage.tsx:40](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L40) diff --git a/docs/upgrade-guide.md b/docs/upgrade-guide.md index b5277dd9..9aa85b8b 100644 --- a/docs/upgrade-guide.md +++ b/docs/upgrade-guide.md @@ -1,3 +1,27 @@ +# Upgrade from ReactFire v4.2 to v4.3 + +ReactFire v4.3.0 changes how errors are surfaced in non-suspense mode (the default). + +## Error handling behavior change + +Previously, errors from any reactfire hook were thrown unconditionally, making `status: 'error'` unreachable in practice. In v4.3.0, error handling depends on the mode: + +- **Non-suspense mode** (default, or `suspense: false`): errors are returned via `status: 'error'` so components can handle them locally. +- **Suspense mode** (`suspense: true`): errors are re-thrown so a React Error Boundary can catch them. No change from prior behavior. + +**If you rely on a React Error Boundary to catch Firebase errors in non-suspense mode**, you must add an explicit re-throw in your component: + +```tsx +const { status, error } = useStorageDownloadURL(ref); +if (status === 'error') throw error; // re-throw to reach your Error Boundary +``` + +**If you already check `status` before using `data`**, no change is needed. + +Note: once an observable errors there is no automatic retry. The errored observable remains in the global cache under its `observableId`, so unmounting and remounting the same component rejoins the same errored state. Today the only workaround is to change the `observableId`. A proper retry mechanism is tracked as a follow-up issue. + +--- + # Upgrade from ReactFire v3 to v4 As announced in [Discussion 402](https://github.com/FirebaseExtended/reactfire/discussions/402), ReactFire v4 contains breaking changes. This guide details how to upgrade from v3 to v4. diff --git a/docs/use.md b/docs/use.md index 5686a368..404cc625 100644 --- a/docs/use.md +++ b/docs/use.md @@ -426,12 +426,16 @@ function CatImage() { const storage = useStorage(); const catRef = ref(storage, 'cats/newspaper'); - const { status, data: imageURL } = useStorageDownloadURL(catRef); + const { status, data: imageURL, error } = useStorageDownloadURL(catRef); if (status === 'loading') { return loading...; } + if (status === 'error') { + return Error: {error.message}; + } + return cat reading the newspaper; } ``` diff --git a/package.json b/package.json index ec5a2f00..01f7698c 100644 --- a/package.json +++ b/package.json @@ -1,5 +1,5 @@ { - "version": "4.2.3", + "version": "4.3.0", "license": "MIT", "type": "module", "main": "dist/index.umd.cjs", diff --git a/src/storage.tsx b/src/storage.tsx index 5a0299da..ea4e483f 100644 --- a/src/storage.tsx +++ b/src/storage.tsx @@ -1,5 +1,6 @@ import * as React from 'react'; import { getDownloadURL, fromTask } from 'rxfire/storage'; +import { defer } from 'rxjs'; import { ReactFireOptions, useObservable, ObservableStatus, useStorage } from './'; import { useSuspenseEnabledFromConfigAndContext } from './firebaseApp'; import { ref } from 'firebase/storage'; @@ -28,7 +29,7 @@ export function useStorageTask(task: UploadTask, ref: StorageRefere */ export function useStorageDownloadURL(ref: StorageReference, options?: ReactFireOptions): ObservableStatus { const observableId = `storage:downloadUrl:${ref.toString()}`; - const observable$ = getDownloadURL(ref); + const observable$ = defer(() => getDownloadURL(ref)); return useObservable(observableId, observable$, options); } diff --git a/src/useObservable.ts b/src/useObservable.ts index fe05be5d..f98a2e1b 100644 --- a/src/useObservable.ts +++ b/src/useObservable.ts @@ -71,7 +71,6 @@ export interface ObservableStatusSuccess extends ObservableStatusBase { export interface ObservableStatusError extends ObservableStatusBase { status: 'error'; - isComplete: true; error: Error; } @@ -83,6 +82,17 @@ export interface ObservableStatusLoading extends ObservableStatusBase { export type ObservableStatus = ObservableStatusLoading | ObservableStatusError | ObservableStatusSuccess; +/** + * Subscribe to an Observable and return its current status. + * + * Error handling depends on the suspense mode: + * - Non-suspense mode (default): errors are returned as `{ status: 'error', error }` so the + * component can handle them locally without needing a React Error Boundary. + * - Suspense mode (`suspense: true`): errors are re-thrown so a React Error Boundary can catch them. + * + * If the observable emits a value and then errors, `data` retains the last emitted value and + * `status` changes to `'error'`. There is no automatic retry path once an error occurs. + */ export function useObservable(observableId: string, source: Observable, config: ReactFireOptions = {}): ObservableStatus { if (!observableId) { throw new Error('cannot call useObservable without an observableId'); @@ -105,9 +115,8 @@ export function useObservable(observableId: string, source: Observa next: () => { onStoreChange(); }, - error: (e) => { + error: () => { onStoreChange(); - throw e; }, complete: () => { onStoreChange(); @@ -145,9 +154,9 @@ export function useObservable(observableId: string, source: Observa } as ObservableStatus; } - // throw an error if there is an error - // TODO(jhuleatt) this is the current, tested-for, behavior. But do we actually want it? - if (update.error) { + // In suspense mode, throw errors so React Error Boundaries can catch them. + // In non-suspense mode, surface errors via status so consumers can handle them locally. + if (suspenseEnabled && update.error) { throw update.error; } diff --git a/test/storage.test.tsx b/test/storage.test.tsx index bfea2297..ee331529 100644 --- a/test/storage.test.tsx +++ b/test/storage.test.tsx @@ -58,6 +58,13 @@ describe('Storage', () => { }); describe('useStorageDownloadURL', () => { + it('surfaces storage/object-not-found as status: error for a nonexistent file', async () => { + const missingRef = ref(storage, `nonexistent/${randomString()}.txt`); + const { result } = renderHook(() => useStorageDownloadURL(missingRef), { wrapper: Provider }); + await waitFor(() => expect(result.current.status).toEqual('error')); + expect((result.current.error as any)?.code).toEqual('storage/object-not-found'); + }); + it('returns the same value as getDownloadURL', async () => { const someBytes = Uint8Array.from(Buffer.from(new ArrayBuffer(500_000))); const testFileRef = ref(storage, `${randomString()}/${randomString()}.txt`); diff --git a/test/useObservable.test.tsx b/test/useObservable.test.tsx index f16d327a..b841e428 100644 --- a/test/useObservable.test.tsx +++ b/test/useObservable.test.tsx @@ -2,7 +2,9 @@ import '@testing-library/jest-dom/extend-expect'; import { act, cleanup, render, renderHook, waitFor } from '@testing-library/react'; import * as React from 'react'; import { of, Subject, BehaviorSubject, throwError } from 'rxjs'; -import { useObservable } from '../src/index'; +import { useObservable, FirebaseAppProvider } from '../src/index'; +import { initializeApp } from 'firebase/app'; +import { baseConfig } from './appConfig'; describe('useObservable', () => { afterEach(cleanup); @@ -124,10 +126,46 @@ describe('useObservable', () => { act(() => observable$.next('val')); expect(result.current.isComplete).toEqual(false); - + act(() => observable$.complete()); await waitFor(() => expect(result.current.isComplete).toEqual(true)); }); + + it('surfaces errors via status in non-suspense mode', async () => { + const error = new Error('I am an error'); + const observable$ = throwError(error); + + const { result } = renderHook(() => useObservable('test-error-non-suspense', observable$, { suspense: false })); + + await waitFor(() => expect(result.current.status).toEqual('error')); + expect(result.current.error).toEqual(error); + }); + + it('surfaces errors via status when no suspense option is provided', async () => { + const error = new Error('default mode error'); + const observable$ = throwError(error); + + const { result } = renderHook(() => useObservable('test-error-default-mode', observable$)); + + await waitFor(() => expect(result.current.status).toEqual('error')); + expect(result.current.error).toEqual(error); + }); + + it('retains last emitted data when observable errors after emitting', async () => { + const subject$ = new Subject(); + const error = new Error('late error'); + + const { result } = renderHook(() => useObservable('test-late-error', subject$, { suspense: false })); + + act(() => subject$.next('good value')); + await waitFor(() => expect(result.current.status).toEqual('success')); + expect(result.current.data).toEqual('good value'); + + act(() => subject$.error(error)); + await waitFor(() => expect(result.current.status).toEqual('error')); + expect(result.current.error).toEqual(error); + expect(result.current.data).toEqual('good value'); + }); }); describe('Suspense Mode', () => { @@ -328,5 +366,23 @@ describe('useObservable', () => { // if useObservable doesn't re-emit, the value here will still be "Jeff" expect(refreshedComp).toHaveTextContent('James'); }); + it('throws an error via FirebaseAppProvider suspense context path', () => { + const spy = vi.spyOn(console, 'error'); + spy.mockImplementation(() => {}); + + const app = initializeApp(baseConfig, 'suspense-context-test'); + const error = new Error('context-path error'); + const observable$ = throwError(error); + + const wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + + expect(() => renderHook(() => useObservable('test-context-suspense-error', observable$), { wrapper })).toThrow(error); + + spy.mockRestore(); + }); }); });