Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make FieldValue a Compat class #3819

Merged
merged 6 commits into from
Sep 23, 2020
Merged

Make FieldValue a Compat class #3819

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
59687d3
Add Compat layer for FieldValue
schmidt-sebastian Sep 21, 2020
090be8a
Fix equals test
schmidt-sebastian Sep 22, 2020
af589ed
Fix console build
schmidt-sebastian Sep 22, 2020
d1509b1
Lint fix
schmidt-sebastian Sep 22, 2020
adbf3f3
Merge branch 'master' into mrschmidt/compat-fieldvalue
schmidt-sebastian Sep 23, 2020
acf89c7
Feedback
schmidt-sebastian Sep 23, 2020
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
104 changes: 37 additions & 67 deletions packages/firestore/exp/test/shim.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,10 @@ import * as exp from '../index';

import {
addDoc,
arrayRemove,
arrayUnion,
clearIndexedDbPersistence,
collection,
collectionGroup,
deleteDoc,
deleteField,
disableNetwork,
doc,
DocumentReference as DocumentReferenceExp,
Expand All @@ -43,15 +40,13 @@ import {
getDocs,
getDocsFromCache,
getDocsFromServer,
increment,
initializeFirestore,
onSnapshot,
onSnapshotsInSync,
query,
queryEqual,
refEqual,
runTransaction,
serverTimestamp,
setDoc,
snapshotEqual,
terminate,
Expand All @@ -71,18 +66,20 @@ import {
import { UntypedFirestoreDataConverter } from '../../src/api/user_data_reader';
import { isPartialObserver, PartialObserver } from '../../src/api/observer';
import { isPlainObject } from '../../src/util/input_validation';
import { Compat } from '../../src/compat/compat';

export { GeoPoint, Timestamp } from '../index';
export { FieldValue } from '../../src/compat/field_value';

/* eslint-disable @typescript-eslint/no-explicit-any */

// This module defines a shim layer that implements the legacy API on top
// of the experimental SDK. This shim is used to run integration tests against
// both SDK versions.

export class FirebaseApp implements FirebaseAppLegacy {
constructor(readonly _delegate: FirebaseAppExp) {}

export class FirebaseApp
extends Compat<FirebaseAppExp>
implements FirebaseAppLegacy {
name = this._delegate.name;
options = this._delegate.options;
automaticDataCollectionEnabled = this._delegate
Expand All @@ -93,9 +90,9 @@ export class FirebaseApp implements FirebaseAppLegacy {
}
}

export class FirebaseFirestore implements legacy.FirebaseFirestore {
constructor(private readonly _delegate: exp.FirebaseFirestore) {}

export class FirebaseFirestore
extends Compat<exp.FirebaseFirestore>
implements legacy.FirebaseFirestore {
app = new FirebaseApp(this._delegate.app);

settings(settings: legacy.Settings): void {
Expand Down Expand Up @@ -170,11 +167,15 @@ export class FirebaseFirestore implements legacy.FirebaseFirestore {
};
}

export class Transaction implements legacy.Transaction {
export class Transaction
extends Compat<exp.Transaction>
implements legacy.Transaction {
constructor(
private readonly _firestore: FirebaseFirestore,
private readonly _delegate: exp.Transaction
) {}
private readonly delegate: exp.Transaction
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the private and readonly modifiers on delegate be dropped? This may be my TypeScript noobyness surfacing, but wouldn't this create two properties, delegate and _delegate, both of which store the same exp.Transaction object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

s/noobyness/expertyness/

Thanks for catching.

) {
super(delegate);
}

get<T>(documentRef: DocumentReference<T>): Promise<DocumentSnapshot<T>> {
return this._delegate
Expand Down Expand Up @@ -231,9 +232,9 @@ export class Transaction implements legacy.Transaction {
}
}

export class WriteBatch implements legacy.WriteBatch {
constructor(private readonly _delegate: exp.WriteBatch) {}

export class WriteBatch
extends Compat<exp.WriteBatch>
implements legacy.WriteBatch {
set<T>(
documentRef: DocumentReference<T>,
data: T,
Expand Down Expand Up @@ -288,11 +289,14 @@ export class WriteBatch implements legacy.WriteBatch {
}

export class DocumentReference<T = legacy.DocumentData>
extends Compat<exp.DocumentReference<T>>
implements legacy.DocumentReference<T> {
constructor(
readonly firestore: FirebaseFirestore,
readonly _delegate: exp.DocumentReference<T>
) {}
delegate: exp.DocumentReference<T>
) {
super(delegate);
}

readonly id = this._delegate.id;
readonly path = this._delegate.path;
Expand Down Expand Up @@ -407,11 +411,14 @@ export class DocumentReference<T = legacy.DocumentData>
}

export class DocumentSnapshot<T = legacy.DocumentData>
extends Compat<exp.DocumentSnapshot<T>>
implements legacy.DocumentSnapshot<T> {
constructor(
private readonly _firestore: FirebaseFirestore,
readonly _delegate: exp.DocumentSnapshot<T>
) {}
delegate: exp.DocumentSnapshot<T>
) {
super(delegate);
}

readonly ref = new DocumentReference<T>(this._firestore, this._delegate.ref);
readonly id = this._delegate.id;
Expand Down Expand Up @@ -449,11 +456,12 @@ export class QueryDocumentSnapshot<T = legacy.DocumentData>
}
}

export class Query<T = legacy.DocumentData> implements legacy.Query<T> {
constructor(
readonly firestore: FirebaseFirestore,
readonly _delegate: exp.Query<T>
) {}
export class Query<T = legacy.DocumentData>
extends Compat<exp.Query<T>>
implements legacy.Query<T> {
constructor(readonly firestore: FirebaseFirestore, delegate: exp.Query<T>) {
super(delegate);
}

where(
fieldPath: string | FieldPath,
Expand Down Expand Up @@ -680,34 +688,6 @@ export class CollectionReference<T = legacy.DocumentData>
}
}

export class FieldValue implements legacy.FieldValue {
constructor(readonly _delegate: exp.FieldValue) {}

static serverTimestamp(): FieldValue {
return new FieldValue(serverTimestamp());
}

static delete(): FieldValue {
return new FieldValue(deleteField());
}

static arrayUnion(...elements: any[]): FieldValue {
return new FieldValue(arrayUnion(...unwrap(elements)));
}

static arrayRemove(...elements: any[]): FieldValue {
return new FieldValue(arrayRemove(...unwrap(elements)));
}

static increment(n: number): FieldValue {
return new FieldValue(increment(n));
}

isEqual(other: FieldValue): boolean {
return this._delegate.isEqual(other._delegate);
}
}

export class FieldPath implements legacy.FieldPath {
private readonly fieldNames: string[];

Expand All @@ -728,9 +708,7 @@ export class FieldPath implements legacy.FieldPath {
}
}

export class Blob implements legacy.Blob {
constructor(readonly _delegate: BytesExp) {}

export class Blob extends Compat<BytesExp> implements legacy.Blob {
static fromBase64String(base64: string): Blob {
return new Blob(BytesExp.fromBase64String(base64));
}
Expand Down Expand Up @@ -790,17 +768,9 @@ function wrap(value: any): any {
function unwrap(value: any): any {
if (Array.isArray(value)) {
return value.map(v => unwrap(v));
} else if (value instanceof FieldPath) {
return value._delegate;
} else if (value instanceof FieldValue) {
} else if (value instanceof Compat) {
return value._delegate;
} else if (value instanceof Blob) {
return value._delegate;
} else if (value instanceof DocumentReference) {
return value._delegate;
} else if (value instanceof DocumentSnapshot) {
return value._delegate;
} else if (value instanceof QueryDocumentSnapshot) {
} else if (value instanceof FieldPath) {
return value._delegate;
} else if (isPlainObject(value)) {
const obj: any = {};
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/index.console.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@ export {
export { Blob } from './src/api/blob';
export { GeoPoint } from './src/api/geo_point';
export { FieldPath } from './src/api/field_path';
export { FieldValue } from './src/api/field_value';
export { FieldValue } from './src/compat/field_value';
export { Timestamp } from './src/api/timestamp';
52 changes: 6 additions & 46 deletions packages/firestore/lite/src/api/field_value.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,74 +21,34 @@ import {
ArrayUnionFieldValueImpl,
DeleteFieldValueImpl,
NumericIncrementFieldValueImpl,
_SerializableFieldValue,
ServerTimestampFieldValueImpl
} from '../../../src/api/field_value';
import { ParseContext } from '../../../src/api/user_data_reader';
import { FieldTransform } from '../../../src/model/mutation';

/** The public FieldValue class of the lite API. */
export abstract class FieldValue extends _SerializableFieldValue {}

/**
* A delegate class that allows the FieldValue implementations returned by
* deleteField(), serverTimestamp(), arrayUnion(), arrayRemove() and
* increment() to be an instance of the lite FieldValue class declared above.
*
* We don't directly subclass `FieldValue` in the various field value
* implementations as the base FieldValue class differs between the lite, full
* and legacy SDK.
*/
class FieldValueDelegate extends FieldValue {
readonly _methodName: string;

constructor(readonly _delegate: _SerializableFieldValue) {
super();
this._methodName = _delegate._methodName;
}

_toFieldTransform(context: ParseContext): FieldTransform | null {
return this._delegate._toFieldTransform(context);
}

isEqual(other: FieldValue): boolean {
if (!(other instanceof FieldValueDelegate)) {
return false;
}
return this._delegate.isEqual(other._delegate);
}
}
export abstract class FieldValue {}

export function deleteField(): FieldValue {
return new FieldValueDelegate(new DeleteFieldValueImpl('deleteField'));
return new DeleteFieldValueImpl('deleteField');
}

export function serverTimestamp(): FieldValue {
return new FieldValueDelegate(
new ServerTimestampFieldValueImpl('serverTimestamp')
);
return new ServerTimestampFieldValueImpl('serverTimestamp');
}

export function arrayUnion(...elements: unknown[]): FieldValue {
validateAtLeastNumberOfArgs('arrayUnion()', arguments, 1);
// NOTE: We don't actually parse the data until it's used in set() or
// update() since we'd need the Firestore instance to do this.
return new FieldValueDelegate(
new ArrayUnionFieldValueImpl('arrayUnion', elements)
);
return new ArrayUnionFieldValueImpl('arrayUnion', elements);
}

export function arrayRemove(...elements: unknown[]): FieldValue {
validateAtLeastNumberOfArgs('arrayRemove()', arguments, 1);
// NOTE: We don't actually parse the data until it's used in set() or
// update() since we'd need the Firestore instance to do this.
return new FieldValueDelegate(
new ArrayRemoveFieldValueImpl('arrayRemove', elements)
);
return new ArrayRemoveFieldValueImpl('arrayRemove', elements);
}

export function increment(n: number): FieldValue {
return new FieldValueDelegate(
new NumericIncrementFieldValueImpl('increment', n)
);
return new NumericIncrementFieldValueImpl('increment', n);
}
Loading