Skip to content

Commit

Permalink
review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
IvanGoncharov committed Sep 5, 2023
1 parent 70ff0ed commit 2991112
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 13 deletions.
18 changes: 18 additions & 0 deletions src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
37 changes: 24 additions & 13 deletions src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import type { Maybe } from '../../jsutils/Maybe.js';
import { GraphQLError } from '../../error/GraphQLError.js';

import type {
ArgumentNode,
DirectiveNode,
FieldNode,
FragmentDefinitionNode,
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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));
}

Expand All @@ -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;
Expand Down

0 comments on commit 2991112

Please sign in to comment.