refactor(deprecation)!: remove legacy support for array arguments (#625)

googleapis · May 16, 2019 · 54dd405 · 54dd405
1 parent 9474e3f
commit 54dd405
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 55 deletions.
diff --git a/dev/conformance/runner.ts b/dev/conformance/runner.ts
@@ -162,7 +162,7 @@ const convertInput = {
     if (path.field.length === 1 && path.field[0] === '__name__') {
       return FieldPath.documentId();
     }
-    return new FieldPath(path.field);
+    return new FieldPath(...path.field);
   },
   paths: (fields: ConformanceProto): FieldPath[] => {
     const convertedPaths: FieldPath[] = [];
@@ -409,7 +409,7 @@ function runTest(spec: ConformanceProto) {
       } else if (spec.option && spec.option.fields) {
         setOption.mergeFields = [];
         for (const fieldPath of spec.option.fields) {
-          setOption.mergeFields.push(new FieldPath(fieldPath.field));
+          setOption.mergeFields.push(new FieldPath(...fieldPath.field));
         }
       }
       return docRef(setSpec.docRefPath).set(

diff --git a/dev/src/index.ts b/dev/src/index.ts
@@ -819,9 +819,8 @@ export class Firestore {
    * followed by any additional `DocumentReference` documents. If used, the
    * optional `ReadOptions` must be the last argument.
    *
-   * @param {Array.<DocumentReference|ReadOptions>} documentRefsOrReadOptions
-   * The `DocumentReferences` to receive, followed by an optional field
-   * mask.
+   * @param {...DocumentReference|ReadOptions} documentRefsOrReadOptions The
+   * `DocumentReferences` to receive, followed by an optional field mask.
    * @returns {Promise<Array.<DocumentSnapshot>>} A Promise that
    * contains an array with the resulting document snapshots.
    *

diff --git a/dev/src/path.ts b/dev/src/path.ts
@@ -508,8 +508,7 @@ export class FieldPath extends Path<FieldPath> {
   /**
    * Constructs a Firestore Field Path.
    *
-   * @param {...string|string[]} segments Sequence of field names that form
-   * this path.
+   * @param {...string} segments Sequence of field names that form this path.
    *
    * @example
    * let query = firestore.collection('col');
@@ -522,20 +521,23 @@ export class FieldPath extends Path<FieldPath> {
    * });
    */
   constructor(...segments: string[]) {
-    const elements: string[] = Array.isArray(segments[0])
-      ? ((segments[0] as unknown) as string[])
-      : segments;
+    if (Array.isArray(segments[0])) {
+      throw new Error(
+        'The FieldPath constructor no longer supports an array as its first argument. ' +
+          'Please unpack your array and call FieldPath() with individual arguments.'
+      );
+    }
 
-    validateMinNumberOfArguments('FieldPath', elements, 1);
+    validateMinNumberOfArguments('FieldPath', segments, 1);
 
-    for (let i = 0; i < elements.length; ++i) {
-      validateString(i, elements[i]);
-      if (elements[i].length === 0) {
+    for (let i = 0; i < segments.length; ++i) {
+      validateString(i, segments[i]);
+      if (segments[i].length === 0) {
         throw new Error(`Element at index ${i} should not be an empty string.`);
       }
     }
 
-    super(elements);
+    super(segments);
   }
 
   /**

diff --git a/dev/src/transaction.ts b/dev/src/transaction.ts
@@ -163,9 +163,8 @@ export class Transaction {
    * followed by any additional `DocumentReference` documents. If used, the
    * optional `ReadOptions` must be the last argument.
    *
-   * @param {Array.<DocumentReference|ReadOptions>} documentRefsOrReadOptions
-   * The `DocumentReferences` to receive, followed by an optional field
-   * mask.
+   * @param {...DocumentReference|ReadOptions} documentRefsOrReadOptions The
+   * `DocumentReferences` to receive, followed by an optional field mask.
    * @returns {Promise<Array.<DocumentSnapshot>>} A Promise that
    * contains an array with the resulting document snapshots.
    *
@@ -433,30 +432,23 @@ export function parseGetAllArguments(
   let documents: DocumentReference[];
   let readOptions: ReadOptions | undefined = undefined;
 
-  // In the original release of the SDK, getAll() was documented to accept
-  // either a varargs list of DocumentReferences or a single array of
-  // DocumentReferences. To support this usage in the TypeScript client, we have
-  // to manually verify the arguments to determine which input the user
-  // provided.
-  const usesDeprecatedArgumentStyle = Array.isArray(
-    documentRefsOrReadOptions[0]
-  );
-
-  if (usesDeprecatedArgumentStyle) {
-    documents = documentRefsOrReadOptions[0] as DocumentReference[];
-    readOptions = documentRefsOrReadOptions[1] as ReadOptions;
+  if (Array.isArray(documentRefsOrReadOptions[0])) {
+    throw new Error(
+      'getAll() no longer accepts an array as its first argument. ' +
+        'Please unpack your array and call getAll() with individual arguments.'
+    );
+  }
+
+  if (
+    documentRefsOrReadOptions.length > 0 &&
+    isPlainObject(
+      documentRefsOrReadOptions[documentRefsOrReadOptions.length - 1]
+    )
+  ) {
+    readOptions = documentRefsOrReadOptions.pop() as ReadOptions;
+    documents = documentRefsOrReadOptions as DocumentReference[];
   } else {
-    if (
-      documentRefsOrReadOptions.length > 0 &&
-      isPlainObject(
-        documentRefsOrReadOptions[documentRefsOrReadOptions.length - 1]
-      )
-    ) {
-      readOptions = documentRefsOrReadOptions.pop() as ReadOptions;
-      documents = documentRefsOrReadOptions as DocumentReference[];
-    } else {
-      documents = documentRefsOrReadOptions as DocumentReference[];
-    }
+    documents = documentRefsOrReadOptions as DocumentReference[];
   }
 
   for (let i = 0; i < documents.length; ++i) {

diff --git a/dev/test/index.ts b/dev/test/index.ts
@@ -938,20 +938,6 @@ describe('getAll() method', () => {
     });
   });
 
-  it('accepts array', () => {
-    const overrides: ApiOverride = {
-      batchGetDocuments: () => stream(found('documentId')),
-    };
-
-    return createInstance(overrides).then(firestore => {
-      return (firestore as InvalidApiUsage)
-        .getAll([firestore.doc('collectionId/documentId')])
-        .then((result: DocumentSnapshot[]) => {
-          resultEquals(result, found('documentId'));
-        });
-    });
-  });
-
   it('returns not found for missing documents', () => {
     const overrides: ApiOverride = {
       batchGetDocuments: () => stream(found('exists'), missing('missing')),