Skip to content

Commit

Permalink
fix(merge): add default value for mergeDirectives (#1671)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rufman committed Jun 23, 2020
1 parent 4b20d8e commit 6bb170f
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 7 deletions.
10 changes: 5 additions & 5 deletions packages/merge/src/typedefs-mergers/directives.ts
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down
76 changes: 74 additions & 2 deletions 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';
Expand Down Expand Up @@ -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);
});
})
});

0 comments on commit 6bb170f

Please sign in to comment.