OverlappingFieldsCanBeMergedRule: Fix performance degradation (#3958)

Co-authored-by: AaronMoat <AaronMoat@users.noreply.github.com>
Co-authored-by: Ivan Goncharov <ivan.goncharov.ua@gmail.com>
Resolves #3955 (at least

benchmark/repeated-fields-benchmark.js

@@ -0,0 +1,13 @@
+import { graphqlSync } from 'graphql/graphql.js';
+import { buildSchema } from 'graphql/utilities/buildASTSchema.js';
+
+const schema = buildSchema('type Query { hello: String! }');
+const source = `{ ${'hello '.repeat(250)}}`;
+
+export const benchmark = {
+ name: 'Many repeated fields',
+ count: 5,
+ measure() {
+ graphqlSync({ schema, source });
+ },
+};

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 {

src/validation/rules/OverlappingFieldsCanBeMergedRule.ts

@@ -7,8 +7,8 @@ import type {
  DirectiveNode,
  FieldNode,
  FragmentDefinitionNode,
- ObjectValueNode,
  SelectionSetNode,
+ ValueNode,
 } from '../../language/ast.js';
 import { Kind } from '../../language/kinds.js';
 import { print } from '../../language/printer.js';
@@ -592,7 +592,7 @@ function findConflict(
  }
 
  // Two field calls must have the same arguments.
- if (stringifyArguments(node1) !== stringifyArguments(node2)) {
+ if (!sameArguments(node1, node2)) {
  return [
  [responseName, 'they have differing arguments'],
  [node1],
@@ -649,19 +649,38 @@ function findConflict(
  }
 }
 
-function stringifyArguments(fieldNode: FieldNode | DirectiveNode): string {
- // FIXME https://github.com/graphql/graphql-js/issues/2203
- const args = /* c8 ignore next */ fieldNode.arguments ?? [];
-
- const inputObjectWithArgs: ObjectValueNode = {
- kind: Kind.OBJECT,
- fields: args.map((argNode) => ({
- kind: Kind.OBJECT_FIELD,
- name: argNode.name,
- value: argNode.value,
- })),
- };
- return print(sortValueNode(inputObjectWithArgs));
+function sameArguments(
+ node1: FieldNode | DirectiveNode,
+ node2: FieldNode | DirectiveNode,
+): boolean {
+ 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;
+ }
+
+ 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(value1) === stringifyValue(value2);
+ });
+}
+
+function stringifyValue(value: ValueNode): string | null {
+ return print(sortValueNode(value));
 }
 
 function getStreamDirective(
@@ -681,7 +700,7 @@ function sameStreams(
  return true;
  } else if (stream1 && stream2) {
  // check if both fields have equivalent streams
- return stringifyArguments(stream1) === stringifyArguments(stream2);
+ return sameArguments(stream1, stream2);
  }
  // fields have a mix of stream and no stream
  return false;