Skip to content

Fixes for Bundles@exp #4355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/firestore/exp/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ export {
namedQuery
} from '../src/exp/database';

export {
LoadBundleTask,
LoadBundleTaskProgress,
TaskState
} from '../src/exp/bundle';

export { Settings, PersistenceSettings } from '../src/exp/settings';

export {
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,4 @@ export { FieldPath } from './src/api/field_path';
export { FieldValue } from './src/api/field_value';
export { Timestamp } from './src/api/timestamp';
export { FirebaseFirestore as ExpFirebaseFirestore } from './src/exp/database';
export { loadBundle, namedQuery } from './src/api/bundle';
export { loadBundle, namedQuery } from './src/exp/database';
15 changes: 11 additions & 4 deletions packages/firestore/index.bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* limitations under the License.
*/

import { Firestore, loadBundle, namedQuery } from './export';
import { Firestore, loadBundle, namedQuery, Query } from './export';

/**
* Prototype patches bundle loading to Firestore.
Expand All @@ -25,13 +25,20 @@ export function registerBundle(instance: typeof Firestore): void {
this: Firestore,
data: ArrayBuffer | ReadableStream<Uint8Array> | string
) {
return loadBundle(this, data);
return loadBundle(this._delegate, data);
};
instance.prototype.namedQuery = function (
instance.prototype.namedQuery = async function (
this: Firestore,
queryName: string
) {
return namedQuery(this, queryName);
const expQuery = await namedQuery(this._delegate, queryName);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or with then since async functions have a lot of overhead.

if (!expQuery) return null;
return new Query(
this,
// We can pass the exp-query here directly since named queries don't have UserDataConverters
// Otherwise we would have to create a new ExpQuery and pass the old UserDataConverter
expQuery
);
};
}

Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/src/api/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ import {
import { setLogLevel as setClientLogLevel } from '../util/log';

import { Blob } from './blob';
import { LoadBundleTask } from './bundle';
import { Compat } from './compat';
import {
CompleteFn,
Expand All @@ -133,6 +132,7 @@ import {
} from './observer';
import { UntypedFirestoreDataConverter } from './user_data_reader';
import { AbstractUserDataWriter } from './user_data_writer';
import { LoadBundleTask } from '../exp/bundle';

/**
* A persistence provider for either memory-only or IndexedDB persistence.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,30 @@
* limitations under the License.
*/

import {
LoadBundleTask as ApiLoadBundleTask,
LoadBundleTaskProgress
} from '@firebase/firestore-types';

import { ensureFirestoreConfigured } from '../../src/exp/database';
import { Query as ExpQuery } from '../../src/exp/reference';
import {
firestoreClientGetNamedQuery,
firestoreClientLoadBundle
} from '../core/firestore_client';
import { debugAssert } from '../util/assert';
import { FirestoreError } from '../util/error';
import { Deferred } from '../util/promise';
import { PartialObserver } from '../api/observer';

// I moved this filed to match the current pattern where api relies on exp.
// I also added all interfaces here, since we want this file to be the source
// of truth for the generated API (if you run yarn:build it should generate
// a new API report with these changes)

// Add comments
export type TaskState = 'Error' | 'Running' | 'Success';

// Add comments
export interface LoadBundleTaskProgress {
documentsLoaded: number;
totalDocuments: number;
bytesLoaded: number;
totalBytes: number;
taskState: TaskState;
}

import { Query, Firestore } from './database';
import { PartialObserver } from './observer';

export class LoadBundleTask
implements ApiLoadBundleTask, PromiseLike<LoadBundleTaskProgress> {
// Add comments
export class LoadBundleTask implements PromiseLike<LoadBundleTaskProgress> {
private _progressObserver: PartialObserver<LoadBundleTaskProgress> = {};
private _taskCompletionResolver = new Deferred<LoadBundleTaskProgress>();

Expand All @@ -46,6 +50,7 @@ export class LoadBundleTask
documentsLoaded: 0
};

// Add comments
onProgress(
next?: (progress: LoadBundleTaskProgress) => unknown,
error?: (err: Error) => unknown,
Expand All @@ -58,12 +63,14 @@ export class LoadBundleTask
};
}

// Add comments
catch<R>(
onRejected: (a: Error) => R | PromiseLike<R>
): Promise<R | LoadBundleTaskProgress> {
return this._taskCompletionResolver.promise.catch(onRejected);
}

// Add comments
then<T, R>(
onFulfilled?: (a: LoadBundleTaskProgress) => T | PromiseLike<T>,
onRejected?: (a: Error) => R | PromiseLike<R>
Expand Down Expand Up @@ -122,30 +129,3 @@ export class LoadBundleTask
}
}
}

export function loadBundle(
db: Firestore,
bundleData: ArrayBuffer | ReadableStream<Uint8Array> | string
): LoadBundleTask {
const resultTask = new LoadBundleTask();
firestoreClientLoadBundle(
ensureFirestoreConfigured(db._delegate),
db._databaseId,
bundleData,
resultTask
);
return resultTask;
}

export function namedQuery(db: Firestore, name: string): Promise<Query | null> {
return firestoreClientGetNamedQuery(
ensureFirestoreConfigured(db._delegate),
name
).then(namedQuery => {
if (!namedQuery) {
return null;
}

return new Query(db, new ExpQuery(db._delegate, null, namedQuery.query));
});
}