address reviewer comments

Author: stephenarosaj

CONTRIBUTING.md

@@ -153,8 +153,8 @@ And then:
 Currently, only the Auth, Database, and Firestore test suites work. Some test cases
 will be automatically skipped due to lack of emulator support. 
 
-You can also run the Data Connect test suite against the emulators using the same command,
-but you must run only the dataconnect tests, using a config file specific to Data Connect 
+**Note:** You can also run the Data Connect test suite against the emulators using the same command.
+However, you must isolate the Data Connect tests by using a configuration file specific to Data Connect
 emulator testing:
 
 ```bash

src/data-connect/data-connect-api-client-internal.ts

@@ -645,14 +645,18 @@ export function getFieldsString(data: unknown): string {
   return serializeFieldNode(root);
 }
 
-function mergeFieldsIntoTree(data: unknown, node: FieldNode): void {
+function mergeFieldsIntoTree(data: unknown, node: FieldNode, visited: Set<unknown> = new Set<unknown>()): void {
   if (validator.isArray(data)) {
-    data.forEach((item) => mergeFieldsIntoTree(item, node));
+    data.forEach((item) => mergeFieldsIntoTree(item, node, visited));
     return;
   }
   if (!validator.isNonNullObject(data) || data instanceof Date) {
     return;
   }
+  if (visited.has(data)) {
+    throw new Error('Circular reference detected in input.');
+  }
+  visited.add(data);
   const record = data as Record<string, unknown>;
   for (const [key, val] of Object.entries(record)) {
     if (val === undefined) {
@@ -664,9 +668,10 @@ function mergeFieldsIntoTree(data: unknown, node: FieldNode): void {
       node.children.set(key, childNode);
     }
     if (key.includes('_on_')) {
-      mergeFieldsIntoTree(val, childNode);
+      mergeFieldsIntoTree(val, childNode, visited);
     }
   }
+  visited.delete(data);
 }
 
 function serializeFieldNode(node: FieldNode): string {

test/unit/data-connect/data-connect-api-client-internal.spec.ts

@@ -775,6 +775,30 @@ describe('DataConnectApiClient CRUD helpers', () => {
         const fields = getFieldsString(dataWithUndefined);
         expect(fields).to.equal('director extras genre ratings title');
       });
+
+      it('should allow duplicate object references in different sibling branches (non-circular DAG)', () => {
+        const shared = { name: 'shared' };
+        const data = {
+          child1_on_foo: shared,
+          child2_on_bar: shared,
+        };
+        const fields = getFieldsString(data);
+        expect(fields).to.equal('child1_on_foo { name } child2_on_bar { name }');
+      });
+
+      it('should throw an error if a direct circular reference is detected', () => {
+        const data: any = { id: 'abc' };
+        data.direct_circular_on_reference = data;
+        expect(() => getFieldsString(data)).to.throw('Circular reference detected in input.');
+      });
+
+      it('should throw an error if an indirect circular reference is detected', () => {
+        const data: any = { id: 'abc' };
+        data.indirect_circular_on_reference = {
+          circular_on_reference: data,
+        };
+        expect(() => getFieldsString(data)).to.throw('Circular reference detected in input.');
+      });
     });
 
     describe('array of objects', () => {