diff --git a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts index 582e4602af..af91496b22 100644 --- a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts +++ b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts @@ -98,13 +98,22 @@ describe('Validate: Overlapping fields can be merged', () => { `); }); - it('Same stream directives supported', () => { - expectValid(` + it('stream directive used on different instances of the same field', () => { + expectErrors(` fragment differentDirectivesWithDifferentAliases on Dog { name @stream(label: "streamLabel", initialCount: 1) name @stream(label: "streamLabel", initialCount: 1) } - `); + `).toDeepEqual([ + { + message: + 'Fields "name" conflict because they have overlapping stream directives. See https://github.com/graphql/defer-stream-wg/discussions/100. Use different aliases on the fields to fetch both if this was intentional.', + locations: [ + { line: 3, column: 9 }, + { line: 4, column: 9 }, + ], + }, + ]); }); it('different stream directive label', () => { @@ -116,7 +125,7 @@ describe('Validate: Overlapping fields can be merged', () => { `).toDeepEqual([ { message: - 'Fields "name" conflict because they have differing stream directives. Use different aliases on the fields to fetch both if this was intentional.', + 'Fields "name" conflict because they have overlapping stream directives. Use different aliases on the fields to fetch both if this was intentional.', locations: [ { line: 3, column: 9 }, { line: 4, column: 9 }, @@ -134,7 +143,7 @@ describe('Validate: Overlapping fields can be merged', () => { `).toDeepEqual([ { message: - 'Fields "name" conflict because they have differing stream directives. Use different aliases on the fields to fetch both if this was intentional.', + 'Fields "name" conflict because they have overlapping stream directives. Use different aliases on the fields to fetch both if this was intentional.', locations: [ { line: 3, column: 9 }, { line: 4, column: 9 }, @@ -152,7 +161,7 @@ describe('Validate: Overlapping fields can be merged', () => { `).toDeepEqual([ { message: - 'Fields "name" conflict because they have differing stream directives. Use different aliases on the fields to fetch both if this was intentional.', + 'Fields "name" conflict because they have overlapping stream directives. Use different aliases on the fields to fetch both if this was intentional.', locations: [ { line: 3, column: 9 }, { line: 4, column: 9 }, @@ -170,7 +179,7 @@ describe('Validate: Overlapping fields can be merged', () => { `).toDeepEqual([ { message: - 'Fields "name" conflict because they have differing stream directives. Use different aliases on the fields to fetch both if this was intentional.', + 'Fields "name" conflict because they have overlapping stream directives. Use different aliases on the fields to fetch both if this was intentional.', locations: [ { line: 3, column: 9 }, { line: 4, column: 9 }, @@ -188,7 +197,7 @@ describe('Validate: Overlapping fields can be merged', () => { `).toDeepEqual([ { message: - 'Fields "name" conflict because they have differing stream directives. Use different aliases on the fields to fetch both if this was intentional.', + 'Fields "name" conflict because they have overlapping stream directives. Use different aliases on the fields to fetch both if this was intentional.', locations: [ { line: 3, column: 9 }, { line: 4, column: 9 }, @@ -206,7 +215,7 @@ describe('Validate: Overlapping fields can be merged', () => { `).toDeepEqual([ { message: - 'Fields "name" conflict because they have differing stream directives. Use different aliases on the fields to fetch both if this was intentional.', + 'Fields "name" conflict because they have overlapping stream directives. Use different aliases on the fields to fetch both if this was intentional.', locations: [ { line: 3, column: 9 }, { line: 4, column: 9 }, @@ -216,12 +225,21 @@ describe('Validate: Overlapping fields can be merged', () => { }); it('different stream directive both missing args', () => { - expectValid(` + expectErrors(` fragment conflictingArgs on Dog { name @stream name @stream } - `); + `).toDeepEqual([ + { + message: + 'Fields "name" conflict because they have overlapping stream directives. See https://github.com/graphql/defer-stream-wg/discussions/100. Use different aliases on the fields to fetch both if this was intentional.', + locations: [ + { line: 3, column: 9 }, + { line: 4, column: 9 }, + ], + }, + ]); }); it('Same aliases with different field targets', () => { diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts index c74ecc7fe5..cacee0c77a 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts +++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts @@ -707,12 +707,14 @@ function findConflict( const directives1 = node1.directives ?? []; const directives2 = node2.directives ?? []; - if (!sameStreams(directives1, varMap1, directives2, varMap2)) { - return [ - [responseName, 'they have differing stream directives'], - [node1], - [node2], - ]; + const overlappingStreamReason = hasNoOverlappingStreams( + directives1, + varMap1, + directives2, + varMap2, + ); + if (overlappingStreamReason !== undefined) { + return [[responseName, overlappingStreamReason], [node1], [node2]]; } // The return type for each field. @@ -830,28 +832,27 @@ function getStreamDirective( return directives.find((directive) => directive.name.value === 'stream'); } -function sameStreams( +function hasNoOverlappingStreams( directives1: ReadonlyArray, varMap1: Map | undefined, directives2: ReadonlyArray, varMap2: Map | undefined, -): boolean { +): string | undefined { const stream1 = getStreamDirective(directives1); const stream2 = getStreamDirective(directives2); if (!stream1 && !stream2) { // both fields do not have streams - return true; + return; } else if (stream1 && stream2) { // check if both fields have equivalent streams - return sameArguments( - stream1.arguments, - varMap1, - stream2.arguments, - varMap2, - ); + if (sameArguments(stream1.arguments, varMap1, stream2.arguments, varMap2)) { + // This was allowed in previous alpha versions of `graphql-js`. + return 'they have overlapping stream directives. See https://github.com/graphql/defer-stream-wg/discussions/100'; + } + return 'they have overlapping stream directives'; } // fields have a mix of stream and no stream - return false; + return 'they have overlapping stream directives'; } // Two types conflict if both types could not apply to a value simultaneously.