Skip to content

Commit 1b87ea1

Browse files
tyler-reitztyler-reitz
authored
fix: remove NO_ID_FIELD sentinel from data hooks when idField is not provided (#738)
When no options (or no idField/keyField) are passed, the Firestore and RTDB data hooks no longer inject the NO_ID_FIELD sentinel into returned objects, so hooks return exactly the stored data. Also unifies the omitted-options vs {} behavior and preserves data-converter class prototypes on the no-idField path. Adds regression tests covering the no-options path for all five hooks.
1 parent f6c6862 commit 1b87ea1

4 files changed

Lines changed: 70 additions & 11 deletions

File tree

‎src/database.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ export function useDatabaseObject<T = unknown>(ref: DatabaseReference, options?:
3232
}
3333

3434
export function useDatabaseObjectData<T>(ref: DatabaseReference, options?: ReactFireOptions<T>): ObservableStatus<T> {
35-
const idField = options ? checkIdField(options) : 'NO_ID_FIELD';
36-
const observableId = `database:objectVal:${ref.toString()}:idField=${idField}`;
35+
const idField = options ? checkIdField(options) : undefined;
36+
const observableId = `database:objectVal:${ref.toString()}:idField=${JSON.stringify(idField)}`;
3737
const observable$ = objectVal<T>(ref, { keyField: idField });
3838

3939
return useObservable(observableId, observable$, options);
@@ -59,8 +59,8 @@ export function useDatabaseListData<T = { [key: string]: unknown }>(
5959
ref: DatabaseReference | DatabaseQuery,
6060
options?: ReactFireOptions<T[]>
6161
): ObservableStatus<T[] | null> {
62-
const idField = options ? checkIdField(options) : 'NO_ID_FIELD';
63-
const observableId = `database:listVal:${getUniqueIdForDatabaseQuery(ref)}:idField=${idField}`;
62+
const idField = options ? checkIdField(options) : undefined;
63+
const observableId = `database:listVal:${getUniqueIdForDatabaseQuery(ref)}:idField=${JSON.stringify(idField)}`;
6464
const observable$ = listVal<T>(ref, { keyField: idField });
6565
return useObservable(observableId, observable$, options);
6666
}

‎src/firestore.tsx‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,9 @@ export function useFirestoreDocOnce<T = DocumentData>(ref: DocumentReference<T>,
6060
* Subscribe to Firestore Document changes and unwrap the document into a plain object
6161
*/
6262
export function useFirestoreDocData<T = unknown>(ref: DocumentReference<T>, options?: ReactFireOptions<T>): ObservableStatus<T | undefined> {
63-
const idField = options ? checkIdField(options) : 'NO_ID_FIELD';
63+
const idField = options ? checkIdField(options) : undefined;
6464

65-
const observableId = `firestore:docData:${ref.firestore.app.name}:${ref.path}:idField=${idField}`;
65+
const observableId = `firestore:docData:${ref.firestore.app.name}:${ref.path}:idField=${JSON.stringify(idField)}`;
6666
const observable = docData(ref, { idField });
6767

6868
return useObservable(observableId, observable, options) as ObservableStatus<T>;
@@ -72,9 +72,9 @@ export function useFirestoreDocData<T = unknown>(ref: DocumentReference<T>, opti
7272
* Get a Firestore document, unwrap the document into a plain object, and don't subscribe to changes
7373
*/
7474
export function useFirestoreDocDataOnce<T = unknown>(ref: DocumentReference<T>, options?: ReactFireOptions<T>): ObservableStatus<T | undefined> {
75-
const idField = options ? checkIdField(options) : 'NO_ID_FIELD';
75+
const idField = options ? checkIdField(options) : undefined;
7676

77-
const observableId = `firestore:docDataOnce:${ref.firestore.app.name}:${ref.path}:idField=${idField}`;
77+
const observableId = `firestore:docDataOnce:${ref.firestore.app.name}:${ref.path}:idField=${JSON.stringify(idField)}`;
7878
const observable$ = docData(ref, { idField }).pipe(first());
7979

8080
return useObservable(observableId, observable$, options) as ObservableStatus<T>;
@@ -94,8 +94,8 @@ export function useFirestoreCollection<T = DocumentData>(query: FirestoreQuery<T
9494
* Subscribe to a Firestore collection and unwrap the snapshot into an array.
9595
*/
9696
export function useFirestoreCollectionData<T = DocumentData>(query: FirestoreQuery<T>, options?: ReactFireOptions<T[]>): ObservableStatus<T[]> {
97-
const idField = options ? checkIdField(options) : 'NO_ID_FIELD';
98-
const observableId = `firestore:collectionData:${getUniqueIdForFirestoreQuery(query)}:idField=${idField}`;
97+
const idField = options ? checkIdField(options) : undefined;
98+
const observableId = `firestore:collectionData:${getUniqueIdForFirestoreQuery(query)}:idField=${JSON.stringify(idField)}`;
9999
const observable$ = collectionData(query, { idField });
100100

101101
return useObservable(observableId, observable$, options);

‎test/database.test.tsx‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { renderHook, act, cleanup, waitFor } from '@testing-library/react';
22
import * as React from 'react';
3-
import { useDatabaseObject, useDatabaseObjectData, useDatabaseList, FirebaseAppProvider, DatabaseProvider } from '../src/index';
3+
import { useDatabaseObject, useDatabaseList, useDatabaseObjectData, useDatabaseListData, FirebaseAppProvider, DatabaseProvider } from '../src/index';
44
import { baseConfig } from './appConfig';
55
import { initializeApp } from 'firebase/app';
66
import { getDatabase, connectDatabaseEmulator, ref, set, push, query, orderByChild, equalTo, get } from 'firebase/database';
@@ -132,4 +132,30 @@ describe('Realtime Database (RTDB)', () => {
132132
expect(unfilteredResult.current.data.length).toBeGreaterThan(filteredResult.current.data.length);
133133
});
134134
});
135+
136+
describe('useDatabaseObjectData', () => {
137+
it('returns exactly the stored data when no options are provided', async () => {
138+
const mockData = { a: 'hello' };
139+
const objectRef = ref(database, randomString());
140+
await set(objectRef, mockData);
141+
142+
const { result } = renderHook(() => useDatabaseObjectData(objectRef), { wrapper: Provider });
143+
await waitFor(() => expect(result.current.status).toEqual('success'));
144+
145+
expect(result.current.data).toEqual(mockData);
146+
});
147+
});
148+
149+
describe('useDatabaseListData', () => {
150+
it('returns exactly the stored data when no options are provided', async () => {
151+
const mockData = { b: 'world' };
152+
const listRef = ref(database, randomString());
153+
await push(listRef, mockData);
154+
155+
const { result } = renderHook(() => useDatabaseListData(listRef), { wrapper: Provider });
156+
await waitFor(() => expect(result.current.status).toEqual('success'));
157+
158+
expect(result.current.data).toEqual([mockData]);
159+
});
160+
});
135161
});

‎test/firestore.test.tsx‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,17 @@ describe('Firestore', () => {
157157
expect(result.current.data.a).toEqual(otherMockData.a);
158158
});
159159
});
160+
161+
it('returns exactly the stored data when no options are provided', async () => {
162+
const mockData = { a: 'hello' };
163+
const ref = doc(collection(db, randomString()), randomString());
164+
await setDoc(ref, mockData);
165+
166+
const { result } = renderHook(() => useFirestoreDocData(ref), { wrapper: Provider });
167+
await waitFor(() => expect(result.current.status).toEqual('success'));
168+
169+
expect(result.current.data).toEqual(mockData);
170+
});
160171
});
161172

162173
describe('useFirestoreDocOnce', () => {
@@ -203,6 +214,17 @@ describe('Firestore', () => {
203214
expect(onceResult.current.data.a).toEqual(mockData1.a);
204215
expect(subscribeResult.current.data).not.toEqual(onceResult.current.data);
205216
});
217+
218+
it('returns exactly the stored data when no options are provided', async () => {
219+
const mockData = { a: 'hello' };
220+
const ref = doc(collection(db, randomString()), randomString());
221+
await setDoc(ref, mockData);
222+
223+
const { result } = renderHook(() => useFirestoreDocDataOnce(ref), { wrapper: Provider });
224+
await waitFor(() => expect(result.current.status).toEqual('success'));
225+
226+
expect(result.current.data).toEqual(mockData);
227+
});
206228
});
207229

208230
describe('useFirestoreCollection', () => {
@@ -300,5 +322,16 @@ describe('Firestore', () => {
300322
// the full list should be bigger than the filtered list
301323
expect(list.length).toBeGreaterThan(filteredList.length);
302324
});
325+
326+
it('returns exactly the stored data when no options are provided', async () => {
327+
const mockData = { a: 'hello' };
328+
const ref = collection(db, randomString());
329+
await addDoc(ref, mockData);
330+
331+
const { result } = renderHook(() => useFirestoreCollectionData(ref), { wrapper: Provider });
332+
await waitFor(() => expect(result.current.status).toEqual('success'));
333+
334+
expect(result.current.data).toEqual([mockData]);
335+
});
303336
});
304337
});

0 commit comments

Comments
 (0)