From 2991112867dbe25937c330806864e133951a57e7 Mon Sep 17 00:00:00 2001
From: Ivan Goncharov <ivan.goncharov.ua@gmail.com>
Date: Tue, 5 Sep 2023 21:21:28 +0300
Subject: [PATCH] review changes

---
 .../OverlappingFieldsCanBeMergedRule-test.ts  | 18 +++++++++
 .../rules/OverlappingFieldsCanBeMergedRule.ts | 37 ++++++++++++-------
 2 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts
index 52c2deb1a0..ecb56a15cf 100644
--- a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts
+++ b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts
@@ -179,6 +179,24 @@ describe('Validate: Overlapping fields can be merged', () => {
     ]);
   });
 
+  it('different stream directive extra argument', () => {
+    expectErrors(`
+      fragment conflictingArgs on Dog {
+        name @stream(label: "streamLabel", initialCount: 1)
+        name @stream(label: "streamLabel", initialCount: 1, extraArg: true)
+      }
+    `).toDeepEqual([
+      {
+        message:
+          'Fields "name" conflict because they have differing stream directives. Use different aliases on the fields to fetch both if this was intentional.',
+        locations: [
+          { line: 3, column: 9 },
+          { line: 4, column: 9 },
+        ],
+      },
+    ]);
+  });
+
   it('mix of stream and no stream', () => {
     expectErrors(`
       fragment conflictingArgs on Dog {
diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
index 11ad8bd991..851f7ea625 100644
--- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
+++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
@@ -4,7 +4,6 @@ import type { Maybe } from '../../jsutils/Maybe.js';
 import { GraphQLError } from '../../error/GraphQLError.js';
 
 import type {
-  ArgumentNode,
   DirectiveNode,
   FieldNode,
   FragmentDefinitionNode,
@@ -593,7 +592,7 @@ function findConflict(
     }
 
     // Two field calls must have the same arguments.
-    if (!sameArguments(node1.arguments, node2.arguments)) {
+    if (!sameArguments(node1, node2)) {
       return [
         [responseName, 'they have differing arguments'],
         [node1],
@@ -651,24 +650,36 @@ function findConflict(
 }
 
 function sameArguments(
-  arguments1: ReadonlyArray<ArgumentNode> = [],
-  arguments2: ReadonlyArray<ArgumentNode> = [],
+  node1: FieldNode | DirectiveNode,
+  node2: FieldNode | DirectiveNode,
 ): boolean {
-  if (arguments1?.length !== arguments2?.length) {
+  const args1 = node1.arguments;
+  const args2 = node2.arguments;
+
+  if (args1 === undefined || args1.length === 0) {
+    return args2 === undefined || args2.length === 0;
+  }
+  if (args2 === undefined || args2.length === 0) {
     return false;
   }
-  return arguments1.every((argument1) => {
-    const argument2 = arguments2.find(
-      (argument) => argument.name.value === argument1.name.value,
-    );
-    if (!argument2) {
+
+  if (args1.length !== args2.length) {
+    return false;
+  }
+
+  const values2 = new Map(args2.map(({ name, value }) => [name.value, value]));
+  return args1.every((arg1) => {
+    const value1 = arg1.value;
+    const value2 = values2.get(arg1.name.value);
+    if (value2 === undefined) {
       return false;
     }
-    return stringifyValue(argument1.value) === stringifyValue(argument2.value);
+
+    return stringifyValue(value1) === stringifyValue(value2);
   });
 }
 
-function stringifyValue(value: ValueNode): string {
+function stringifyValue(value: ValueNode): string | null {
   return print(sortValueNode(value));
 }
 
@@ -689,7 +700,7 @@ function sameStreams(
     return true;
   } else if (stream1 && stream2) {
     // check if both fields have equivalent streams
-    return sameArguments(stream1.arguments, stream2.arguments);
+    return sameArguments(stream1, stream2);
   }
   // fields have a mix of stream and no stream
   return false;