Skip to content

Commit

Permalink
OverlappingFieldsCanBeMergedRule: Fix performance degradation (#3967)
Browse files Browse the repository at this point in the history
fixes from #3958 into
  • Loading branch information
AaronMoat authored Sep 10, 2023
1 parent e4f759d commit 8f4c64e
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 15 deletions.
15 changes: 15 additions & 0 deletions benchmark/repeated-fields-benchmark.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';

const { graphqlSync } = require('graphql/graphql.js');
const { buildSchema } = require('graphql/utilities/buildASTSchema.js');

const schema = buildSchema('type Query { hello: String! }');
const source = `{ ${'hello '.repeat(250)}}`;

module.exports = {
name: 'Many repeated fields',
count: 5,
measure() {
graphqlSync({ schema, source });
},
};
53 changes: 38 additions & 15 deletions src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ import type { ObjMap } from '../../jsutils/ObjMap';
import { GraphQLError } from '../../error/GraphQLError';

import type {
DirectiveNode,
FieldNode,
FragmentDefinitionNode,
ObjectValueNode,
SelectionSetNode,
ValueNode,
} from '../../language/ast';
import { Kind } from '../../language/kinds';
import { print } from '../../language/printer';
Expand Down Expand Up @@ -588,7 +589,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],
Expand Down Expand Up @@ -634,19 +635,41 @@ function findConflict(
}
}

function stringifyArguments(fieldNode: FieldNode): 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;
}

/* c8 ignore next */
if (args1.length !== args2.length) {
/* c8 ignore next */
return false;
/* c8 ignore next */
}

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));
}

// Two types conflict if both types could not apply to a value simultaneously.
Expand Down

0 comments on commit 8f4c64e

Please sign in to comment.