diff --git a/benchmark/repeated-fields-benchmark.js b/benchmark/repeated-fields-benchmark.js new file mode 100644 index 0000000000..b416986214 --- /dev/null +++ b/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 }); + }, +}; 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 185648fe29..851f7ea625 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts +++ b/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;