Add ProtoJS types to generated proto types (#2749)

Author: schmidt-sebastian

packages/firestore/src/model/proto_values.ts

@@ -32,11 +32,6 @@ const ISO_TIMESTAMP_REG_EXP = new RegExp(
   /^\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d(?:\.(\d+))?Z$/
 );
 
-// Denotes the possible representations for timestamps in the Value type.
-type ProtoTimestampValue =
-  | string
-  | { seconds?: string | number; nanos?: number };
-
 /** Extracts the backend's type order for the provided value. */
 export function typeOrder(value: api.Value): TypeOrder {
   if ('nullValue' in value) {
@@ -235,10 +230,7 @@ function compareNumbers(left: api.Value, right: api.Value): number {
   return numericComparator(leftNumber, rightNumber);
 }
 
-function compareTimestamps(
-  left: ProtoTimestampValue,
-  right: ProtoTimestampValue
-): number {
+function compareTimestamps(left: api.Timestamp, right: api.Timestamp): number {
   if (typeof left === 'string' && typeof right === 'string') {
     // Use string ordering for ISO 8601 timestamps, but strip the timezone
     // suffix to ensure proper ordering for timestamps of different precision.
@@ -376,7 +368,7 @@ function canonifyByteString(byteString: string | Uint8Array): string {
   return normalizeByteString(byteString).toBase64();
 }
 
-function canonifyTimestamp(timestamp: ProtoTimestampValue): string {
+function canonifyTimestamp(timestamp: api.Timestamp): string {
   const normalizedTimestamp = normalizeTimestamp(timestamp);
   return `time(${normalizedTimestamp.seconds},${normalizedTimestamp.nanos})`;
 }
@@ -478,7 +470,7 @@ function estimateArrayByteSize(arrayValue: api.ArrayValue): number {
  * nanos" representation.
  */
 export function normalizeTimestamp(
-  date: ProtoTimestampValue
+  date: api.Timestamp
 ): { seconds: number; nanos: number } {
   assert(!!date, 'Cannot normalize null or undefined timestamp.');
 

packages/firestore/src/protos/firestore_proto_api.d.ts

@@ -26,6 +26,9 @@ export declare type PromiseRequestService = any;
 export interface ApiClientObjectMap<T> {
   [k: string]: T;
 }
+export declare type Timestamp =
+  | string
+  | { seconds?: string | number; nanos?: number };
 
 export declare type CompositeFilterOp = 'OPERATOR_UNSPECIFIED' | 'AND';
 export interface ICompositeFilterOpEnum {
@@ -180,7 +183,7 @@ export declare namespace firestoreV1ApiClientInterfaces {
     name?: string;
     fields?: ApiClientObjectMap<Value>;
     createTime?: string;
-    updateTime?: string;
+    updateTime?: Timestamp;
   }
   interface DocumentChange {
     document?: Document;
@@ -190,7 +193,7 @@ export declare namespace firestoreV1ApiClientInterfaces {
   interface DocumentDelete {
     document?: string;
     removedTargetIds?: number[];
-    readTime?: string;
+    readTime?: Timestamp;
   }
   interface DocumentMask {
     fieldPaths?: string[];
@@ -290,7 +293,7 @@ export declare namespace firestoreV1ApiClientInterfaces {
   }
   interface Precondition {
     exists?: boolean;
-    updateTime?: string;
+    updateTime?: Timestamp;
   }
   interface Projection {
     fields?: FieldReference[];
@@ -333,12 +336,12 @@ export declare namespace firestoreV1ApiClientInterfaces {
     startAt?: Cursor;
     endAt?: Cursor;
     offset?: number;
-    limit?: number;
+    limit?: number | { value: number };
   }
   interface Target {
     query?: QueryTarget;
     documents?: DocumentsTarget;
-    resumeToken?: string;
+    resumeToken?: string | Uint8Array;
     readTime?: string;
     targetId?: number;
     once?: boolean;
@@ -347,8 +350,8 @@ export declare namespace firestoreV1ApiClientInterfaces {
     targetChangeType?: TargetChangeTargetChangeType;
     targetIds?: number[];
     cause?: Status;
-    resumeToken?: string;
-    readTime?: string;
+    resumeToken?: string | Uint8Array;
+    readTime?: Timestamp;
   }
   interface TransactionOptions {
     readOnly?: ReadOnly;
@@ -363,7 +366,7 @@ export declare namespace firestoreV1ApiClientInterfaces {
     booleanValue?: boolean;
     integerValue?: string | number;
     doubleValue?: string | number;
-    timestampValue?: string | { seconds?: string | number; nanos?: number };
+    timestampValue?: Timestamp;
     stringValue?: string;
     bytesValue?: string | Uint8Array;
     referenceValue?: string;
@@ -382,17 +385,17 @@ export declare namespace firestoreV1ApiClientInterfaces {
   interface WriteRequest {
     streamId?: string;
     writes?: Write[];
-    streamToken?: string;
+    streamToken?: string | Uint8Array;
     labels?: ApiClientObjectMap<string>;
   }
   interface WriteResponse {
     streamId?: string;
     streamToken?: string;
     writeResults?: WriteResult[];
-    commitTime?: string;
+    commitTime?: Timestamp;
   }
   interface WriteResult {
-    updateTime?: string;
+    updateTime?: Timestamp;
     transformResults?: Value[];
   }
 }

packages/firestore/src/remote/serializer.ts

@@ -54,7 +54,7 @@ import { assert, fail } from '../util/assert';
 import { Code, FirestoreError } from '../util/error';
 import * as obj from '../util/obj';
 import { ByteString } from '../util/byte_string';
-import * as typeUtils from '../util/types';
+import { isNegativeZero, isNullOrUndefined } from '../util/types';
 
 import {
   ArrayRemoveTransformOperation,
@@ -99,14 +99,7 @@ const OPERATORS = (() => {
 })();
 
 function assertPresent(value: unknown, description: string): asserts value {
-  assert(!typeUtils.isNullOrUndefined(value), description + ' is missing');
-}
-
-// This is a supplement to the generated proto interfaces, which fail to account
-// for the fact that a timestamp may be encoded as either a string OR this.
-interface TimestampProto {
-  seconds?: string | number;
-  nanos?: number;
+  assert(!isNullOrUndefined(value), description + ' is missing');
 }
 
 export interface SerializerOptions {
@@ -148,46 +141,33 @@ export class JsonProtoSerializer {
    * our generated proto interfaces say Int32Value must be. But GRPC actually
    * expects a { value: <number> } struct.
    */
-  private toInt32Value(val: number | null): number | null {
-    if (this.options.useProto3Json || typeUtils.isNullOrUndefined(val)) {
+  private toInt32Proto(val: number | null): number | { value: number } | null {
+    if (this.options.useProto3Json || isNullOrUndefined(val)) {
       return val;
     } else {
-      // ProtobufJS requires that we wrap Int32Values.
-      // Use any because we need to match generated Proto types.
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      return { value: val } as any;
+      return { value: val };
     }
   }
 
   /**
    * Returns a number (or null) from a google.protobuf.Int32Value proto.
-   * DO NOT USE THIS FOR ANYTHING ELSE.
-   * This method cheats. It's typed as accepting "number" because that's what
-   * our generated proto interfaces say Int32Value must be, but it actually
-   * accepts { value: number } to match our serialization in toInt32Value().
    */
-  private fromInt32Value(val: number | undefined): number | null {
+  private fromInt32Proto(
+    val: number | { value: number } | undefined
+  ): number | null {
     let result;
     if (typeof val === 'object') {
-      // Use any because we need to match generated Proto types.
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      result = (val as any).value;
+      result = val.value;
     } else {
-      // We accept raw numbers (without the {value: ... } wrapper) for
-      // compatibility with legacy persisted data.
       result = val;
     }
-    return typeUtils.isNullOrUndefined(result) ? null : result;
+    return isNullOrUndefined(result) ? null : result;
   }
 
   /**
    * Returns a value for a Date that's appropriate to put into a proto.
-   * DO NOT USE THIS FOR ANYTHING ELSE.
-   * This method cheats. It's typed as returning "string" because that's what
-   * our generated proto interfaces say dates must be. But it's easier and safer
-   * to actually return a Timestamp proto.
    */
-  private toTimestamp(timestamp: Timestamp): string {
+  private toTimestamp(timestamp: Timestamp): api.Timestamp {
     if (this.options.useProto3Json) {
       // Serialize to ISO-8601 date format, but with full nano resolution.
       // Since JS Date has only millis, let's only use it for the seconds and
@@ -208,25 +188,21 @@ export class JsonProtoSerializer {
     }
   }
 
-  private fromTimestamp(date: string | TimestampProto): Timestamp {
+  private fromTimestamp(date: api.Timestamp): Timestamp {
     const timestamp = normalizeTimestamp(date);
     return new Timestamp(timestamp.seconds, timestamp.nanos);
   }
 
   /**
    * Returns a value for bytes that's appropriate to put in a proto.
-   * DO NOT USE THIS FOR ANYTHING ELSE.
-   * This method cheats. It's typed as returning "string" because that's what
-   * our generated proto interfaces say bytes must be. But it should return
-   * an Uint8Array in Node.
    *
    * Visible for testing.
    */
-  toBytes(bytes: Blob | ByteString): string {
+  toBytes(bytes: Blob | ByteString): string | Uint8Array {
     if (this.options.useProto3Json) {
       return bytes.toBase64();
     } else {
-      return (bytes.toUint8Array() as unknown) as string;
+      return bytes.toUint8Array();
     }
   }
 
@@ -249,11 +225,11 @@ export class JsonProtoSerializer {
     }
   }
 
-  toVersion(version: SnapshotVersion): string {
+  toVersion(version: SnapshotVersion): api.Timestamp {
     return this.toTimestamp(version.toTimestamp());
   }
 
-  fromVersion(version: string): SnapshotVersion {
+  fromVersion(version: api.Timestamp): SnapshotVersion {
     assert(!!version, "Trying to deserialize version that isn't set");
     return SnapshotVersion.fromTimestamp(this.fromTimestamp(version));
   }
@@ -374,7 +350,7 @@ export class JsonProtoSerializer {
           return { doubleValue: 'Infinity' } as {};
         } else if (doubleValue === -Infinity) {
           return { doubleValue: '-Infinity' } as {};
-        } else if (typeUtils.isNegativeZero(doubleValue)) {
+        } else if (isNegativeZero(doubleValue)) {
           return { doubleValue: '-0' } as {};
         }
       }
@@ -846,7 +822,7 @@ export class JsonProtoSerializer {
 
   private fromWriteResult(
     proto: api.WriteResult,
-    commitTime: string
+    commitTime: api.Timestamp
   ): MutationResult {
     // NOTE: Deletes don't have an updateTime.
     let version = proto.updateTime
@@ -873,7 +849,7 @@ export class JsonProtoSerializer {
 
   fromWriteResults(
     protos: api.WriteResult[] | undefined,
-    commitTime?: string
+    commitTime?: api.Timestamp
   ): MutationResult[] {
     if (protos && protos.length > 0) {
       assert(
@@ -1000,7 +976,7 @@ export class JsonProtoSerializer {
       result.structuredQuery!.orderBy = orderBy;
     }
 
-    const limit = this.toInt32Value(target.limit);
+    const limit = this.toInt32Proto(target.limit);
     if (limit !== null) {
       result.structuredQuery!.limit = limit;
     }
@@ -1046,7 +1022,7 @@ export class JsonProtoSerializer {
 
     let limit: number | null = null;
     if (query.limit) {
-      limit = this.fromInt32Value(query.limit);
+      limit = this.fromInt32Proto(query.limit);
     }
 
     let startAt: Bound | null = null;

packages/firestore/src/util/types.ts

@@ -23,12 +23,12 @@ export interface StringMap {
 /**
  * Returns whether a variable is either undefined or null.
  */
-export function isNullOrUndefined(value: unknown): boolean {
+export function isNullOrUndefined(value: unknown): value is null | undefined {
   return value === null || value === undefined;
 }
 
 /** Returns whether the value represents -0. */
-export function isNegativeZero(value: number) : boolean {
+export function isNegativeZero(value: number): boolean {
   // Detect if the value is -0.0. Based on polyfill from
   // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is
   return value === -0 && 1 / value === 1 / -0;

packages/firestore/test/unit/specs/spec_test_runner.ts

@@ -196,7 +196,10 @@ class MockConnection implements Connection {
     return this.watchOpen.promise;
   }
 
-  ackWrite(commitTime?: string, mutationResults?: api.WriteResult[]): void {
+  ackWrite(
+    commitTime?: api.Timestamp,
+    mutationResults?: api.WriteResult[]
+  ): void {
     this.writeStream!.callOnMessage({
       // Convert to base64 string so it can later be parsed into ByteString.
       streamToken: PlatformSupport.getPlatform().btoa(
@@ -1151,7 +1154,7 @@ abstract class TestRunner {
       expect(actualTarget.query).to.deep.equal(expectedTarget.query);
       expect(actualTarget.targetId).to.equal(expectedTarget.targetId);
       expect(actualTarget.readTime).to.equal(expectedTarget.readTime);
-      expect(actualTarget.resumeToken || '').to.equal(expectedTarget.resumeToken || '');
+      expect(actualTarget.resumeToken).to.equal(expectedTarget.resumeToken);
       delete actualTargets[targetId];
     });
     expect(obj.size(actualTargets)).to.equal(