Skip to content

Commit

Permalink
Merge pull request FirebaseExtended#79 from FirebaseExtended/de-empty…
Browse files Browse the repository at this point in the history 
…-vals

Fix initial empty query bug
  • Loading branch information
@davideast
davideast committed Aug 26, 2023
2 parents aadbc39 + ddc932c commit fe53e8a
Show file tree
Hide file tree
Showing 7 changed files with 1,999 additions and 2,686 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ jobs:
strategy:
matrix:
node: ["20"]
firebase: ["9"]
firebase: ["9", "10"]
rxjs: ["6", "7"]
fail-fast: false
name: Test firebase@${{ matrix.firebase }} rxjs@${{ matrix.rxjs }} on Node.js ${{ matrix.node }}
Expand Down Expand Up @@ -126,7 +126,7 @@ jobs:
strategy:
matrix:
node: ["20"]
firebase: ["9"]
firebase: ["9", "10"]
rxjs: ["7"]
fail-fast: false
name: Lint
Expand Down
12 changes: 5 additions & 7 deletions database/list/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,11 @@ export function list(
query: Query,
options: {
events?: ListenEvent[]
}={},
} = {},
): Observable<QueryChange[]> {
const events = validateEventsArray(options.events);
return get(query).pipe(
switchMap((change) => {
if (!change.snapshot.exists()) {
return of([]);
}
const childEvent$ = [of(change)];
events.forEach((event) => {
childEvent$.push(fromRef(query, event));
Expand All @@ -73,9 +70,9 @@ export function list(
export function listVal<T>(
query: Query,
options: {
keyField?: string,
}={},
): Observable<T[] | null> {
keyField?: string,
} = {},
): Observable<T[]> {
return list(query).pipe(
map((arr) => {
return arr.map((change) => changeToData(change, options) as T);
Expand Down Expand Up @@ -111,6 +108,7 @@ function buildView(current: QueryChange[], change: QueryChange): QueryChange[] {
const {key} = snapshot;
const currentKeyPosition = positionFor(current, key);
const afterPreviousKeyPosition = positionAfter(current, prevKey || undefined);

switch (event) {
case ListenEvent.value:
if (change.snapshot && change.snapshot.exists()) {
Expand Down
17 changes: 10 additions & 7 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,14 @@
"build:docs": "cp README.md ./dist/ && cp -r ./docs ./dist/",
"dev": "rollup -c -w",
"echo:chrome": "echo 'Open Chrome DevTools: \nchrome://inspect/#devices'",
"test": "firebase emulators:exec \"jest --detectOpenHandles\" --project=rxfire-test-c497c ",
"test:debug": "yarn echo:chrome && firebase emulators:exec ./test-debug.sh --project=rxfire-test-c497c"
"test": "firebase emulators:exec \"jest --detectOpenHandles\" --project=rxfire-test-c497c",
"test:debug": "yarn echo:chrome && firebase emulators:exec ./test-debug.sh --project=rxfire-test-c497c",
"emulators": "firebase emulators:start --project=rxfire-test-c497c",
"jest": "jest --detectOpenHandles"
},
"dependencies": {},
"peerDependencies": {
"firebase": "^9.0.0",
"firebase": "^10.0.0",
"rxjs": "^6.0.0 || ^7.0.0"
},
"devDependencies": {
Expand All @@ -88,17 +90,18 @@
"@rollup/plugin-commonjs": "^15.1.0",
"@rollup/plugin-node-resolve": "^9.0.0",
"@rollup/plugin-typescript": "^8.1.0",
"@types/jest": "^26.0.20",
"@types/jest": "^29.5.4",
"@typescript-eslint/eslint-plugin": "^4.13.0",
"@typescript-eslint/parser": "^4.13.0",
"babel-jest": "^26.6.3",
"babel-jest": "^29.6.4",
"cross-fetch": "^3.1.4",
"eslint": "^7.32.0",
"eslint-config-google": "^0.14.0",
"firebase": "^10.0.0",
"firebase-tools": "^12.4.3",
"firebase-tools": "^12.5.2",
"glob": "^7.1.6",
"jest": "^26.6.3",
"jest": "^29.6.4",
"jest-environment-jsdom": "^29.6.4",
"md5": "^2.3.0",
"npm-run-all": "^4.1.5",
"rollup": "^2.33.2",
Expand Down
18 changes: 17 additions & 1 deletion test/database.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ import {
import {take, skip, switchMap} from 'rxjs/operators';
import {BehaviorSubject, Observable} from 'rxjs';
import {default as TEST_PROJECT, databaseEmulatorPort} from './config';

const rando = (): string => Math.random().toString(36).substring(5);

const batch = (
Expand Down Expand Up @@ -591,6 +590,23 @@ describe('RxFire Database', () => {
.add(done);
});

it('should handle empty sets after items are added', (done) => {
const aref = builtRef(rando());
let count = 0;
const sub = listVal(aref)
.subscribe((data) => {
if (count == 0) {
expect(data).toEqual([]);
push(aref, {name: 'one'});
count = count + 1;
} else {
expect(data.length).toEqual(1);
sub.unsubscribe();
done();
}
});
});

/**
* This test checks that dynamic querying works even with results that
* are empty.
Expand Down
25 changes: 9 additions & 16 deletions test/firestore-lite.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,13 @@ describe('RxFire firestore/lite', () => {
* creats an observable with the `collection()` method and
* asserts that the two "people" are in the array.
*/
it('should emit snapshots', async (done: jest.DoneCallback) => {
it('should emit snapshots', async () => {
const {colRef, expectedNames} = await seedTest(firestore);

collection(colRef)
.pipe(map((docs) => docs.map((doc) => doc.data().name)))
.subscribe((names) => {
expect(names).toEqual(expectedNames);
done();
});
});
});
Expand All @@ -118,7 +117,7 @@ describe('RxFire firestore/lite', () => {
* creats an observable with the `collection()` method and
* asserts that the two "people" are in the array.
*/
it('should emit snapshots', async (done: jest.DoneCallback) => {
it('should emit snapshots', async () => {
const {colRef} = await seedTest(firestore);

class Folk {
Expand All @@ -143,7 +142,6 @@ describe('RxFire firestore/lite', () => {
const classes = docs.map((doc) => doc.data()?.constructor?.name);
expect(names).toEqual(['David!', undefined]);
expect(classes).toEqual(['Folk', undefined]);
done();
});
});
});
Expand All @@ -152,7 +150,7 @@ describe('RxFire firestore/lite', () => {
/**
* The `unwrap(id)` method will map a collection to its data payload and map the doc ID to a the specificed key.
*/
it('collectionData should map a QueryDocumentSnapshot[] to an array of plain objects', async (done: jest.DoneCallback) => {
it('collectionData should map a QueryDocumentSnapshot[] to an array of plain objects', async () => {
const {colRef} = await seedTest(firestore);

// const unwrapped = collection(colRef).pipe(unwrap('userId'));
Expand All @@ -165,11 +163,10 @@ describe('RxFire firestore/lite', () => {
};
expect(val).toBeInstanceOf(Array);
expect(val[0]).toEqual(expectedDoc);
done();
});
});

it('docData should map a QueryDocumentSnapshot to a plain object', async (done: jest.DoneCallback) => {
it('docData should map a QueryDocumentSnapshot to a plain object', async () => {
const {davidDoc} = await seedTest(firestore);

// const unwrapped = doc(davidDoc).pipe(unwrap('UID'));
Expand All @@ -181,7 +178,6 @@ describe('RxFire firestore/lite', () => {
UID: 'david',
};
expect(val).toEqual(expectedDoc);
done();
});
});

Expand All @@ -191,8 +187,8 @@ describe('RxFire firestore/lite', () => {
* FIRESTORE (8.5.0) INTERNAL ASSERTION FAILED: Unexpected state
*/

it('docData matches the result of docSnapShot.data() when the document doesn\'t exist', async (done) => {
pending('Not working against the emulator');
it('docData matches the result of docSnapShot.data() when the document doesn\'t exist', async () => {
// pending('Not working against the emulator');

const {colRef} = await seedTest(firestore);

Expand All @@ -205,13 +201,12 @@ describe('RxFire firestore/lite', () => {
getDoc(nonExistentDoc).then((snap) => {
unwrapped.subscribe((val) => {
expect(val).toEqual(snap.data());
done();
});
});
});

it('collectionData matches the result of querySnapShot.docs when the collection doesn\'t exist', (done) => {
pending('Not working against the emulator');
// pending('Not working against the emulator');

const nonExistentCollection = firestoreCollection(firestore, createId());

Expand All @@ -227,7 +222,7 @@ describe('RxFire firestore/lite', () => {
});

describe('Aggregations', () => {
it('should provide an observable with a count aggregate', async (done) => {
it('should provide an observable with a count aggregate', async () => {
const colRef = createRandomCol(firestore);
const entries = [
addDoc(colRef, {id: createId()}),
Expand All @@ -237,11 +232,10 @@ describe('RxFire firestore/lite', () => {

collectionCountSnap(colRef).subscribe((snap) => {
expect(snap.data().count).toEqual(entries.length);
done();
});
});

it('should provide an observable with a count aggregate number', async (done) => {
it('should provide an observable with a count aggregate number', async () => {
const colRef = createRandomCol(firestore);
const entries = [
addDoc(colRef, {id: createId()}),
Expand All @@ -254,7 +248,6 @@ describe('RxFire firestore/lite', () => {

collectionCount(colRef).subscribe((count) => {
expect(count).toEqual(entries.length);
done();
});
});
});
Expand Down
10 changes: 4 additions & 6 deletions test/firestore.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ describe('RxFire Firestore', () => {
*/

it('docData matches the result of docSnapShot.data() when the document doesn\'t exist', (done) => {
pending('Not working against the emulator');
// pending('Not working against the emulator');

const {colRef} = seedTest(firestore);

Expand All @@ -410,7 +410,7 @@ describe('RxFire Firestore', () => {
});

it('collectionData matches the result of querySnapShot.docs when the collection doesn\'t exist', (done) => {
pending('Not working against the emulator');
// pending('Not working against the emulator');

const nonExistentCollection = firestoreCollection(firestore, createId());

Expand All @@ -426,7 +426,7 @@ describe('RxFire Firestore', () => {
});

describe('Aggregations', () => {
it('should provide an observable with a count aggregate snapshot', async (done) => {
it('should provide an observable with a count aggregate snapshot', async () => {
const colRef = createRandomCol(firestore);
const entries = [
addDoc(colRef, {id: createId()}),
Expand All @@ -436,11 +436,10 @@ describe('RxFire Firestore', () => {

collectionCountSnap(colRef).subscribe((snap) => {
expect(snap.data().count).toEqual(entries.length);
done();
});
});

it('should provide an observable with a count aggregate number', async (done) => {
it('should provide an observable with a count aggregate number', async () => {
const colRef = createRandomCol(firestore);
const entries = [
addDoc(colRef, {id: createId()}),
Expand All @@ -453,7 +452,6 @@ describe('RxFire Firestore', () => {

collectionCount(colRef).subscribe((count) => {
expect(count).toEqual(entries.length);
done();
});
});
});
Expand Down
Loading

0 comments on commit fe53e8a

Please sign in to comment.