From 6bb170f69cc20916eb8330492573c9a26d4f44f5 Mon Sep 17 00:00:00 2001
From: Stephane Rufer <app.s@rufer.casa>
Date: Tue, 23 Jun 2020 09:29:41 -0700
Subject: [PATCH] fix(merge): add default value for `mergeDirectives` (#1671)

The GraphQL reference implementation treats directives  as optional,
so mergeDirectives needs to handle the `undefined` case, since some
libraries will set `directives` to be undefined instead of an empty list.

see https://github.com/graphql/graphql-js/blob/20a8f555e7eaa5c2fa2fd188abcef83ee8169e7f/src/language/ast.d.ts#L453
---
 .../merge/src/typedefs-mergers/directives.ts  | 10 +--
 packages/merge/tests/merge-typedefs.spec.ts   | 76 ++++++++++++++++++-
 2 files changed, 79 insertions(+), 7 deletions(-)

diff --git a/packages/merge/src/typedefs-mergers/directives.ts b/packages/merge/src/typedefs-mergers/directives.ts
index 19129445763..3b1579acb7a 100644
--- a/packages/merge/src/typedefs-mergers/directives.ts
+++ b/packages/merge/src/typedefs-mergers/directives.ts
@@ -10,7 +10,7 @@ function nameAlreadyExists(name: NameNode, namesArr: ReadonlyArray<NameNode>): b
   return namesArr.some(({ value }) => value === name.value);
 }
 
-function mergeArguments(a1: ArgumentNode[], a2: ArgumentNode[]): ArgumentNode[] {
+function mergeArguments(a1: readonly ArgumentNode[], a2: readonly ArgumentNode[]): ArgumentNode[] {
   const result: ArgumentNode[] = [...a2];
 
   for (const argument of a1) {
@@ -57,8 +57,8 @@ function deduplicateDirectives(directives: ReadonlyArray<DirectiveNode>): Direct
 }
 
 export function mergeDirectives(
-  d1: ReadonlyArray<DirectiveNode>,
-  d2: ReadonlyArray<DirectiveNode>,
+  d1: ReadonlyArray<DirectiveNode> = [],
+  d2: ReadonlyArray<DirectiveNode> = [],
   config?: Config
 ): DirectiveNode[] {
   const reverseOrder: boolean = config && config.reverseDirectives;
@@ -71,8 +71,8 @@ export function mergeDirectives(
       const existingDirectiveIndex = result.findIndex(d => d.name.value === directive.name.value);
       const existingDirective = result[existingDirectiveIndex];
       (result[existingDirectiveIndex] as any).arguments = mergeArguments(
-        directive.arguments as any,
-        existingDirective.arguments as any
+        directive.arguments || [],
+        existingDirective.arguments || []
       );
     } else {
       result.push(directive);
diff --git a/packages/merge/tests/merge-typedefs.spec.ts b/packages/merge/tests/merge-typedefs.spec.ts
index 93714e94791..efb4b9c6f09 100644
--- a/packages/merge/tests/merge-typedefs.spec.ts
+++ b/packages/merge/tests/merge-typedefs.spec.ts
@@ -1,7 +1,7 @@
-import { mergeTypeDefs, mergeGraphQLTypes } from '../src';
+import { mergeDirectives, mergeTypeDefs, mergeGraphQLTypes } from '../src';
 import { makeExecutableSchema } from '@graphql-tools/schema';
 import { stitchSchemas } from '@graphql-tools/stitch'
-import { buildSchema, buildClientSchema, print, parse } from 'graphql';
+import { buildSchema, buildClientSchema, print, parse, Kind } from 'graphql';
 import { stripWhitespaces } from './utils';
 import gql from 'graphql-tag';
 import { readFileSync } from 'fs';
@@ -1272,4 +1272,76 @@ describe('Merge TypeDefs', () => {
     expect(result).toContain(`#  - second line2`);
   });
 
+  describe.each([['normal', false], ['reverse', true]])('mergeDirectives', (direction, value) => {
+    const config = {
+      reverseDirectives: value,
+    };
+
+    it(`should merge with both schema directives undefined in ${direction} order`, () => {
+      expect(mergeDirectives(undefined, undefined, config)).toEqual([]);
+    });
+
+    it(`should merge with first schema directives set in ${direction} order`, () => {
+      const directives = [{
+        kind: Kind.DIRECTIVE,
+        name: {
+          kind: Kind.NAME,
+          value: 'firstDirective'
+        }
+      }];
+      expect(mergeDirectives(directives, undefined, config)).toEqual(directives);
+    });
+
+    it(`should merge with second schema directives set in ${direction} order`, () => {
+      const directives = [{
+        kind: Kind.DIRECTIVE,
+        name: {
+          kind: Kind.NAME,
+          value: 'firstDirective'
+        }
+      }];
+      expect(mergeDirectives(undefined, directives, config)).toEqual(directives);
+    });
+
+    it(`should merge with both schema directives set in ${direction} order`, () => {
+      const directives = [{
+        kind: Kind.DIRECTIVE,
+        name: {
+          kind: Kind.NAME,
+          value: 'firstDirective'
+        }
+      }];
+      expect(mergeDirectives(directives, directives, config)).toEqual(directives);
+    });
+
+    it(`should merge with both schema directives set, one of which has arguments in ${direction} order`, () => {
+      const directivesOne = [{
+        kind: Kind.DIRECTIVE,
+        name: {
+          kind: Kind.NAME,
+          value: 'firstDirective'
+        }
+      }];
+      const directivesTwo = [{
+        kind: Kind.DIRECTIVE,
+        name: {
+          kind: Kind.NAME,
+          value: 'firstDirective'
+        },
+        arguments: [{
+          kind: Kind.ARGUMENT,
+          name: {
+            kind: Kind.NAME,
+            value: 'firstArg'
+          },
+          value: {
+            kind: Kind.STRING,
+            value: 'arg'
+          }
+        }]
+      }];
+
+      expect(mergeDirectives(directivesOne, directivesTwo, config)).toEqual(directivesTwo);
+    });
+  })
 });