Merge pull request #230 from graphql/diverging-directives

[validation] Allow differing directives.

Author: leebyron

src/validation/__tests__/OverlappingFieldsCanBeMerged.js

@@ -86,6 +86,18 @@ describe('Validate: Overlapping fields can be merged', () => {
     `);
   });
 
+  it('different skip/include directives accepted', () => {
+    // Note: Differing skip/include directives don't create an ambiguous return
+    // value and are acceptable in conditions where differing runtime values
+    // may have the same desired effect of including or skipping a field.
+    expectPassesRule(OverlappingFieldsCanBeMerged, `
+      fragment differentDirectivesWithDifferentAliases on Dog {
+        name @include(if: true)
+        name @include(if: false)
+      }
+    `);
+  });
+
   it('Same aliases with different field targets', () => {
     expectFailsRule(OverlappingFieldsCanBeMerged, `
       fragment sameAliasesWithDifferentFieldTargets on Dog {
@@ -191,66 +203,6 @@ describe('Validate: Overlapping fields can be merged', () => {
     `);
   });
 
-  it('conflicting directives', () => {
-    expectFailsRule(OverlappingFieldsCanBeMerged, `
-      fragment conflictingDirectiveArgs on Dog {
-        name @include(if: true)
-        name @skip(if: false)
-      }
-    `, [
-      { message: fieldsConflictMessage(
-          'name',
-          'they have differing directives'
-        ),
-        locations: [ { line: 3, column: 9 }, { line: 4, column: 9 } ] }
-    ]);
-  });
-
-  it('conflicting directive args', () => {
-    expectFailsRule(OverlappingFieldsCanBeMerged, `
-      fragment conflictingDirectiveArgs on Dog {
-        name @include(if: true)
-        name @include(if: false)
-      }
-    `, [
-      { message: fieldsConflictMessage(
-          'name',
-          'they have differing directives'
-        ),
-        locations: [ { line: 3, column: 9 }, { line: 4, column: 9 } ] }
-    ]);
-  });
-
-  it('conflicting args with matching directives', () => {
-    expectFailsRule(OverlappingFieldsCanBeMerged, `
-      fragment conflictingArgsWithMatchingDirectiveArgs on Dog {
-        doesKnowCommand(dogCommand: SIT) @include(if: true)
-        doesKnowCommand(dogCommand: HEEL) @include(if: true)
-      }
-    `, [
-      { message: fieldsConflictMessage(
-          'doesKnowCommand',
-          'they have differing arguments'
-        ),
-        locations: [ { line: 3, column: 9 }, { line: 4, column: 9 } ] }
-    ]);
-  });
-
-  it('conflicting directives with matching args', () => {
-    expectFailsRule(OverlappingFieldsCanBeMerged, `
-      fragment conflictingDirectiveArgsWithMatchingArgs on Dog {
-        doesKnowCommand(dogCommand: SIT) @include(if: true)
-        doesKnowCommand(dogCommand: SIT) @skip(if: false)
-      }
-    `, [
-      { message: fieldsConflictMessage(
-          'doesKnowCommand',
-          'they have differing directives'
-        ),
-        locations: [ { line: 3, column: 9 }, { line: 4, column: 9 } ] }
-    ]);
-  });
-
   it('encounters conflict in fragments', () => {
     expectFailsRule(OverlappingFieldsCanBeMerged, `
       {

src/validation/rules/OverlappingFieldsCanBeMerged.js

@@ -15,7 +15,6 @@ import type {
   SelectionSet,
   Field,
   Argument,
-  Directive
 } from '../../language/ast';
 import { FIELD, INLINE_FRAGMENT, FRAGMENT_SPREAD } from '../../language/kinds';
 import { print } from '../../language/printer';
@@ -136,14 +135,6 @@ export function OverlappingFieldsCanBeMerged(context: ValidationContext): any {
       ];
     }
 
-    if (!sameDirectives(ast1.directives || [], ast2.directives || [])) {
-      return [
-        [ responseName, 'they have differing directives' ],
-        [ ast1 ],
-        [ ast2 ]
-      ];
-    }
-
     var selectionSet1 = ast1.selectionSet;
     var selectionSet2 = ast2.selectionSet;
     if (selectionSet1 && selectionSet2) {
@@ -209,28 +200,6 @@ type ConflictReason = [ string, ConflictReasonMessage ];
 // Reason is a string, or a nested list of conflicts.
 type ConflictReasonMessage = string | Array<ConflictReason>;
 
-function sameDirectives(
-  directives1: Array<Directive>,
-  directives2: Array<Directive>
-): boolean {
-  if (directives1.length !== directives2.length) {
-    return false;
-  }
-  return directives1.every(directive1 => {
-    var directive2 = find(
-      directives2,
-      directive => directive.name.value === directive1.name.value
-    );
-    if (!directive2) {
-      return false;
-    }
-    return sameArguments(
-      directive1.arguments || [],
-      directive2.arguments || []
-    );
-  });
-}
-
 function sameArguments(
   arguments1: Array<Argument>,
   arguments2: Array<Argument>