Skip to content

Commit 8f4c64e

Browse files
authored
OverlappingFieldsCanBeMergedRule: Fix performance degradation (#3967)
fixes from #3958 into
1 parent e4f759d commit 8f4c64e

File tree

2 files changed

+53
-15
lines changed

2 files changed

+53
-15
lines changed
+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
'use strict';
2+
3+
const { graphqlSync } = require('graphql/graphql.js');
4+
const { buildSchema } = require('graphql/utilities/buildASTSchema.js');
5+
6+
const schema = buildSchema('type Query { hello: String! }');
7+
const source = `{ ${'hello '.repeat(250)}}`;
8+
9+
module.exports = {
10+
name: 'Many repeated fields',
11+
count: 5,
12+
measure() {
13+
graphqlSync({ schema, source });
14+
},
15+
};

src/validation/rules/OverlappingFieldsCanBeMergedRule.ts

+38-15
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@ import type { ObjMap } from '../../jsutils/ObjMap';
55
import { GraphQLError } from '../../error/GraphQLError';
66

77
import type {
8+
DirectiveNode,
89
FieldNode,
910
FragmentDefinitionNode,
10-
ObjectValueNode,
1111
SelectionSetNode,
12+
ValueNode,
1213
} from '../../language/ast';
1314
import { Kind } from '../../language/kinds';
1415
import { print } from '../../language/printer';
@@ -588,7 +589,7 @@ function findConflict(
588589
}
589590

590591
// Two field calls must have the same arguments.
591-
if (stringifyArguments(node1) !== stringifyArguments(node2)) {
592+
if (!sameArguments(node1, node2)) {
592593
return [
593594
[responseName, 'they have differing arguments'],
594595
[node1],
@@ -634,19 +635,41 @@ function findConflict(
634635
}
635636
}
636637

637-
function stringifyArguments(fieldNode: FieldNode): string {
638-
// FIXME https://github.com/graphql/graphql-js/issues/2203
639-
const args = /* c8 ignore next */ fieldNode.arguments ?? [];
640-
641-
const inputObjectWithArgs: ObjectValueNode = {
642-
kind: Kind.OBJECT,
643-
fields: args.map((argNode) => ({
644-
kind: Kind.OBJECT_FIELD,
645-
name: argNode.name,
646-
value: argNode.value,
647-
})),
648-
};
649-
return print(sortValueNode(inputObjectWithArgs));
638+
function sameArguments(
639+
node1: FieldNode | DirectiveNode,
640+
node2: FieldNode | DirectiveNode,
641+
): boolean {
642+
const args1 = node1.arguments;
643+
const args2 = node2.arguments;
644+
645+
if (args1 === undefined || args1.length === 0) {
646+
return args2 === undefined || args2.length === 0;
647+
}
648+
if (args2 === undefined || args2.length === 0) {
649+
return false;
650+
}
651+
652+
/* c8 ignore next */
653+
if (args1.length !== args2.length) {
654+
/* c8 ignore next */
655+
return false;
656+
/* c8 ignore next */
657+
}
658+
659+
const values2 = new Map(args2.map(({ name, value }) => [name.value, value]));
660+
return args1.every((arg1) => {
661+
const value1 = arg1.value;
662+
const value2 = values2.get(arg1.name.value);
663+
if (value2 === undefined) {
664+
return false;
665+
}
666+
667+
return stringifyValue(value1) === stringifyValue(value2);
668+
});
669+
}
670+
671+
function stringifyValue(value: ValueNode): string | null {
672+
return print(sortValueNode(value));
650673
}
651674

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

0 commit comments

Comments
 (0)