Add support for collection(coll) and doc(doc) (#3403)

schmidt-sebastian · web-flow · commit 5b7d812a4ea9 · 2020-07-16T15:12:27.000-07:00
diff --git a/packages/firestore/exp/index.d.ts b/packages/firestore/exp/index.d.ts
@@ -109,6 +109,10 @@ export function collection(
   firestore: FirebaseFirestore,
   collectionPath: string
 ): CollectionReference<DocumentData>;
+export function collection(
+  reference: CollectionReference<unknown>,
+  collectionPath: string
+): CollectionReference<DocumentData>;
 export function collection(
   reference: DocumentReference,
   collectionPath: string
@@ -121,6 +125,10 @@ export function doc<T>(
   reference: CollectionReference<T>,
   documentPath?: string
 ): DocumentReference<T>;
+export function doc(
+  reference: DocumentReference<unknown>,
+  documentPath: string
+): DocumentReference<DocumentData>;
 export function parent(
   reference: CollectionReference<unknown>
 ): DocumentReference<DocumentData> | null;
diff --git a/packages/firestore/lite/index.d.ts b/packages/firestore/lite/index.d.ts
@@ -70,6 +70,10 @@ export function collection(
   firestore: FirebaseFirestore,
   collectionPath: string
 ): CollectionReference<DocumentData>;
+export function collection(
+  reference: CollectionReference<unknown>,
+  collectionPath: string
+): CollectionReference<DocumentData>;
 export function collection(
   reference: DocumentReference,
   collectionPath: string
@@ -82,6 +86,10 @@ export function doc<T>(
   reference: CollectionReference<T>,
   documentPath?: string
 ): DocumentReference<T>;
+export function doc(
+  reference: DocumentReference<unknown>,
+  documentPath: string
+): DocumentReference<DocumentData>;
 export function parent(
   reference: CollectionReference<unknown>
 ): DocumentReference<DocumentData> | null;
diff --git a/packages/firestore/lite/src/api/reference.ts b/packages/firestore/lite/src/api/reference.ts
@@ -289,25 +289,43 @@ export function collection(
   firestore: firestore.FirebaseFirestore,
   collectionPath: string
 ): CollectionReference<firestore.DocumentData>;
+export function collection(
+  reference: firestore.CollectionReference<unknown>,
+  collectionPath: string
+): CollectionReference<firestore.DocumentData>;
 export function collection(
   reference: firestore.DocumentReference,
   collectionPath: string
 ): CollectionReference<firestore.DocumentData>;
 export function collection(
-  parent: firestore.FirebaseFirestore | firestore.DocumentReference<unknown>,
+  parent:
+    | firestore.FirebaseFirestore
+    | firestore.DocumentReference<unknown>
+    | firestore.CollectionReference<unknown>,
   relativePath: string
 ): CollectionReference<firestore.DocumentData> {
   validateArgType('collection', 'non-empty string', 2, relativePath);
-  const path = ResourcePath.fromString(relativePath);
   if (parent instanceof Firestore) {
-    validateCollectionPath(path);
-    return new CollectionReference(parent, path, /* converter= */ null);
+    const absolutePath = ResourcePath.fromString(relativePath);
+    validateCollectionPath(absolutePath);
+    return new CollectionReference(parent, absolutePath, /* converter= */ null);
   } else {
-    const doc = cast(parent, DocumentReference);
-    const absolutePath = doc._key.path.child(path);
+    if (
+      !(parent instanceof DocumentReference) &&
+      !(parent instanceof CollectionReference)
+    ) {
+      throw new FirestoreError(
+        Code.INVALID_ARGUMENT,
+        'Expected first argument to collection() to be a CollectionReference, ' +
+          'a DocumentReference or FirebaseFirestore'
+      );
+    }
+    const absolutePath = ResourcePath.fromString(
+      `${parent.path}/${relativePath}`
+    );
     validateCollectionPath(absolutePath);
     return new CollectionReference(
-      doc.firestore,
+      parent.firestore,
       absolutePath,
       /* converter= */ null
     );
@@ -346,8 +364,15 @@ export function doc<T>(
   reference: firestore.CollectionReference<T>,
   documentPath?: string
 ): DocumentReference<T>;
+export function doc(
+  reference: firestore.DocumentReference<unknown>,
+  documentPath: string
+): DocumentReference<firestore.DocumentData>;
 export function doc<T>(
-  parent: firestore.FirebaseFirestore | firestore.CollectionReference<T>,
+  parent:
+    | firestore.FirebaseFirestore
+    | firestore.CollectionReference<T>
+    | firestore.DocumentReference<unknown>,
   relativePath?: string
 ): DocumentReference {
   // We allow omission of 'pathString' but explicitly prohibit passing in both
@@ -356,22 +381,34 @@ export function doc<T>(
     relativePath = AutoId.newId();
   }
   validateArgType('doc', 'non-empty string', 2, relativePath);
-  const path = ResourcePath.fromString(relativePath!);
+
   if (parent instanceof Firestore) {
-    validateDocumentPath(path);
+    const absolutePath = ResourcePath.fromString(relativePath!);
+    validateDocumentPath(absolutePath);
     return new DocumentReference(
       parent,
-      new DocumentKey(path),
+      new DocumentKey(absolutePath),
       /* converter= */ null
     );
   } else {
-    const coll = cast(parent, CollectionReference);
-    const absolutePath = coll._path.child(path);
+    if (
+      !(parent instanceof DocumentReference) &&
+      !(parent instanceof CollectionReference)
+    ) {
+      throw new FirestoreError(
+        Code.INVALID_ARGUMENT,
+        'Expected first argument to collection() to be a CollectionReference, ' +
+          'a DocumentReference or FirebaseFirestore'
+      );
+    }
+    const absolutePath = ResourcePath.fromString(
+      `${parent.path}/${relativePath}`
+    );
     validateDocumentPath(absolutePath);
     return new DocumentReference(
-      coll.firestore,
+      parent.firestore,
       new DocumentKey(absolutePath),
-      coll._converter
+      parent instanceof CollectionReference ? parent._converter : null
     );
   }
 }
diff --git a/packages/firestore/lite/test/integration.test.ts b/packages/firestore/lite/test/integration.test.ts
@@ -143,7 +143,7 @@ describe('Firestore', () => {
 });
 
 describe('doc', () => {
-  it('can provide name', () => {
+  it('can be used relative to Firestore root', () => {
     return withTestDb(db => {
       const result = doc(db, 'coll/doc');
       expect(result).to.be.an.instanceOf(DocumentReference);
@@ -152,6 +152,24 @@ describe('doc', () => {
     });
   });
 
+  it('can be used relative to collection', () => {
+    return withTestDb(db => {
+      const result = doc(collection(db, 'coll'), 'doc');
+      expect(result).to.be.an.instanceOf(DocumentReference);
+      expect(result.id).to.equal('doc');
+      expect(result.path).to.equal('coll/doc');
+    });
+  });
+
+  it('can be relative to doc', () => {
+    return withTestDb(db => {
+      const result = doc(doc(db, 'coll/doc'), 'subcoll/subdoc');
+      expect(result).to.be.an.instanceOf(DocumentReference);
+      expect(result.id).to.equal('subdoc');
+      expect(result.path).to.equal('coll/doc/subcoll/subdoc');
+    });
+  });
+
   it('validates path', () => {
     return withTestDb(db => {
       expect(() => doc(db, 'coll')).to.throw(
@@ -179,7 +197,7 @@ describe('doc', () => {
 });
 
 describe('collection', () => {
-  it('can provide name', () => {
+  it('can be used relative to Firestore root', () => {
     return withTestDb(db => {
       const result = collection(db, 'coll/doc/subcoll');
       expect(result).to.be.an.instanceOf(CollectionReference);
@@ -188,6 +206,24 @@ describe('collection', () => {
     });
   });
 
+  it('can be used relative to collection', () => {
+    return withTestDb(db => {
+      const result = collection(collection(db, 'coll'), 'doc/subcoll');
+      expect(result).to.be.an.instanceOf(CollectionReference);
+      expect(result.id).to.equal('subcoll');
+      expect(result.path).to.equal('coll/doc/subcoll');
+    });
+  });
+
+  it('can be used relative to doc', () => {
+    return withTestDb(db => {
+      const result = collection(doc(db, 'coll/doc'), 'subcoll');
+      expect(result).to.be.an.instanceOf(CollectionReference);
+      expect(result.id).to.equal('subcoll');
+      expect(result.path).to.equal('coll/doc/subcoll');
+    });
+  });
+
   it('validates path', () => {
     return withTestDb(db => {
       expect(() => collection(db, 'coll/doc')).to.throw(
@@ -196,7 +232,7 @@ describe('collection', () => {
       // TODO(firestorelite): Explore returning a more helpful message
       // (e.g. "Empty document paths are not supported.")
       expect(() => collection(doc(db, 'coll/doc'), '')).to.throw(
-        'Function doc() requires its second argument to be of type non-empty string, but it was: ""'
+        'Function collection() requires its second argument to be of type non-empty string, but it was: ""'
       );
       expect(() => collection(doc(db, 'coll/doc'), 'coll/doc')).to.throw(
         'Invalid collection path (coll/doc/coll/doc). Path points to a document.'
@@ -945,8 +981,7 @@ describe('equality', () => {
         const query1a = collRef.orderBy('foo');
         const query1b = collRef.orderBy('foo', 'asc');
         const query2 = collRef.orderBy('foo', 'desc');
-        // TODO(firestorelite): Should we allow `collectionRef(collRef, 'a/b')?
-        const query3 = collection(doc(collRef, 'a'), 'b').orderBy('foo');
+        const query3 = collection(collRef, 'a/b').orderBy('foo');
 
         expect(queryEqual(query1a, query1b)).to.be.true;
         expect(queryEqual(query1a, query2)).to.be.false;
@@ -1029,6 +1064,16 @@ describe('withConverter() support', () => {
     });
   });
 
+  it('keeps the converter when calling parent() with a DocumentReference', () => {
+    return withTestDb(async db => {
+      const coll = doc(db, 'root/doc').withConverter(postConverter);
+      const typedColl = parent(coll)!;
+      expect(
+        refEqual(typedColl, collection(db, 'root').withConverter(postConverter))
+      ).to.be.true;
+    });
+  });
+
   it('drops the converter when calling parent() with a CollectionReference', () => {
     return withTestDb(async db => {
       const coll = collection(db, 'root/doc/parent').withConverter(
